Broken HTTP Station after commit fd7cdad

[marcinkowalczyk]

Hi,

HTTP API get for running zone is broken - when saving details all data goes into server and port, on off gets undef variable. In such config HTTP is not called. Latest working commit is fd7cdad

[rayshobby]

What is the specific on and off command you are setting? It likely has to do with URI encoding (like if your command contains space or other special characters).

[marcinkowalczyk]

It doesn't matter. Just on or off texts

[rayshobby]

That sounds like a UI related issue. What app version are you on? I suggest clearing browser cache and try again.

[marcinkowalczyk]

Tried only with Chrome. Couple of people on Domoticz UG Poland had same problem.

[rayshobby]

I was asking about app version: if you go to the app's About page, what app version does it say? The most recent version is 1.7.7.

[marcinkowalczyk]

Sorry missed app word :-)

Currently I've sticked to last working commit fd7cdad while master branch is ahead like 6 commits from 27 may.

App shows: 1.7.7
Firmware 2.1.7 (2)
HW: OSpi-AC

Tomorrow I will check above with latest master.

[marcinkowalczyk]

Latest master shows
app: 1.7.7
Firmare 2.1.8
HW: OSPi

commit: f9ab487

[filipejc]

I have the same problem.

App: 1.7.7
Firmware: 2.1.8
HW version: OSPi

Tried with Firefox: 61.0.1
And Chrome: 67.0.3396.99

[PeteBa]

I can also replicate this using latest App and Firmware on OSPi. Linking to related forum post here and corresponding App issue here.

Looks due to a change in this commit that comments out the urlDecode line here.

So the App is still sending the special station configuration as an encoded message but it doesn't get decoded in Firmware into a comma separated set of configuration options and ends up a garbled setting.

@rayshobby, looks like the commenting out was an intentional change and appreciate any insight so we can try and work out a fix ?

For those using OSPi you can use a hex editor (sudo apt-get install hexeditor) to modify the incorrect setting in stns.dat file in pi home directory as a workaround.

[rayshobby]

I think I remember the reason why it was commented out: there was an earlier request to fix the HTTP command issue, that involved a HTTP command that contains a space character, which is quite unusual. I concluded that the url encoding should be done on the app side, otherwise the space character will be lost when it's received on the firmware side. For that reason, the assumption is not that the app will encode the url, and the firmware merely stores and uses it as is. I've already communicated with Samer so we are on the same page and it should be addressed in the app soon.

[rayshobby]

I figured out the issue. It has two parts: first, when the UI encodes the command, comma is also encoded (previously I thought it doesn't); second, ESP8266's server functions automatically perform a urlDecode, and when I was testing HTTP station feature I was using OS3.0 as testing platform so it all worked fine, but I didn't know the decode was done automatically, which is why I didn't know the issue happens on other platforms like OSPi. In any case, now I understand the issue, we will recover it to the previous code (making an exception for ESP8266 since it already performs decoding internally). The app will also be fixed so that it's consistent with firmware, and that the import/export will be correct too.

[rayshobby]

This issue has now been fixed by recovering the urlDecode after HTTP command is received.

[PeteBa]

@rayshobby , could you possibly test the following scenario on OS3.0 in HTTP Station settings:

server: os3
port: 8080
on command: json.htm?type=command&param=switchlight&idx=99&switchcmd=On
off command: json.htm?type=command&param=switchlight&idx=99&switchcmd=Off 

Note the "&" in the on and off command. The "&" character will be encoded in UI to %26 and it should stay encoded until Firmware is ready to parse out the various sections of the special station data.

From your comments, if the ESP8266 decodes the received uri before calling server_change_stations() then the sd parameter will get truncated as the "&" will have been decoded and findKeyVal(..., PSTR("sd"), ...) will interpret as a termination character.

Unfortunately, I cant test this here as only have OSPi but your description of how the ESP8266 code works has left me a bit concerned that this may not be fully fixed. Sorry if I have misunderstood.

[marcinkowalczyk]

I've just tried to do following on latest master

on: control?cmd=event,startwatering
off: control?cmd=event,stopwatering

but it splits to:
on: /control?cmd
off: event,stopwatering

Can't check on 3.0 as I don't see 3.0 branch and master shows

App: 1.8.0
firmware: 2.1.8
HW: OSPi

remote branches

origin/2.1.8
origin/HEAD -> origin/master
origin/fix-2.1.8-CORS
origin/fix-ospi-build-error
origin/master

[rayshobby]

There is no separate 3.0 branch -- the firmware is united so the same firmware compiles on all platforms.

One quick note: your HTTP command cannot have comma (,) because uses comma as delimiter to split the entire string that includes the IP address, port number, on command, and off command.

[rayshobby]

The urlDeocde appears here:
https://github.com/OpenSprinkler/OpenSprinkler-Firmware/blob/master/server.cpp#L720
so it's after findKeyVal is already parsed. I will test this on OS3.0 later, but I am pretty sure I tested this specifically for OS3.0 before, because I use it for triggering OpenGarage and the command has at least one & in it.

[marcinkowalczyk]

Please remeber, that coma is a valid URL character and encodes as %2C so in general this is a bug.

[PeteBa]

Was just googling valid uri characters when I saw marcinkowalczyk, post 8)

@rayshobby, I'm happy to submit a PR to swap delimiter to an unused uri character to avoid the "," clash

[rayshobby]

@marcinkowalczyk: I was not saying that comma is in general invalid, I was just saying that this particular implementation does not allow comma in the command because it uses it for delimiter. This is of course fixable, by doing an individual uri encoding on the on command and off command, instead of encoding the entire string. It will involve both UI and firmware changes, which I've written down on the todo list. For now, I think one work-around is to replace , in your command by %2C, essentially double encoding it. See if this works.

@PetaBa: I took a closer look at the ESP8266 core library and I figured out that the & thing you mentioned is not a problem. Here is why: when the UI sends out the HTTP GET command to the firmware, & is in general not encoded, it's only the & in the on and off command strings are encoded as the UI intentionally does that. On the ESP8266 side, when it receives the GET command, it parses the string immediately, splitting it around &. It then calls uridecode on the key and value separately, and stores the key-value pairs into an array. The findKeyVal implementation in the firmware simply searches through the array to find the key. At that point, the key and value are both already decoded. So as you can see, it works because the & in general is not encoded in an HTTP GET command, so can be used to split the parameters; and ESP8266 automatically applies decode on the key and value after they are parsed:
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WebServer/src/Parsing.cpp#L325
Hope this makes sense.

[PeteBa]

Fantastic that's reassuring. I now get how the ESPWebServer works and quite a good way of doing things. I can also see the ESP8266 ifdef in findKeyVal() so get how it fits together end-to-end. Thanks for the link/explanation.