[fix] Ensure packet data is an array (#83)

Related: socketio/socket.io#3140

Author: darrachequesne

index.js

@@ -7,6 +7,7 @@ var debug = require('debug')('socket.io-parser');
 var Emitter = require('component-emitter');
 var hasBin = require('has-binary2');
 var binary = require('./binary');
+var isArray = require('isarray');
 var isBuf = require('./is-buffer');
 
 /**
@@ -272,7 +273,9 @@ function decodeString(str) {
     type: Number(str.charAt(0))
   };
 
-  if (null == exports.types[p.type]) return error();
+  if (null == exports.types[p.type]) {
+    return error('unknown packet type ' + p.type);
+  }
 
   // look up attachments if type binary
   if (exports.BINARY_EVENT === p.type || exports.BINARY_ACK === p.type) {
@@ -318,20 +321,25 @@ function decodeString(str) {
 
   // look up json data
   if (str.charAt(++i)) {
-    p = tryParse(p, str.substr(i));
+    var payload = tryParse(str.substr(i));
+    var isPayloadValid = payload !== false && (p.type === exports.ERROR || isArray(payload));
+    if (isPayloadValid) {
+      p.data = payload;
+    } else {
+      return error('invalid payload');
+    }
   }
 
   debug('decoded %s as %j', str, p);
   return p;
 }
 
-function tryParse(p, str) {
+function tryParse(str) {
   try {
-    p.data = JSON.parse(str);
+    return JSON.parse(str);
   } catch(e){
-    return error();
+    return false;
   }
-  return p; 
 }
 
 /**
@@ -392,9 +400,9 @@ BinaryReconstructor.prototype.finishedReconstruction = function() {
   this.buffers = [];
 };
 
-function error() {
+function error(msg) {
   return {
     type: exports.ERROR,
-    data: 'parser error'
+    data: 'parser error: ' + msg
   };
 }

test/arraybuffer.js

@@ -7,7 +7,7 @@ describe('parser', function() {
   it('encodes an ArrayBuffer', function() {
     var packet = {
       type: parser.BINARY_EVENT,
-      data: new ArrayBuffer(2),
+      data: ['a', new ArrayBuffer(2)],
       id: 0,
       nsp: '/'
     };
@@ -17,7 +17,7 @@ describe('parser', function() {
   it('encodes ArrayBuffers deep in JSON', function() {
     var packet = {
       type: parser.BINARY_EVENT,
-      data: {a: 'hi', b: {why: new ArrayBuffer(3)}, c: {a: 'bye', b: { a: new ArrayBuffer(6)}}},
+      data: ['a', {a: 'hi', b: {why: new ArrayBuffer(3)}, c: {a: 'bye', b: { a: new ArrayBuffer(6)}}}],
       id: 999,
       nsp: '/deep'
     };
@@ -27,7 +27,7 @@ describe('parser', function() {
   it('encodes deep binary JSON with null values', function() {
     var packet = {
       type: parser.BINARY_EVENT,
-      data: {a: 'b', c: 4, e: {g: null}, h: new ArrayBuffer(9)},
+      data: ['a', {a: 'b', c: 4, e: {g: null}, h: new ArrayBuffer(9)}],
       nsp: '/',
       id: 600
     };

test/buffer.js

@@ -8,7 +8,7 @@ describe('parser', function() {
   it('encodes a Buffer', function() {
       helpers.test_bin({
         type: parser.BINARY_EVENT,
-        data: new Buffer('abc', 'utf8'),
+        data: ['a', new Buffer('abc', 'utf8')],
         id: 23,
         nsp: '/cool'
       });

test/parser.js

@@ -12,6 +12,8 @@ describe('parser', function(){
     expect(parser.EVENT).to.be.a('number');
     expect(parser.ACK).to.be.a('number');
     expect(parser.ERROR).to.be.a('number');
+    expect(parser.BINARY_EVENT).to.be.a('number');
+    expect(parser.BINARY_ACK).to.be.a('number');
   });
 
   it('encodes connection', function(){
@@ -51,6 +53,14 @@ describe('parser', function(){
     });
   });
 
+  it('encodes an error', function(){
+    helpers.test({
+      type: parser.ERROR,
+      data: 'Unauthorized',
+      nsp: '/'
+    });
+  });
+
   it('decodes a bad binary packet', function(){
     try {
       var decoder = new parser.Decoder();
@@ -59,4 +69,13 @@ describe('parser', function(){
       expect(e.message).to.match(/Illegal/);
     }
   });
+
+  it('returns an error packet on parsing error', function(done){
+    var decoder = new parser.Decoder();
+    decoder.on('decoded', function(packet) {
+      expect(packet).to.eql({ type: 4, data: 'parser error: invalid payload' });
+      done();
+    });
+    decoder.add('442["some","data"');
+  });
 });