From d2a920cb69ceaa9cc8c6a116de03b58a9ad242cb Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sun, 3 May 2020 11:02:13 +0530 Subject: [PATCH 1/2] http: don't throw on `Uint8Array`s for `http.ServerResponse#write` Don't throw errors on Uint8Arrays and added test for all valid types. Backport-PR-URL: https://github.com/nodejs/node/pull/33488 PR-URL: https://github.com/nodejs/node/pull/33155 Fixes: https://github.com/nodejs/node/issues/33379 Refs: https://github.com/nodejs/node/issues/29829 Reviewed-By: Robert Nagy Reviewed-By: Anna Henningsen Reviewed-By: Zeyu Yang Reviewed-By: James M Snell --- lib/_http_outgoing.js | 19 ++++++++++++--- test/parallel/test-http-outgoing-proto.js | 18 ++++++++++---- .../test-http-outgoing-write-types.js | 24 +++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-http-outgoing-write-types.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 606c3da9bd75fc..19ea2fe30e6f9d 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -55,11 +55,13 @@ const { ERR_INVALID_CHAR, ERR_METHOD_NOT_IMPLEMENTED, ERR_STREAM_CANNOT_PIPE, + ERR_STREAM_NULL_VALUES, ERR_STREAM_WRITE_AFTER_END }, hideStackFrames } = require('internal/errors'); const { validateString } = require('internal/validators'); +const { isUint8Array } = require('internal/util/types'); const HIGH_WATER_MARK = getDefaultHighWaterMark(); const { CRLF, debug } = common; @@ -625,6 +627,16 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) { }; function write_(msg, chunk, encoding, callback, fromEnd) { + if (typeof callback !== 'function') + callback = function() {}; + + if (chunk === null) + throw new ERR_STREAM_NULL_VALUES(); + + if (typeof chunk !== 'string' && !isUint8Array(chunk)) + throw new ERR_INVALID_ARG_TYPE( + 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk); + if (msg.finished) { const err = new ERR_STREAM_WRITE_AFTER_END(); const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined; @@ -649,9 +661,9 @@ function write_(msg, chunk, encoding, callback, fromEnd) { return true; } - if (!fromEnd && typeof chunk !== 'string' && !(chunk instanceof Buffer)) { + if (!fromEnd && typeof chunk !== 'string' && !isUint8Array(chunk)) { throw new ERR_INVALID_ARG_TYPE('first argument', - ['string', 'Buffer'], chunk); + ['string', 'Buffer', 'Uint8Array'], chunk); } if (!fromEnd && msg.connection && !msg.connection.writableCorked) { @@ -742,7 +754,8 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) { if (chunk) { if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) { - throw new ERR_INVALID_ARG_TYPE('chunk', ['string', 'Buffer'], chunk); + throw new ERR_INVALID_ARG_TYPE( + 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk); } if (!this._header) { if (typeof chunk === 'string') diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index b037c88c6833e9..d658bec166d8c8 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -71,7 +71,15 @@ assert.throws(() => { outgoingMessage.write(''); } -assert(OutgoingMessage.prototype.write.call({ _header: 'test' })); +assert.throws(() => { + const outgoingMessage = new OutgoingMessage(); + outgoingMessage.write.call({ _header: 'test' }); +}, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: 'The "chunk" argument must be of type string or an instance of ' + + 'Buffer or Uint8Array. Received undefined' +}); assert.throws(() => { const outgoingMessage = new OutgoingMessage(); @@ -79,8 +87,8 @@ assert.throws(() => { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The first argument must be of type string or an instance of ' + - 'Buffer. Received undefined' + message: 'The "chunk" argument must be of type string or an instance of ' + + 'Buffer or Uint8Array. Received undefined' }); assert.throws(() => { @@ -89,8 +97,8 @@ assert.throws(() => { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The first argument must be of type string or an instance of ' + - 'Buffer. Received type number (1)' + message: 'The "chunk" argument must be of type string or an instance of ' + + 'Buffer or Uint8Array. Received type number (1)' }); // addTrailers() diff --git a/test/parallel/test-http-outgoing-write-types.js b/test/parallel/test-http-outgoing-write-types.js new file mode 100644 index 00000000000000..6257b87eea8eb1 --- /dev/null +++ b/test/parallel/test-http-outgoing-write-types.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const httpServer = http.createServer(common.mustCall(function(req, res) { + httpServer.close(); + assert.throws(() => { + res.write(['Throws.']); + }, { + code: 'ERR_INVALID_ARG_TYPE' + }); + // should not throw + res.write('1a2b3c'); + // should not throw + res.write(new Uint8Array(1024)); + // should not throw + res.write(Buffer.from('1'.repeat(1024))); + res.end(); +})); + +httpServer.listen(0, common.mustCall(function() { + http.get({ port: this.address().port }); +})); From 7a2037807b9891280e35562fde5f3c3771eadd09 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sun, 31 May 2020 00:59:11 +0530 Subject: [PATCH 2/2] remove semver major changes --- lib/_http_outgoing.js | 11 ----------- test/parallel/test-http-outgoing-proto.js | 14 +++----------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 19ea2fe30e6f9d..442369581d3779 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -55,7 +55,6 @@ const { ERR_INVALID_CHAR, ERR_METHOD_NOT_IMPLEMENTED, ERR_STREAM_CANNOT_PIPE, - ERR_STREAM_NULL_VALUES, ERR_STREAM_WRITE_AFTER_END }, hideStackFrames @@ -627,16 +626,6 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) { }; function write_(msg, chunk, encoding, callback, fromEnd) { - if (typeof callback !== 'function') - callback = function() {}; - - if (chunk === null) - throw new ERR_STREAM_NULL_VALUES(); - - if (typeof chunk !== 'string' && !isUint8Array(chunk)) - throw new ERR_INVALID_ARG_TYPE( - 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk); - if (msg.finished) { const err = new ERR_STREAM_WRITE_AFTER_END(); const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined; diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index d658bec166d8c8..638e860f62b538 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -71,15 +71,7 @@ assert.throws(() => { outgoingMessage.write(''); } -assert.throws(() => { - const outgoingMessage = new OutgoingMessage(); - outgoingMessage.write.call({ _header: 'test' }); -}, { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', - message: 'The "chunk" argument must be of type string or an instance of ' + - 'Buffer or Uint8Array. Received undefined' -}); +assert(OutgoingMessage.prototype.write.call({ _header: 'test' })); assert.throws(() => { const outgoingMessage = new OutgoingMessage(); @@ -87,7 +79,7 @@ assert.throws(() => { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "chunk" argument must be of type string or an instance of ' + + message: 'The first argument must be of type string or an instance of ' + 'Buffer or Uint8Array. Received undefined' }); @@ -97,7 +89,7 @@ assert.throws(() => { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "chunk" argument must be of type string or an instance of ' + + message: 'The first argument must be of type string or an instance of ' + 'Buffer or Uint8Array. Received type number (1)' });