skaarj1989/mWebSockets

On message type Binary length uses strlen so length is incorrect

daverathbone opened this issue · 6 comments

When sending binary data to the Websocket Server the:-

🔴

ws.onMessage(WebSocket &ws, WebSocketDataType dataType, const char *message, uint16_t length)

length is ok on char data that is string based (e.g. Hello.txt) but not ok binary based (e.g. mypic.png)

I thought ArrayBuffer can be any 8 bit unsigned binary data?

🔵

Sending to my node.js websocket server it works.
I think the problem is in WebSocket.cpp line 411 e.g.

onMessage_(*this, BINARY, dataBuffer_, strlen(dataBuffer_));
which would get strange lengths on data that would contain byte value 0;

😥

I hope (I/we) can sort this out, as it renders the websocket server to text only!
unless I'm missing something?

Hi, how big is that binary file?

Hi I fixed it.
I chunk all the binary files to the Web socket server at 256 and then changed line 411 to

onMessage_(*this, BINARY, dataBuffer_, header.length); // BUG NOT strlen(dataBuffer_));

There is then no limit to the file size.

Using this at the JavaScript browser send:

const BYTES_PER_CHUNK = 256;
var file;
var currentChunk;
var fileInput = $( 'input[type=file]' );
var fileReader = new FileReader();

function readNextChunk() {
    var start = BYTES_PER_CHUNK * currentChunk;
    var end = Math.min( file.size, start + BYTES_PER_CHUNK );
    fileReader.readAsArrayBuffer( file.slice( start, end ) );
}

fileReader.onload = function() {
    ws.send( fileReader.result );  // Web socket send 
    currentChunk++;

    if( BYTES_PER_CHUNK * currentChunk < file.size ) {
        readNextChunk();
    }
};

fileInput.on( 'change', function() {
    file = fileInput[ 0 ].files[ 0 ];
    currentChunk = 0;
    // send some metadata about our file
    // to the receiver -  I convert this in the Arduino to switch to save to SDcard
   ws.send(JSON.stringify({
        fileName: file.name,
        fileSize: file.size
    }));
    readNextChunk();
});

Hi David - Update:

There was two fixes needed to read binary data:
Having played around with it further - this is what now works.
Can you update the version and close this, if you feel this is correct?

Here is the fix to WebSocket.cpp from lines 406-417

case BINARY_FRAME: {
  // strncat(dataBuffer_, payload, header.length);  // Not required not a string

  if (header.fin) {			
        if (onMessage_)
	 onMessage_(*this, BINARY, payload, header.length);
         // Bug was - onMessage_(*this, BINARY, dataBuffer_, strlen(dataBuffer_));
	memset(dataBuffer_, '\0', BUFFER_MAX_SIZE); //Can go no tested so left it.
    }
    break;
}

Unfortunately it's not that simple. Your fix won't work with fragmented binary data. Today (in couple of hours) 'm gonna submit updated code with some fixes here and there.

Hi Dave,
I can see your updates.. (many thanks).
Was your concern that websocket binary packets could arrive in any order?
I have just been lucky with around 40K image files @ 256 chunks arriving correctly.
Tried test video file from browser upload to Arduino SDcard (BigBuck.mp4 118,442KB) it copied in 256 byte chunks (2.4 hours later!) and worked...

If it was the binary order thing... did you fix it?
Some protocols confirm back to the client on each packet. But I think this is overkill and I'm sure it's accommodated for in the websocket header?

So I can still use my 256 byte binary chunks? From Browser websocket client to Arduino websocket server... I would stream data (still ws payload chunks) but the SD card write speed would be an issue.

I need them small, as each received chunk is written direct to the SD Card.
The Arduino is a balancing act on dynamic memory so I'm doing every trick to save it.
Even the websocket arraybuffer pointer is just passed to SDFat file write, so not duplicating more buffers. For most average files around 16K the websocket server works great. I can use HTTP Post, but it's not ideal.

@daverathbone

Was your concern that websocket binary packets could arrive in any order?

No, the only frames that can interleave text and binary data are ping frames (and it's completely safe)

If it was the binary order thing... did you fix it?

I changed code from strcat to memcpy.

Some protocols confirm back to the client on each packet.

This library doesn't do anything like that. Maybe Ethernet/Wifi does it behind the scenes.