websocket small frames bug and proposed fix

[kolijoco]

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]

Thanks, I filed SF patch.

[kolijoco]

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]

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

done.

[guenter]

fix is in 1.4.4 SVN