websocket small frames bug and proposed fix

Please post support and help requests here.
kolijoco
Posts: 7
Joined: 16 Feb 2010, 10:35

websocket small frames bug and proposed fix

Postby kolijoco » 31 Jul 2012, 17:23

i found the following bug in the websocket implementation (found in version 1.4.3p1 on windows on the client side):
in case the total size of an incoming frame is smaller then MAX_HEADER_LENGTH (=14) - for example a single character in a text frame = 3 bytes in total -, AND successive frames are already queued in the receiving buffer, the current implementation of

Code: Select all

int WebSocketImpl::receiveBytes(void* buffer, int length, int)
will always read (and digest/remove!!!) the first 14 bytes of the receving buffer, thus swallowing the first bytes of the next frame and corrupting the stream.

(my app is streaming alternating small text and large binary payloads)
i recommend a fix by introducing MIN_HEADER_SIZE(=2) constant, and modifying the beginning of receiveBytes() to:

Code: Select all

int WebSocketImpl::receiveBytes(void* buffer, int length, int)
{
   char header[MAX_HEADER_LENGTH];
   int n = _pStreamSocketImpl->receiveBytes(header, MIN_HEADER_LENGTH);
   Poco::UInt8 lengthByte = ((Poco::UInt8)header[1]) & 0x7f;
   if( lengthByte+MIN_HEADER_LENGTH < MAX_HEADER_LENGTH )
      n += _pStreamSocketImpl->receiveBytes(header + MIN_HEADER_LENGTH, lengthByte );
   else
      n += _pStreamSocketImpl->receiveBytes(header + MIN_HEADER_LENGTH, MAX_HEADER_LENGTH - MIN_HEADER_LENGTH);
   if (n > 0)
   { //... the rest is unchanged



hope this helps some people. i also haven't checked for similar mechanics elsewhere, i'm sure the guys responsible for the websocket implementation will know where else to look...

alex
Posts: 1255
Joined: 11 Jul 2006, 16:27
Location: United_States

Re: websocket small frames bug and proposed fix

Postby alex » 31 Jul 2012, 18:25

Thanks, I filed SF patch.

kolijoco
Posts: 7
Joined: 16 Feb 2010, 10:35

Re: websocket small frames bug and proposed fix

Postby kolijoco » 10 Aug 2012, 11:36

unfortunately there is more :-(
this took a while to hunt down. there is also a problem when the initial read

Code: Select all

int n = _pStreamSocketImpl->receiveBytes(header, MIN_HEADER_LENGTH);
only returns a single byte (the opcode). my suggested solution was unsafe, because it assumed the second byte (payload length) has also been read - silly me. i've hit this on iphone/simulator, but could probably happen elsewhere. seems a successive read attempt will read the byte needed, and all gets well... so the proposed fix to my proposed fix:

Code: Select all

int WebSocketImpl::receiveBytes(void* buffer, int length, int)
{
   char header[MAX_HEADER_LENGTH];
   int n = _pStreamSocketImpl->receiveBytes(header, MIN_HEADER_LENGTH);
    if( n == 0 )
        return 0;
    if( n == 1 )
    {
        // we still need the second byte for the minimum header
        if( _pStreamSocketImpl->receiveBytes(header+1, 1) != 1 )
            return 0;
        else
            n = 2;
    }
   Poco::UInt8 lengthByte = ((Poco::UInt8)header[1]) & 0x7f;
   if( lengthByte+MIN_HEADER_LENGTH < MAX_HEADER_LENGTH )
      n += _pStreamSocketImpl->receiveBytes(header + MIN_HEADER_LENGTH, lengthByte );
   else
      n += _pStreamSocketImpl->receiveBytes(header + MIN_HEADER_LENGTH, MAX_HEADER_LENGTH - MIN_HEADER_LENGTH);
   if (n > 0)
   { ... // rest is unchanged
sorry for the misfix :-)
cheers
@alex: please update the SF patch, i haven't tried using it... thanks!

alex
Posts: 1255
Joined: 11 Jul 2006, 16:27
Location: United_States

Re: websocket small frames bug and proposed fix

Postby alex » 10 Aug 2012, 16:42

kolijoco wrote:@alex: please update the SF patch, i haven't tried using it... thanks!

done.

guenter
Posts: 1186
Joined: 11 Jul 2006, 16:27
Location: Austria

Re: websocket small frames bug and proposed fix

Postby guenter » 10 Aug 2012, 18:59

fix is in 1.4.4 SVN


Return to “Support”

Who is online

Users browsing this forum: No registered users and 2 guests