From 74f37f35d5c5f22202964e74cda1d90689fa67ca Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Wed, 12 Dec 2018 23:34:49 -0500 Subject: [PATCH 1/4] Fix suppression of fatal protocol server errors fixes #2021 --- Changes.md | 1 + lib/Connection.js | 6 ++- lib/protocol/PacketWriter.js | 2 +- lib/protocol/Parser.js | 48 ++++++++++++------- lib/protocol/sequences/Sequence.js | 5 ++ .../connection/test-exceed-max-packet-size.js | 31 ++++++++++++ 6 files changed, 75 insertions(+), 18 deletions(-) create mode 100644 test/integration/connection/test-exceed-max-packet-size.js diff --git a/Changes.md b/Changes.md index 73e549c8d..034036bc4 100644 --- a/Changes.md +++ b/Changes.md @@ -31,6 +31,7 @@ you spot any mistakes. * Fix `connection.threadId` missing on handshake failure * Fix duplicate packet name in debug output * Fix no password support for old password protocol +* Fix suppression of fatal protocol server errors #2021 * Remove special case for handshake in determine packet code * Small performance improvement starting command sequence * Support auth switch in change user flow #1776 diff --git a/lib/Connection.js b/lib/Connection.js index 6802255dd..cabb23d96 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -415,7 +415,11 @@ Connection.prototype._handleConnectTimeout = function() { }; Connection.prototype._handleNetworkError = function(err) { - this._protocol.handleNetworkError(err); + if (err.code === 'EPIPE' && err.syscall === 'write') { + setTimeout(this._protocol.handleNetworkError.bind(this._protocol, err), 10); + } else { + this._protocol.handleNetworkError(err); + } }; Connection.prototype._handleProtocolError = function(err) { diff --git a/lib/protocol/PacketWriter.js b/lib/protocol/PacketWriter.js index 4d0afd2af..232d0b244 100644 --- a/lib/protocol/PacketWriter.js +++ b/lib/protocol/PacketWriter.js @@ -32,7 +32,7 @@ PacketWriter.prototype.toBuffer = function toBuffer(parser) { ? length % MAX_PACKET_LENGTH : MAX_PACKET_LENGTH; - var packetNumber = parser.incrementPacketNumber(); + var packetNumber = parser.incrementPacketNumber(packet > 0); this.writeUnsignedNumber(3, packetLength); this.writeUnsignedNumber(1, packetNumber); diff --git a/lib/protocol/Parser.js b/lib/protocol/Parser.js index e72555f2e..518bbfa30 100644 --- a/lib/protocol/Parser.js +++ b/lib/protocol/Parser.js @@ -11,19 +11,20 @@ module.exports = Parser; function Parser(options) { options = options || {}; - this._supportBigNumbers = options.config && options.config.supportBigNumbers; - this._buffer = Buffer.alloc(0); - this._nextBuffers = new BufferList(); - this._longPacketBuffers = new BufferList(); - this._offset = 0; - this._packetEnd = null; - this._packetHeader = null; - this._packetOffset = null; - this._onError = options.onError || function(err) { throw err; }; - this._onPacket = options.onPacket || function() {}; - this._nextPacketNumber = 0; - this._encoding = 'utf-8'; - this._paused = false; + this._supportBigNumbers = options.config && options.config.supportBigNumbers; + this._buffer = Buffer.alloc(0); + this._nextBuffers = new BufferList(); + this._expectedPacketNumber = 0; + this._longPacketBuffers = new BufferList(); + this._offset = 0; + this._packetEnd = null; + this._packetHeader = null; + this._packetOffset = null; + this._onError = options.onError || function(err) { throw err; }; + this._onPacket = options.onPacket || function() {}; + this._nextPacketNumber = 0; + this._encoding = 'utf-8'; + this._paused = false; } Parser.prototype.write = function write(chunk) { @@ -103,6 +104,16 @@ Parser.prototype.append = function append(chunk) { : null; }; +Parser.prototype.isExpectedPacketNumber = function isExpectedPacketNumber(num) { + if (this._expectedPacketNumber === this._nextPacketNumber) { + return this._expectedPacketNumber === num; + } else if (this._expectedPacketNumber < this._nextPacketNumber) { + return this._expectedPacketNumber <= num && this._nextPacketNumber >= num; + } else { + return this._expectedPacketNumber <= num || this._nextPacketNumber >= num; + } +}; + Parser.prototype.pause = function() { this._paused = true; }; @@ -345,15 +356,20 @@ Parser.prototype.reachedPacketEnd = function() { return this._offset === this._packetEnd; }; -Parser.prototype.incrementPacketNumber = function() { +Parser.prototype.incrementPacketNumber = function incrementPacketNumber(isContinuation) { var currentPacketNumber = this._nextPacketNumber; this._nextPacketNumber = (this._nextPacketNumber + 1) % 256; + if (!isContinuation) { + this._expectedPacketNumber = this._nextPacketNumber; + } + return currentPacketNumber; }; Parser.prototype.resetPacketNumber = function() { - this._nextPacketNumber = 0; + this._expectedPacketNumber = 0; + this._nextPacketNumber = 0; }; Parser.prototype.packetLength = function packetLength() { @@ -466,7 +482,7 @@ Parser.prototype._tryReadPacketHeader = function _tryReadPacketHeader() { this.parseUnsignedNumber(1) ); - if (this._packetHeader.number !== this._nextPacketNumber) { + if (!this.isExpectedPacketNumber(this._packetHeader.number)) { var err = new Error( 'Packets out of order. Got: ' + this._packetHeader.number + ' ' + 'Expected: ' + this._nextPacketNumber diff --git a/lib/protocol/sequences/Sequence.js b/lib/protocol/sequences/Sequence.js index de82dc270..c44a3b6ce 100644 --- a/lib/protocol/sequences/Sequence.js +++ b/lib/protocol/sequences/Sequence.js @@ -51,6 +51,11 @@ Sequence.prototype._packetToError = function(packet) { err.sqlMessage = packet.message; err.sqlState = packet.sqlState; + // fatal sql state + if (packet.sqlState === '08S01') { + err.fatal = true; + } + return err; }; diff --git a/test/integration/connection/test-exceed-max-packet-size.js b/test/integration/connection/test-exceed-max-packet-size.js new file mode 100644 index 000000000..d901a04cf --- /dev/null +++ b/test/integration/connection/test-exceed-max-packet-size.js @@ -0,0 +1,31 @@ +var assert = require('assert'); +var common = require('../../common'); +var crypto = require('crypto'); + +if (process.platform === 'win32') { + common.skipTest('windows sockets close immediately'); +} + +common.getTestConnection(function (err, conn) { + assert.ifError(err); + + conn.query('SHOW VARIABLES WHERE Variable_name = ?', ['max_allowed_packet'], function (err, rows) { + assert.ifError(err); + assert.strictEqual(rows.length, 1); + + var maxAllowedPacket = Number(rows[0].Value); + assert.ok(maxAllowedPacket > 0); + + var length = maxAllowedPacket; + var random = crypto.pseudoRandomBytes || crypto.randomBytes; // Depends on node.js version + + random(length, function (err, buf) { + assert.ifError(err); + conn.query('SELECT ?', [buf], function (err) { + assert.ok(err); + assert.strictEqual(err.code, 'ER_NET_PACKET_TOO_LARGE'); + assert.strictEqual(err.fatal, true); + }); + }); + }); +}); From 4403da34a2eabe742084f103ea6d720cc78c2ee0 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Fri, 18 Jan 2019 22:31:34 -0500 Subject: [PATCH 2/4] fixup! Fix suppression of fatal protocol server errors --- lib/Connection.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/Connection.js b/lib/Connection.js index cabb23d96..be0c8a47d 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -15,6 +15,7 @@ function Connection(options) { this.config = options.config; + this._buffer = []; this._socket = options.socket; this._protocol = new Protocol({config: this.config, connection: this}); this._connectCalled = false; @@ -81,8 +82,16 @@ Connection.prototype.connect = function connect(options, callback) { } var connection = this; - this._protocol.on('data', function(data) { - connection._socket.write(data); + var onWrite = function onWrite() { + if (connection._buffer.shift() && connection._buffer.length > 0) { + connection._socket.write(connection._buffer[0], onWrite); + } + }; + + this._protocol.on('data', function (data) { + if (connection._buffer.push(data) === 1) { + connection._socket.write(data, onWrite); + } }); this._socket.on('data', wrapToDomain(connection, function (data) { connection._protocol.write(data); @@ -415,11 +424,7 @@ Connection.prototype._handleConnectTimeout = function() { }; Connection.prototype._handleNetworkError = function(err) { - if (err.code === 'EPIPE' && err.syscall === 'write') { - setTimeout(this._protocol.handleNetworkError.bind(this._protocol, err), 10); - } else { - this._protocol.handleNetworkError(err); - } + this._protocol.handleNetworkError(err); }; Connection.prototype._handleProtocolError = function(err) { From a0054cce743df47d69eee1ea16800b9473eb9e48 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Fri, 18 Jan 2019 22:44:48 -0500 Subject: [PATCH 3/4] fixup! Fix suppression of fatal protocol server errors --- lib/protocol/PacketWriter.js | 9 ++++++++- lib/protocol/Protocol.js | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/protocol/PacketWriter.js b/lib/protocol/PacketWriter.js index 232d0b244..687a83d01 100644 --- a/lib/protocol/PacketWriter.js +++ b/lib/protocol/PacketWriter.js @@ -5,13 +5,17 @@ var BUFFER_ALLOC_SIZE = Math.pow(2, 8); // Don't panic: Good enough to represent byte values up to 8192 TB var IEEE_754_BINARY_64_PRECISION = Math.pow(2, 53); var MAX_PACKET_LENGTH = Math.pow(2, 24) - 1; -var Buffer = require('safe-buffer').Buffer; + +var Buffer = require('safe-buffer').Buffer; +var EventEmitter = require('events').EventEmitter; +var Util = require('util'); module.exports = PacketWriter; function PacketWriter() { this._buffer = null; this._offset = 0; } +Util.inherits(PacketWriter, EventEmitter); PacketWriter.prototype.toBuffer = function toBuffer(parser) { if (!this._buffer) { @@ -33,6 +37,7 @@ PacketWriter.prototype.toBuffer = function toBuffer(parser) { : MAX_PACKET_LENGTH; var packetNumber = parser.incrementPacketNumber(packet > 0); + var packetOffset = this._offset; this.writeUnsignedNumber(3, packetLength); this.writeUnsignedNumber(1, packetNumber); @@ -41,6 +46,8 @@ PacketWriter.prototype.toBuffer = function toBuffer(parser) { var end = start + packetLength; this.writeBuffer(buffer.slice(start, end)); + + this.emit('data', this._buffer.slice(packetOffset, this._offset)); } return this._buffer; diff --git a/lib/protocol/Protocol.js b/lib/protocol/Protocol.js index ab371059b..e0bdb9d53 100644 --- a/lib/protocol/Protocol.js +++ b/lib/protocol/Protocol.js @@ -301,8 +301,14 @@ Protocol.prototype._parsePacketDebug = function _parsePacketDebug(packet) { Protocol.prototype._emitPacket = function(packet) { var packetWriter = new PacketWriter(); + var protocol = this; + + packetWriter.on('data', function (data) { + protocol.emit('data', data); + }); + packet.write(packetWriter); - this.emit('data', packetWriter.toBuffer(this._parser)); + packetWriter.toBuffer(this._parser); if (this._config.debug) { this._debugPacket(false, packet); From eb7f3a263a7e4c491f756cdac9b93bb3908938fe Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Fri, 18 Jan 2019 23:36:19 -0500 Subject: [PATCH 4/4] fixup! Fix suppression of fatal protocol server errors --- lib/Connection.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Connection.js b/lib/Connection.js index be0c8a47d..c5a3dfcc1 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -84,7 +84,9 @@ Connection.prototype.connect = function connect(options, callback) { var connection = this; var onWrite = function onWrite() { if (connection._buffer.shift() && connection._buffer.length > 0) { - connection._socket.write(connection._buffer[0], onWrite); + setTimeout(function () { + connection._socket.write(connection._buffer[0], onWrite); + }, 10); } };