Overview
Features
Download
Documentation
Community
Add-Ons & Services

websocket small frames bug and proposed fix

Please post support and help requests here.

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...
kolijoco
 
Posts: 7
Joined: 16 Feb 2010, 10:35

Re: websocket small frames bug and proposed fix

Postby alex » 31 Jul 2012, 18:25

Thanks, I filed SF patch.
alex
 
Posts: 1105
Joined: 11 Jul 2006, 16:27
Location: United_States

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!
kolijoco
 
Posts: 7
Joined: 16 Feb 2010, 10:35

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.
alex
 
Posts: 1105
Joined: 11 Jul 2006, 16:27
Location: United_States

Re: websocket small frames bug and proposed fix

Postby guenter » 10 Aug 2012, 18:59

fix is in 1.4.4 SVN
guenter
 
Posts: 1112
Joined: 11 Jul 2006, 16:27
Location: Austria


Return to Support

Who is online

Users browsing this forum: No registered users and 4 guests