From 4732f15e30be9e0479619b02c2529386f6d791bd Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sun, 3 May 2020 11:02:13 +0530 Subject: [PATCH 1/3] 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/33490 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 | 16 +++++++++++-- test/parallel/test-http-outgoing-proto.js | 18 ++++++++++---- .../test-http-outgoing-write-types.js | 24 +++++++++++++++++++ 3 files changed, 51 insertions(+), 7 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 d9decf22a018a9..6ab2ee3f616304 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -56,11 +56,13 @@ const { ERR_METHOD_NOT_IMPLEMENTED, ERR_STREAM_CANNOT_PIPE, ERR_STREAM_ALREADY_FINISHED, + 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; @@ -651,6 +653,16 @@ function writeAfterEnd(msg, 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) { writeAfterEnd(msg, callback); return true; @@ -667,9 +679,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.socket && !msg.socket.writableCorked) { diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index 4a07d18c601c5c..39c53ba31a7559 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -74,7 +74,15 @@ assert.throws(() => { ); } -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(); @@ -82,8 +90,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(() => { @@ -92,8 +100,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 d7494432d690c52a84bdac8cf0b819f904139904 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sat, 30 May 2020 22:23:44 +0530 Subject: [PATCH 2/3] remove semver major changes --- lib/_http_outgoing.js | 11 ----------- test/parallel/test-http-outgoing-proto.js | 18 +++++------------- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 6ab2ee3f616304..a0fd5886a9daa6 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -56,7 +56,6 @@ const { ERR_METHOD_NOT_IMPLEMENTED, ERR_STREAM_CANNOT_PIPE, ERR_STREAM_ALREADY_FINISHED, - ERR_STREAM_NULL_VALUES, ERR_STREAM_WRITE_AFTER_END }, hideStackFrames @@ -653,16 +652,6 @@ function writeAfterEnd(msg, 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) { writeAfterEnd(msg, callback); return true; diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index 39c53ba31a7559..4a07d18c601c5c 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -74,15 +74,7 @@ assert.throws(() => { ); } -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(); @@ -90,8 +82,8 @@ assert.throws(() => { }, { 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' + message: 'The first argument must be of type string or an instance of ' + + 'Buffer. Received undefined' }); assert.throws(() => { @@ -100,8 +92,8 @@ assert.throws(() => { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "chunk" argument must be of type string or an instance of ' + - 'Buffer or Uint8Array. Received type number (1)' + message: 'The first argument must be of type string or an instance of ' + + 'Buffer. Received type number (1)' }); // addTrailers() From e07ef0b343564bc5956b3709bb6996c6b4d522c8 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Sun, 31 May 2020 00:24:16 +0530 Subject: [PATCH 3/3] fixup --- test/parallel/test-http-outgoing-proto.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js index 4a07d18c601c5c..6968b7b0450168 100644 --- a/test/parallel/test-http-outgoing-proto.js +++ b/test/parallel/test-http-outgoing-proto.js @@ -83,7 +83,7 @@ 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' + 'Buffer or Uint8Array. Received undefined' }); assert.throws(() => { @@ -93,7 +93,7 @@ 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)' + 'Buffer or Uint8Array. Received type number (1)' }); // addTrailers()