Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Store data to buffers in default WebSocket protocol #728

Closed
wants to merge 3 commits into from

4 participants

@lautis

Properly detect 0xFF as end-of-message marker. This makes sure that partial UTF-8 characters (signle character is splitted in multiple data chunks) or code points outside BMP do not cause connection breakages.

This is partial fix for #699. Emojis don't cause disconnection, but characters outside Basic Multilingual Plane are not decoded properly.

@lautis lautis Store data to buffers in default WebSocket protocol
Properly detect 0xFF as end-of-message marker.
This makes sure that partial UTF-8 characters or
code points outside BMP do not cause connection
breakages (although they are still not decoded
correctly).
54de4af
@einaros

Would you mind adding a test case which captures the original issue?

@lautis

I didn't look as thoroughly as I should've for how to write tests. Added test cases for parsing single character splitted in multiple data chunks and non-BMP characters inside single buffer.

@einaros

Great, that looks much better. Thank you!

@baudehlo

Ah I just submitted a different work-around to this bug. This looks like it is probably a better fix.

@rauchg
Owner

Switching to ws for this.

@rauchg rauchg closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 25, 2012
  1. @lautis

    Store data to buffers in default WebSocket protocol

    lautis authored
    Properly detect 0xFF as end-of-message marker.
    This makes sure that partial UTF-8 characters or
    code points outside BMP do not cause connection
    breakages (although they are still not decoded
    correctly).
  2. @lautis
Commits on Feb 10, 2012
  1. @lautis
This page is out of date. Refresh to see the latest.
View
35 lib/transports/websocket/default.js
@@ -19,6 +19,7 @@ var Transport = require('../../transport')
*/
exports = module.exports = WebSocket;
+exports.Parser = Parser;
/**
* HTTP interface constructor. Interface compatible with all transports that
@@ -127,30 +128,25 @@ WebSocket.prototype.onSocketConnect = function () {
this.socket.write(headers.concat('', '').join('\r\n'));
this.socket.setTimeout(0);
this.socket.setNoDelay(true);
- this.socket.setEncoding('utf8');
} catch (e) {
this.end();
return;
}
- if (waitingForNonce) {
- this.socket.setEncoding('binary');
- } else if (this.proveReception(headers)) {
+ if (!waitingForNonce && this.proveReception(headers)) {
self.flush();
}
var headBuffer = '';
- this.socket.on('data', function (data) {
+ this.socket.on('data', function (buffer) {
if (waitingForNonce) {
- headBuffer += data;
+ headBuffer += buffer.toString('binary');
if (headBuffer.length < 8) {
return;
}
- // Restore the connection to utf8 encoding after receiving the nonce
- self.socket.setEncoding('utf8');
waitingForNonce = false;
// Stuff the nonce into the location where it's expected to be
@@ -164,7 +160,7 @@ WebSocket.prototype.onSocketConnect = function () {
return;
}
- self.parser.add(data);
+ self.parser.add(buffer);
});
};
@@ -292,7 +288,7 @@ WebSocket.prototype.doClose = function () {
*/
function Parser () {
- this.buffer = '';
+ this.buffer = new Buffer(0);
this.i = 0;
};
@@ -309,7 +305,10 @@ Parser.prototype.__proto__ = EventEmitter.prototype;
*/
Parser.prototype.add = function (data) {
- this.buffer += data;
+ var buffer = new Buffer(this.buffer.length + data.length);
+ this.buffer.copy(buffer);
+ data.copy(buffer, this.buffer.length);
+ this.buffer = buffer;
this.parse();
};
@@ -323,23 +322,23 @@ Parser.prototype.parse = function () {
for (var i = this.i, chr, l = this.buffer.length; i < l; i++){
chr = this.buffer[i];
- if (this.buffer.length == 2 && this.buffer[1] == '\u0000') {
+ if (this.buffer.length == 2 && this.buffer[1] == 0x00) {
this.emit('close');
- this.buffer = '';
+ this.buffer = new Buffer(0);
this.i = 0;
return;
}
if (i === 0){
- if (chr != '\u0000')
+ if (chr != 0x00)
this.error('Bad framing. Expected null byte as first frame');
else
continue;
}
- if (chr == '\ufffd'){
- this.emit('data', this.buffer.substr(1, i - 1));
- this.buffer = this.buffer.substr(i + 1);
+ if (chr == 0xFF){
+ this.emit('data', this.buffer.slice(1, i).toString('utf8'));
+ this.buffer = this.buffer.slice(i + 1);
this.i = 0;
return this.parse();
}
@@ -353,7 +352,7 @@ Parser.prototype.parse = function () {
*/
Parser.prototype.error = function (reason) {
- this.buffer = '';
+ this.buffer = new Buffer(0);
this.i = 0;
this.emit('error', reason);
return this;
View
71 test/transports.websocket.default.parser.test.js
@@ -0,0 +1,71 @@
+/**
+ * Test dependencies.
+ */
+
+var assert = require('assert');
+var Parser = require('../lib/transports/websocket/default.js').Parser;
+require('./hybi-common');
+
+/**
+ * Tests.
+ */
+
+module.exports = {
+ 'can parse message': function() {
+ var p = new Parser();
+ var packet = '00 48 65 6c 6c 6f ff';
+
+ var gotData = false;
+ p.on('data', function(data) {
+ gotData = true;
+ assert.equal('Hello', data);
+ });
+
+ p.add(getBufferFromHexString(packet));
+ assert.ok(gotData);
+ },
+ 'can parse message from multiple chunks': function() {
+ var p = new Parser();
+ var packet1 = '00 48 65';
+ var packet2 = '6c 6c 6f ff';
+
+ var gotData = false;
+ p.on('data', function(data) {
+ gotData = true;
+ assert.equal('Hello', data);
+ });
+
+ p.add(getBufferFromHexString(packet1));
+ p.add(getBufferFromHexString(packet2));
+ assert.ok(gotData);
+ },
+ 'can parse multibyte UTF-8 from multiple chunks': function() {
+ var p = new Parser();
+ var packet1 = '00 c3';
+ var packet2 = 'b6 ff';
+
+ var gotData = false;
+ p.on('data', function(data) {
+ gotData = true;
+ assert.equal('ö', data);
+ });
+
+ p.add(getBufferFromHexString(packet1));
+ p.add(getBufferFromHexString(packet2));
+ assert.ok(gotData);
+ },
+ 'can parse message containing 4 byte UTF-8': function() {
+ var p = new Parser();
+ var packet = '00 f0 9d 9b a2 ff';
+
+ var gotData = false;
+ p.on('data', function(data) {
+ gotData = true;
+ assert.equal(1, data.length); // Parsed as replacement character
+ });
+
+ p.add(getBufferFromHexString(packet));
+ assert.ok(gotData);
+ },
+};
+
Something went wrong with that request. Please try again.