jmesnil / stomp-websocket

Stomp client for Web browsers and node.js apps
http://jmesnil.net/stomp-websocket/doc/
Apache License 2.0
1.43k stars 588 forks source link

Reading binary data from message body #83

Open rchunduru opened 9 years ago

rchunduru commented 9 years ago

I have an issue with reading message body which is received in binary data. I have a valid content-length in headers but body.data is showing up empty.

{ command: 'MESSAGE', headers: { timestamp: '1410215555137', 'message-id': 'ID\ccp-la-chs1-ostack2-logger1-41671-1410214441385-0\c10\c0\c0\c4', priority: '4', subscription: 'sub-0', destination: '/topic/LOGGER.SYSLOG', expires: '0', 'content-length': '8888' }, body: '', ack: [Function], nack: [Function] }

This looks like a bug in stompjs in reading binary data.

jmesnil commented 9 years ago

do you have a test to reproduce the issue?

alejandrodau commented 9 years ago

I have a similar issue. This is the received message: {"command":"MESSAGE","headers":{"content-length":"76","content-type":"application/octet-stream","persistent":"true","priority":"0","message-id":"T_sub-0@@session-4Evsm2Fk3tKYnwoH-FRmiQ@@1","destination":"/queue/mmo_0_from","subscription":"sub-0"},"body":"\u0003"}

The first byte of the message is indeed a 0x03, followed by a 0x00. Maybe the library is not copying into body beyond that null byte, in spite of having a valid content-length?

In the debug it shows the following including the whole payload received from the server: MESSAGE\nsubscription:sub-0\ndestination:/queue/mmo_0_from\nmessage-id:T_sub-0@@session-XAT7z67AdtDx-ME8HUgu2w@@1\npriority:0\npersistent:true\ncontent-type:application/octet-stream\ncontent-length:76\n\n\u0003\u0000\u0000\u0000G\u0000\u0000\u0000\u0003s\u0000\u0000\u0000\tzzzzzzzde\u0000\u0000\u0000\u0013xxxxxxx_xxxxxxxxxxSb\u0000\u0000\u0000\u0006reeeet\u0000s\u0000\u0000\u0000\u0004tsse\u0000\u0000\u0000\u0005loxxn\u0000"

thanks

alejandrodau commented 9 years ago

in the Frame.unmarshall function there are regexps that match with Byte.NULL, to separate the data in "frames". The rest of the message body gets assigned to separate frames by that function.

adamish commented 9 years ago

I believe the approach in Frame.unmarshall() is incorrect for binary data.

Currently it takes the ArrayBuffer input and converts the whole thing to a string with a loop calling String.fromCharCode(c) including the binary payload.

Instead what I believe it should do is only convert the header part before the content to a string, i.e. everything before content-length:n. it should then pass the binary payload as a ArrayBuffer back to the subscriber as an ArrayBuffer in body attribute - possibly using ArrayBuffer.prototype.slice()

Without this approach any potential performance benefits from binary messages are lost (which is their main appeal in my opinion) and strings are the wrong type of structure to store binary data when ArrayBuffer could be used.

pantonis commented 8 years ago

Was this fixed?