From 7f40107502cf4aedbcdbec4721ee1bbca19526c2 Mon Sep 17 00:00:00 2001 From: zeazad-hub Date: Thu, 8 Dec 2022 03:31:54 -0500 Subject: [PATCH] http: change OutgoingMessage so that it inherits from stream.Writable instead of stream http.OutgoingMessage is supposed to be an instance of stream.Writable, so I set the prototype of OutgoingMessage equal to the Writable prototype and I called the Writable consrtuctor at the end of the constructor for OutgoingMessage. I made sure to override any methods and prpoperties in Writable that were already defined in OutgoingMessage in order to not interfere with the current behavior of OutgoingMessage. Fixes: https://github.com/nodejs/node/issues/44188 --- lib/_http_outgoing.js | 27 ++++++----- lib/internal/streams/writable.js | 2 + script.sh | 13 ++++++ .../test-http-writable-true-after-close.js | 1 + .../test-stream-writable-acceptance.js | 46 +++++++++++++++++++ 5 files changed, 78 insertions(+), 11 deletions(-) create mode 100755 script.sh create mode 100644 test/parallel/test-stream-writable-acceptance.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8c80eabaec9e74..ab55aa41ac0c10 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -102,7 +102,6 @@ function isCookieField(s) { } function OutgoingMessage() { - Stream.call(this); // Queue that holds all currently pending data, until the response will be // assigned to the socket (until it will its turn in the HTTP pipeline). @@ -114,8 +113,8 @@ function OutgoingMessage() { // TCP socket and HTTP Parser and thus handle the backpressure. this.outputSize = 0; - this.writable = true; - this.destroyed = false; + this.writableState = true; + this.destroyedState = false; this._last = false; this.chunkedEncoding = false; @@ -148,24 +147,30 @@ function OutgoingMessage() { this._keepAliveTimeout = 0; this._onPendingData = nop; - - this[kErrored] = null; + Stream.Writable.call(this); } -ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype); -ObjectSetPrototypeOf(OutgoingMessage, Stream); -ObjectDefineProperty(OutgoingMessage.prototype, 'errored', { +ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.Writable.prototype); +ObjectSetPrototypeOf(OutgoingMessage, Stream.Writable); + +ObjectDefineProperty(OutgoingMessage.prototype, 'writable', { __proto__: null, get() { - return this[kErrored]; + return this.writableState; }, + set(val) { + this.writableState = val; + } }); -ObjectDefineProperty(OutgoingMessage.prototype, 'closed', { +ObjectDefineProperty(OutgoingMessage.prototype, 'destroyed', { __proto__: null, get() { - return this._closed; + return this.destroyedState; }, + set(val) { + this.destroyedState = val; + } }); ObjectDefineProperty(OutgoingMessage.prototype, 'writableFinished', { diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index c4d95df9ac7153..f2e99dbea046ae 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -53,6 +53,7 @@ const { getHighWaterMark, getDefaultHighWaterMark } = require('internal/streams/state'); + const { ERR_INVALID_ARG_TYPE, ERR_METHOD_NOT_IMPLEMENTED, @@ -795,6 +796,7 @@ ObjectDefineProperties(Writable.prototype, { // where the writable side was disabled upon construction. // Compat. The user might manually disable writable side through // deprecated setter. + return !!w && w.writable !== false && !w.destroyed && !w.errored && !w.ending && !w.ended; }, diff --git a/script.sh b/script.sh new file mode 100755 index 00000000000000..de1ba6e5e6610b --- /dev/null +++ b/script.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +cd ~/node/test/parallel + +mkdir test-stream-writable-dir + +for file in ./*.js +do + if [[ "$file" =~ .*"test-stream-writable".* ]]; then + cp $file "./test-stream-writable-dir/$file" + echo "{$file} copied" + fi +done \ No newline at end of file diff --git a/test/parallel/test-http-writable-true-after-close.js b/test/parallel/test-http-writable-true-after-close.js index c0db7c34492762..8b1cdfdb9ad901 100644 --- a/test/parallel/test-http-writable-true-after-close.js +++ b/test/parallel/test-http-writable-true-after-close.js @@ -34,6 +34,7 @@ const server = createServer(common.mustCall((req, res) => { })); }).listen(0, () => { external = get(`http://127.0.0.1:${server.address().port}`); + external.on('error', common.mustCall(() => { server.close(); internal.close(); diff --git a/test/parallel/test-stream-writable-acceptance.js b/test/parallel/test-stream-writable-acceptance.js new file mode 100644 index 00000000000000..5acc358f7f9a75 --- /dev/null +++ b/test/parallel/test-stream-writable-acceptance.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const { stream, Writable } = require('stream'); +const { once } = require('events'); + +async function test() { + const o = new http.OutgoingMessage(); + assert.strictEqual(o instanceof http.OutgoingMessage, true); + assert.strictEqual(o instanceof Writable, true); + + let external; + + const server = http.createServer(async (req, nodeStreamResponse) => { + const isOutgoingMessage = nodeStreamResponse instanceof http.OutgoingMessage; + //const isStream = stream.isInstance(nodeStreamResponse); + const isWritable = nodeStreamResponse instanceof Writable; + + assert.strictEqual(nodeStreamResponse.writable, true); + assert.strictEqual(isOutgoingMessage, true); + assert.strictEqual(isWritable, true); + + + const b = typeof nodeStreamResponse?._writableState + + assert.strictEqual(b, 'object') + + const webStreamResponse = Writable.toWeb(nodeStreamResponse); + + setImmediate(common.mustCall(() => { + external.abort(); + nodeStreamResponse.end('Hello World\n'); + })); + }); + + server.listen(0, common.mustCall(() => { + external = http.get(`http://127.0.0.1:${server.address().port}`); + external.on('error', common.mustCall(() => { + server.close(); + })); + })); +} + +test(); \ No newline at end of file