incorrect bug fix. ESP8266 url encoded delimitters parsed incorrectly

[Jeroen88]

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]

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" ***@***.***> 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 <#3415>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQC6Bkbm-i7SbKgSQgMJhJTKZT1TBTQ4ks5sNHY1gaJpZM4OVO4s> .

[Jeroen88]

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

[earlephilhower]

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]

Actually, @Jeroen88 , check out this PR...it seems to fix what you're seeing:
#2956

[Jeroen88]

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

[Jeroen88]

@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]

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]

Thanks again for your help :)