search

esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
15.99k stars 13.34k forks source link

incorrect bug fix. ESP8266 url encoded delimitters parsed incorrectly #3415

Closed Jeroen88 closed 7 years ago

Jeroen88 commented 7 years ago

Basic Infos

incorrect bug fix. ESP8266 url encoded delimitters parsed incorrectly

Hardware

Hardware: ESP-12E Core Version: 2.4.0-rc1

Description

Problem description URL encoded delimitter characters are decoded to early in the latest committed bug fix. A %-encoded '?', '=' and '&' are decoded to early. They should be decoded later. So line 94: searchString = urlDecode(url.substring(hasSearch + 1)); should be replaced (back) by searchString = url.substring(hasSearch + 1); And line 320: arg.key = data.substring(pos, equal_sign_index); should be replaced by arg.key = urlDecode(data.substring(pos, equal_sign_index)); line 321: arg.value = data.substring(equal_sign_index + 1, next_arg_index); should be replaced by arg.value = urlDecode(data.substring(equal_sign_index + 1, next_arg_index));

devyte commented 7 years ago

Core 2.1-rc2 is very old, and not even a stable release. Please try 2.3, or better yet, latest git.

On Jul 12, 2017 3:31 AM, "Jeroen88" notifications@github.com wrote:

Basic Infos

incorrect bug fix. ESP8266 url encoded delimitters parsed incorrectly Hardware

Hardware: ESP-12E Core Version: 2.1.0-rc2 Description

Problem description URL encoded delimitter characters are decoded to early in the latest committed bug fix. A %-encoded '?', '=' and '&' are decoded to early. They should be decoded later. So line 94: searchString = urlDecode(url.substring(hasSearch + 1)); should be replaced (back) by searchString = url.substring(hasSearch + 1); And line 320: arg.key = data.substring(pos, equal_sign_index); should be replaced by arg.key = urlDecode(data.substring(pos, equal_sign_index)); line 321: arg.value = data.substring(equal_sign_index + 1, next_arg_index); should be replaced by arg.value = urlDecode(data.substring(equal_sign_index

  • 1, next_arg_index));

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/esp8266/Arduino/issues/3415, or mute the thread https://github.com/notifications/unsubscribe-auth/AQC6Bkbm-i7SbKgSQgMJhJTKZT1TBTQ4ks5sNHY1gaJpZM4OVO4s .

Jeroen88 commented 7 years ago

@devte, thanx for your comment, the bad fix is in 2.4.0-rc1. I just updated my issue!

earlephilhower commented 7 years ago

Sorry, @Jeroen88, but I've not really used the built-in web server so I don't think I can help with a pull in this case because I wouldn't be able to test it. I suggest you fork the repo and apply your 2-line patch, verify it fixes your problem and doesn't break anything else, then submit in a PR on the 2.4.0-rc1 codebase. It's very easy to do right from the github web interface, even.

earlephilhower commented 7 years ago

Actually, @Jeroen88 , check out this PR...it seems to fix what you're seeing: https://github.com/esp8266/Arduino/pull/2956

Jeroen88 commented 7 years ago

Thank you @earlephilhower, I am going to try that later...!

Jeroen88 commented 7 years ago

@earlephilhower, same issue, indeed. But "This branch is out-of-date with the base branch" let me think that this will not be used?

earlephilhower commented 7 years ago

No it should be fime. They just need to merge it with the latest head. It's a 1-button click operation unless there are merge conflicts.

Jeroen88 commented 7 years ago

Thanks again for your help :)