atanisoft / HttpServer

HTTP Server implemented using OpenMRN StateFlow patterns.
BSD 2-Clause "Simplified" License
1 stars 0 forks source link

WebSockets do not work consistently - return key errors due to data corruption #2

Closed TrainzLuvr closed 4 years ago

TrainzLuvr commented 4 years ago

After trying all 3 above mentioned browsers via WebUI, I cannot get my loco to move at all. DCC commands are not being communicated from the UI due to some error/timeout with WebSockets.

If I try to do this from Chromium on my desktop computer, I can control the locomotive no problem.

TrainzLuvr commented 4 years ago

I can also confirm that using Firefox on Android, this issue is not present and I can control locomotives through the WebUI.

atanisoft commented 4 years ago

@TrainzLuvr if you can capture the console output with CONFIG_HTTP_WS_LOGGING_VERBOSE=y set I might be able to spot where the connection is being closed by the iOS device.

TrainzLuvr commented 4 years ago

It seems that something is wrong with key generation, more precisely ws_key https://github.com/atanisoft/ESP32CommandStation/blob/d51586baf145695a7ec033258be6f7dcf1e9099c/components/Esp32HttpServer/HttpRequestWebSocket.cpp#L48 somehow ends up having a space in the string, which comes from req_.header(HttpHeader::WS_KEY) https://github.com/atanisoft/ESP32CommandStation/blob/fd3b7369bb8aeb8010c2d6f5ad817f2c55a5bbf7/components/Esp32HttpServer/HttpRequestFlow.cpp#L952

21:44:03.109 -> Sec-WebSocket-Key=W jfut09KUUoN5cHlwZbow== 21:44:03.109 -> Sec-WebSocket-Accept=RlAs59dpqJ1SfHoCxANQzYXfI2s= 21:44:03.109 -> [WS 192.168.x.x/51] Connected 21:44:03.109 -> [WebSocket fd:51] read-error (128: Socket is not connected), disconnecting (recv)

Perhaps the req_ gets corrupted somewhere in the flow, not sure. But ws_key ends up with a space in it, and I suspect that space could represent missing characters not necessarily an inserted space character.

atanisoft commented 4 years ago

But ws_key ends up with a space in it, and I suspect that space could represent missing characters not necessarily an inserted space character.

Yes, it looks like something is corrupting the ws_key which comes in from the incoming request header "Sec-WebSocket-Key". Can you add the following log statement here to print the raw header and the parsed header?

LOG_ERROR("[Httpd fd:%d] raw:%s\npart1:%s\npart2:%s", fd_, line.c_str(), h.first.c_str(), h.second.c_str());

I'm guessing there is something in the raw parts of the string that is not working for url_decode (the next couple lines)

atanisoft commented 4 years ago

@TrainzLuvr I'm guessing that the space in the path is actually a "+" after the url_decode calls. I can add a workaround for this possibly.

TrainzLuvr commented 4 years ago

There we go, it's the + that goes missing. :)

22:16:59.797 -> [Httpd fd:51] raw:Sec-WebSocket-Key: dc5WbJlJRDR+GR9rxsH5Lg== 22:16:59.797 -> part1:Sec-WebSocket-Key 22:16:59.797 -> part2:dc5WbJlJRDR+GR9rxsH5Lg== 22:16:59.797 -> [Httpd fd:51] raw:Sec-WebSocket-Extensions: permessage-deflate; client_max_window_bits 22:16:59.797 -> part1:Sec-WebSocket-Extensions 22:16:59.797 -> part2:permessage-deflate; client_max_window_bits 22:16:59.797 -> Sec-WebSocket-Key=dc5WbJlJRDR GR9rxsH5Lg== 22:16:59.797 -> Sec-WebSocket-Accept=Btezc58MSYRg3s1O5taLj5weSxs= 22:17:00.839 -> [WS 192.168.x.x/51] Connected 22:17:00.839 -> [WebSocket fd:51] read-error (128: Socket is not connected), disconnecting (recv) 22:17:00.839 -> [WS 192.168.x.x/51] Disconnected

atanisoft commented 4 years ago

Ok that confirms that iOS is not using the URL Safe base64 variant. Let me add a workaround to replace spaces with + on the key only.

TrainzLuvr commented 4 years ago

This is not iOS only, it is all browsers, presently I'm using my Windows Desktop Chromium.

atanisoft commented 4 years ago

I haven't seen the + used in the key but it is a valid base64 character and the websocket RFC gives a couple character sets to use it is chance when it might be used. So for the safe side it is reverted when seen.