From 1682df15bf4118511c5a1047e9871ee01ddec938 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Thu, 22 Mar 2018 09:49:28 +0100 Subject: [PATCH] Avoid using deprecated Buffer API Usage of `Buffer` constructor has been deprecated due to usability issues that can potentially lead to accidental security issues. Refs: https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor Tracking: https://github.com/nodejs/node/issues/19079 --- lib/websocket/driver/base.js | 7 ++++--- lib/websocket/driver/client.js | 7 ++++--- lib/websocket/driver/draft75.js | 13 +++++++------ lib/websocket/driver/draft76.js | 13 +++++++------ lib/websocket/driver/hybi.js | 13 +++++++------ lib/websocket/driver/hybi/message.js | 6 ++++-- lib/websocket/driver/proxy.js | 7 ++++--- lib/websocket/driver/stream_reader.js | 8 +++++--- lib/websocket/http_parser.js | 5 +++-- package.json | 1 + spec/runner.js | 7 +++---- spec/websocket/driver/client_spec.js | 25 +++++++++++++------------ spec/websocket/driver/draft76_spec.js | 5 +++-- 13 files changed, 65 insertions(+), 52 deletions(-) diff --git a/lib/websocket/driver/base.js b/lib/websocket/driver/base.js index 41ee01e..f05af5f 100644 --- a/lib/websocket/driver/base.js +++ b/lib/websocket/driver/base.js @@ -1,6 +1,7 @@ 'use strict'; -var Emitter = require('events').EventEmitter, +var Buffer = require('safe-buffer').Buffer, + Emitter = require('events').EventEmitter, util = require('util'), streams = require('../streams'), Headers = require('./headers'), @@ -108,10 +109,10 @@ var instance = { _failHandshake: function(error) { var headers = new Headers(); headers.set('Content-Type', 'text/plain'); - headers.set('Content-Length', new Buffer(error.message, 'utf8').length); + headers.set('Content-Length', Buffer.byteLength(error.message, 'utf8')); headers = ['HTTP/1.1 400 Bad Request', headers.toString(), error.message]; - this._write(new Buffer(headers.join('\r\n'), 'utf8')); + this._write(Buffer.from(headers.join('\r\n'), 'utf8')); this._fail('protocol_error', error.message); return false; diff --git a/lib/websocket/driver/client.js b/lib/websocket/driver/client.js index dbc3d01..f24d645 100644 --- a/lib/websocket/driver/client.js +++ b/lib/websocket/driver/client.js @@ -1,6 +1,7 @@ 'use strict'; -var crypto = require('crypto'), +var Buffer = require('safe-buffer').Buffer, + crypto = require('crypto'), url = require('url'), util = require('util'), HttpParser = require('../http_parser'), @@ -18,7 +19,7 @@ var Client = function(_url, options) { this._http = new HttpParser('response'); var uri = url.parse(this.url), - auth = uri.auth && new Buffer(uri.auth, 'utf8').toString('base64'); + auth = uri.auth && Buffer.from(uri.auth, 'utf8').toString('base64'); if (this.VALID_PROTOCOLS.indexOf(uri.protocol) < 0) throw new Error(this.url + ' is not a valid WebSocket URL'); @@ -79,7 +80,7 @@ var instance = { var start = 'GET ' + this._pathname + ' HTTP/1.1', headers = [start, this._headers.toString(), '']; - return new Buffer(headers.join('\r\n'), 'utf8'); + return Buffer.from(headers.join('\r\n'), 'utf8'); }, _failHandshake: function(message) { diff --git a/lib/websocket/driver/draft75.js b/lib/websocket/driver/draft75.js index 6663e55..76c0cad 100644 --- a/lib/websocket/driver/draft75.js +++ b/lib/websocket/driver/draft75.js @@ -1,7 +1,8 @@ 'use strict'; -var Base = require('./base'), - util = require('util'); +var Buffer = require('safe-buffer').Buffer, + Base = require('./base'), + util = require('util'); var Draft75 = function(request, url, options) { Base.apply(this, arguments); @@ -61,7 +62,7 @@ var instance = { case 2: if (octet === 0xFF) { this._stage = 0; - message = new Buffer(this._buffer).toString('utf8', 0, this._buffer.length); + message = Buffer.from(this._buffer).toString('utf8', 0, this._buffer.length); this.emit('message', new Base.MessageEvent(message)); } else { @@ -85,8 +86,8 @@ var instance = { if (typeof buffer !== 'string') buffer = buffer.toString(); - var payload = new Buffer(buffer, 'utf8'), - frame = new Buffer(payload.length + 2); + var payload = Buffer.from(buffer, 'utf8'), + frame = Buffer.allocUnsafe(payload.length + 2); frame[0] = 0x00; frame[payload.length + 1] = 0xFF; @@ -100,7 +101,7 @@ var instance = { var start = 'HTTP/1.1 101 Web Socket Protocol Handshake', headers = [start, this._headers.toString(), '']; - return new Buffer(headers.join('\r\n'), 'utf8'); + return Buffer.from(headers.join('\r\n'), 'utf8'); }, _parseLeadingByte: function(octet) { diff --git a/lib/websocket/driver/draft76.js b/lib/websocket/driver/draft76.js index a6724d4..28eaa5a 100644 --- a/lib/websocket/driver/draft76.js +++ b/lib/websocket/driver/draft76.js @@ -1,6 +1,7 @@ 'use strict'; -var Base = require('./base'), +var Buffer = require('safe-buffer').Buffer, + Base = require('./base'), Draft75 = require('./draft75'), crypto = require('crypto'), util = require('util'); @@ -42,7 +43,7 @@ var instance = { close: function() { if (this.readyState === 3) return false; - if (this.readyState === 1) this._write(new Buffer([0xFF, 0x00])); + if (this.readyState === 1) this._write(Buffer.from([0xFF, 0x00])); this.readyState = 3; this.emit('close', new Base.CloseEvent(null, null)); return true; @@ -70,21 +71,21 @@ var instance = { var start = 'HTTP/1.1 101 WebSocket Protocol Handshake', headers = [start, this._headers.toString(), '']; - return new Buffer(headers.join('\r\n'), 'binary'); + return Buffer.from(headers.join('\r\n'), 'binary'); }, _handshakeSignature: function() { if (this._body.length < this.BODY_SIZE) return null; var md5 = crypto.createHash('md5'), - buffer = new Buffer(8 + this.BODY_SIZE); + buffer = Buffer.allocUnsafe(8 + this.BODY_SIZE); buffer.writeUInt32BE(this._keyValues[0], 0); buffer.writeUInt32BE(this._keyValues[1], 4); - new Buffer(this._body).copy(buffer, 8, 0, this.BODY_SIZE); + Buffer.from(this._body).copy(buffer, 8, 0, this.BODY_SIZE); md5.update(buffer); - return new Buffer(md5.digest('binary'), 'binary'); + return Buffer.from(md5.digest('binary'), 'binary'); }, _sendHandshakeBody: function() { diff --git a/lib/websocket/driver/hybi.js b/lib/websocket/driver/hybi.js index 0bf1875..9027f90 100644 --- a/lib/websocket/driver/hybi.js +++ b/lib/websocket/driver/hybi.js @@ -1,6 +1,7 @@ 'use strict'; -var crypto = require('crypto'), +var Buffer = require('safe-buffer').Buffer, + crypto = require('crypto'), util = require('util'), Extensions = require('websocket-extensions'), Base = require('./base'), @@ -187,7 +188,7 @@ var instance = { if (this.readyState <= 0) return this._queue([buffer, type, code]); if (this.readyState > 2) return false; - if (buffer instanceof Array) buffer = new Buffer(buffer); + if (buffer instanceof Array) buffer = Buffer.from(buffer); if (typeof buffer === 'number') buffer = buffer.toString(); var message = new Message(), @@ -197,11 +198,11 @@ var instance = { message.rsv1 = message.rsv2 = message.rsv3 = false; message.opcode = this.OPCODES[type || (isText ? 'text' : 'binary')]; - payload = isText ? new Buffer(buffer, 'utf8') : buffer; + payload = isText ? Buffer.from(buffer, 'utf8') : buffer; if (code) { copy = payload; - payload = new Buffer(2 + copy.length); + payload = Buffer.allocUnsafe(2 + copy.length); payload.writeUInt16BE(code, 0); copy.copy(payload, 2); } @@ -239,7 +240,7 @@ var instance = { var length = frame.length, header = (length <= 125) ? 2 : (length <= 65535 ? 4 : 10), offset = header + (frame.masked ? 4 : 0), - buffer = new Buffer(offset + length), + buffer = Buffer.allocUnsafe(offset + length), masked = frame.masked ? this.MASK : 0; buffer[0] = (frame.final ? this.FIN : 0) | @@ -291,7 +292,7 @@ var instance = { var start = 'HTTP/1.1 101 Switching Protocols', headers = [start, this._headers.toString(), '']; - return new Buffer(headers.join('\r\n'), 'utf8'); + return Buffer.from(headers.join('\r\n'), 'utf8'); }, _shutdown: function(code, reason, error) { diff --git a/lib/websocket/driver/hybi/message.js b/lib/websocket/driver/hybi/message.js index 3db1a3c..0252eda 100644 --- a/lib/websocket/driver/hybi/message.js +++ b/lib/websocket/driver/hybi/message.js @@ -1,10 +1,12 @@ 'use strict'; +var Buffer = require('safe-buffer').Buffer; + var Message = function() { this.rsv1 = false; this.rsv2 = false; this.rsv3 = false; - this.opcode = null + this.opcode = null; this.length = 0; this._chunks = []; }; @@ -13,7 +15,7 @@ var instance = { read: function() { if (this.data) return this.data; - this.data = new Buffer(this.length); + this.data = Buffer.allocUnsafe(this.length); var offset = 0; for (var i = 0, n = this._chunks.length; i < n; i++) { diff --git a/lib/websocket/driver/proxy.js b/lib/websocket/driver/proxy.js index e7362ea..227493a 100644 --- a/lib/websocket/driver/proxy.js +++ b/lib/websocket/driver/proxy.js @@ -1,6 +1,7 @@ 'use strict'; -var Stream = require('stream').Stream, +var Buffer = require('safe-buffer').Buffer, + Stream = require('stream').Stream, url = require('url'), util = require('util'), Base = require('./base'), @@ -25,7 +26,7 @@ var Proxy = function(client, origin, options) { this._headers.set('Connection', 'keep-alive'); this._headers.set('Proxy-Connection', 'keep-alive'); - var auth = this._url.auth && new Buffer(this._url.auth, 'utf8').toString('base64'); + var auth = this._url.auth && Buffer.from(this._url.auth, 'utf8').toString('base64'); if (auth) this._headers.set('Proxy-Authorization', 'Basic ' + auth); }; util.inherits(Proxy, Stream); @@ -47,7 +48,7 @@ var instance = { var headers = [start, this._headers.toString(), '']; - this.emit('data', new Buffer(headers.join('\r\n'), 'utf8')); + this.emit('data', Buffer.from(headers.join('\r\n'), 'utf8')); return true; }, diff --git a/lib/websocket/driver/stream_reader.js b/lib/websocket/driver/stream_reader.js index 04a15db..f4b5648 100644 --- a/lib/websocket/driver/stream_reader.js +++ b/lib/websocket/driver/stream_reader.js @@ -1,5 +1,7 @@ 'use strict'; +var Buffer = require('safe-buffer').Buffer; + var StreamReader = function() { this._queue = []; this._queueSize = 0; @@ -8,14 +10,14 @@ var StreamReader = function() { StreamReader.prototype.put = function(buffer) { if (!buffer || buffer.length === 0) return; - if (!buffer.copy) buffer = new Buffer(buffer); + if (!buffer.copy) buffer = Buffer.from(buffer); this._queue.push(buffer); this._queueSize += buffer.length; }; StreamReader.prototype.read = function(length) { if (length > this._queueSize) return null; - if (length === 0) return new Buffer(0); + if (length === 0) return Buffer.alloc(0); this._queueSize -= length; @@ -67,7 +69,7 @@ StreamReader.prototype.eachByte = function(callback, context) { StreamReader.prototype._concat = function(buffers, length) { if (Buffer.concat) return Buffer.concat(buffers, length); - var buffer = new Buffer(length), + var buffer = Buffer.allocUnsafe(length), offset = 0; for (var i = 0, n = buffers.length; i < n; i++) { diff --git a/lib/websocket/http_parser.js b/lib/websocket/http_parser.js index 665d8c5..184e8bc 100644 --- a/lib/websocket/http_parser.js +++ b/lib/websocket/http_parser.js @@ -1,6 +1,7 @@ 'use strict'; -var NodeHTTPParser = require('http-parser-js').HTTPParser; +var NodeHTTPParser = require('http-parser-js').HTTPParser, + Buffer = require('safe-buffer').Buffer; var VERSION = process.version.match(/[0-9]+/g).map(function(n) { return parseInt(n, 10) }); @@ -128,7 +129,7 @@ HttpParser.prototype.parse = function(chunk) { if (this._complete) this.body = (consumed < chunk.length) ? chunk.slice(consumed) - : new Buffer(0); + : Buffer.alloc(0); }; module.exports = HttpParser; diff --git a/package.json b/package.json index 560acd0..7a74775 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ , "main" : "./lib/websocket/driver" , "dependencies" : { "http-parser-js": ">=0.4.0" + , "safe-buffer": ">=5.1.1" , "websocket-extensions": ">=0.1.1" } , "devDependencies" : { "jstest": "*" diff --git a/spec/runner.js b/spec/runner.js index d6cebaa..2486de2 100644 --- a/spec/runner.js +++ b/spec/runner.js @@ -1,11 +1,10 @@ -var test = require('jstest').Test, +var Buffer = require('safe-buffer').Buffer, + test = require('jstest').Test, Stream = require('stream').Stream, util = require('util') var BufferMatcher = function(data) { - this._data = (typeof data === 'string') - ? new Buffer(data, 'utf8') - : new Buffer(data) + this._data = Buffer.from(data) } BufferMatcher.prototype.equals = function(other) { if (this._data.length !== other.length) return false; diff --git a/spec/websocket/driver/client_spec.js b/spec/websocket/driver/client_spec.js index 4c8e5da..ca02c25 100644 --- a/spec/websocket/driver/client_spec.js +++ b/spec/websocket/driver/client_spec.js @@ -1,4 +1,5 @@ var Client = require("../../../lib/websocket/driver/client"), + Buffer = require('safe-buffer').Buffer, test = require('jstest').Test test.describe("Client", function() { with(this) { @@ -190,21 +191,21 @@ test.describe("Client", function() { with(this) { it("returns true when the response is written", function() { with(this) { // this prevents downstream connections suddenly closing for no reason - assertEqual( true, proxy.write(new Buffer("HTTP/1.1 200 OK\r\n\r\n")) ) + assertEqual( true, proxy.write(Buffer.from("HTTP/1.1 200 OK\r\n\r\n")) ) }}) it("emits a 'connect' event when the proxy connects", function() { with(this) { expect(proxy, "emit").given("connect", anything()) expect(proxy, "emit").given("close") expect(proxy, "emit").given("end") - proxy.write(new Buffer("HTTP/1.1 200 OK\r\n\r\n")) + proxy.write(Buffer.from("HTTP/1.1 200 OK\r\n\r\n")) }}) it("emits an 'error' event if the proxy does not connect", function() { with(this) { expect(proxy, "emit").given("error", objectIncluding({message: "Can't establish a connection to the server at ws://www.example.com/socket"})) expect(proxy, "emit").given("close") expect(proxy, "emit").given("end") - proxy.write(new Buffer("HTTP/1.1 403 Forbidden\r\n\r\n")) + proxy.write(Buffer.from("HTTP/1.1 403 Forbidden\r\n\r\n")) }}) }}) }}) @@ -213,7 +214,7 @@ test.describe("Client", function() { with(this) { before(function() { this.driver().start() }) describe("with a valid response", function() { with(this) { - before(function() { this.driver().parse(new Buffer(this.response())) }) + before(function() { this.driver().parse(Buffer.from(this.response())) }) it("changes the state to open", function() { with(this) { assertEqual( true, open ) @@ -232,9 +233,9 @@ test.describe("Client", function() { with(this) { describe("with a valid response followed by a frame", function() { with(this) { before(function() { with(this) { - var resp = new Buffer(response().length + 4) - new Buffer(response()).copy(resp) - new Buffer([0x81, 0x02, 72, 105]).copy(resp, resp.length - 4) + var resp = Buffer.alloc(response().length + 4) + Buffer.from(response()).copy(resp) + Buffer.from([0x81, 0x02, 72, 105]).copy(resp, resp.length - 4) driver().parse(resp) }}) @@ -252,7 +253,7 @@ test.describe("Client", function() { with(this) { describe("with a bad status line", function() { with(this) { before(function() { var resp = this.response().replace(/101/g, "4") - this.driver().parse(new Buffer(resp)) + this.driver().parse(Buffer.from(resp)) }) it("changes the state to closed", function() { with(this) { @@ -266,7 +267,7 @@ test.describe("Client", function() { with(this) { describe("with a bad Upgrade header", function() { with(this) { before(function() { var resp = this.response().replace(/websocket/g, "wrong") - this.driver().parse(new Buffer(resp)) + this.driver().parse(Buffer.from(resp)) }) it("changes the state to closed", function() { with(this) { @@ -280,7 +281,7 @@ test.describe("Client", function() { with(this) { describe("with a bad Accept header", function() { with(this) { before(function() { var resp = this.response().replace(/QV3/g, "wrong") - this.driver().parse(new Buffer(resp)) + this.driver().parse(Buffer.from(resp)) }) it("changes the state to closed", function() { with(this) { @@ -296,7 +297,7 @@ test.describe("Client", function() { with(this) { before(function() { var resp = this.response().replace(/\r\n\r\n/, "\r\nSec-WebSocket-Protocol: xmpp\r\n\r\n") - this.driver().parse(new Buffer(resp)) + this.driver().parse(Buffer.from(resp)) }) it("changs the state to open", function() { with(this) { @@ -315,7 +316,7 @@ test.describe("Client", function() { with(this) { before(function() { var resp = this.response().replace(/\r\n\r\n/, "\r\nSec-WebSocket-Protocol: irc\r\n\r\n") - this.driver().parse(new Buffer(resp)) + this.driver().parse(Buffer.from(resp)) }) it("changs the state to closed", function() { with(this) { diff --git a/spec/websocket/driver/draft76_spec.js b/spec/websocket/driver/draft76_spec.js index 3dc2ac5..2cc06e4 100644 --- a/spec/websocket/driver/draft76_spec.js +++ b/spec/websocket/driver/draft76_spec.js @@ -1,8 +1,9 @@ var Draft76 = require("../../../lib/websocket/driver/draft76"), + Buffer = require('safe-buffer').Buffer, test = require('jstest').Test test.describe("Draft76", function() { with(this) { - BODY = new Buffer([0x91, 0x25, 0x3e, 0xd3, 0xa9, 0xe7, 0x6a, 0x88]) + BODY = Buffer.from([0x91, 0x25, 0x3e, 0xd3, 0xa9, 0xe7, 0x6a, 0x88]) define("body", function() { return BODY @@ -149,7 +150,7 @@ test.describe("Draft76", function() { with(this) { describe("with no request body", function() { with(this) { define("body", function() { - return new Buffer([]) + return Buffer.alloc(0) }) it("writes the handshake response with no body", function() { with(this) {