allinurl / gwsocket

fast, standalone, language-agnostic WebSocket server RFC6455 compliant
http://gwsocket.io
MIT License
745 stars 67 forks source link

Strict protocol parsing #30

Closed itsanov closed 3 years ago

itsanov commented 4 years ago

Hi, First and most important: Nice work! Thank you! I'm trying to connect Arduino with web browser with "gwsocket". Communication is binary. gwsocket --strict --pipein=/dev/ttyS1 --pipeout=/dev/ttyS1 --max-frame-size=128 Unfortunately, when Arduino is restarted gwsocket strict protocol parser gets out of sync and can't return back. I have implemented a workaround. Please have a look. websocket.c:2687

   if (validate_fifo_packet (listener, type, size) == 1) {
 //    close (pi->fd);
 //    clear_fifo_packet (pi);
 //    ws_openfifo_in (pi);
     for(int i = 0; i < HDR_SIZE - 1; i++)
         pi->hdr[i] = pi->hdr[i+1];
     pi->hlen--;
     return;
   }

Current implementation is throwing away the complete header (12 bytes) when header is not OK. My change is to throw away just one byte and to try again.

Cheers

allinurl commented 4 years ago

Thanks for posting this. Could you please explain a bit more what do you mean with gwsocket strict protocol parser gets out of sync and can't return back.?

Thanks

itsanov commented 4 years ago

Sure, GWSocket is listening on serial port, Arduino is writing. "Strict" communication protocol is used. Arduino is sending following header [0x00000000 (broadcast), 0x00000002 (binary), 0x00000008 (8 bytes payload)] and after the header Arduino proceeds with payload, but by some reason communication is interrupted after the 5-th byte of the payload (for example Arduino has been restarted). GWSocket is still waiting to receive 3 more bytes payload, but the Arduino does not knows that and is starting a new header. GWSocket is receiving those 3 bytes, completes the current packet and starts waiting for the next header. GWSocket is receiving the next portion of 12 bytes, but this is not a valid header any more, because first 3 bytes from this header were consumed by the previous packet. GWSocket is throwing away this header because it is invalid, closing and reopening the socket and starts waiting for the next header, which is again an invalid header and if the Arduino is sending even number of bytes in payload, this never ends. GWSocket has no chance to receive valid header, because it always starts from odd byte. My fix is, instead of throwing away complete header, to just loose first byte (shifting buffer and reducing the number of received header bytes) , take one more from the stream and to try again to validate header. And it works very well in my case.

allinurl commented 4 years ago

Thank you for expanding on this issue. I'm happy to merge this. However, just to be sure, would you be able to run this on a non-arduino machine and see if this change still produces the right output? Let me know.

itsanov commented 4 years ago

I have tested gwsocket on Fedora-x64 as well as Lede(OpenWRT)-mips_24kc. As a remote point for strict protocol I have used Arduino and I also tested with pipe-in/pipe-out commands simulation on a local machine. I'm not aware of the reason for closing and reopening of the fifo in case of invalid header. I believe that the fifo should not be closed/reopened, so I've commented that out. Please double check that.

itsanov commented 4 years ago

Hi @allinurl, I see that my proposal has not been applied so far and the issue is still open. Do you like me to make a pull request for it?

allinurl commented 4 years ago

@itsanov Thanks for the reminder. Yes, please feel free to submit a PR and I'm happy to take a look and merge it upstream.

allinurl commented 3 years ago

Closing this as it appears it has been addressed in #33.

Feel free to reopen it if needed