Skip to content

Commit

Permalink
http: change OutgoingMessage so that it inherits from stream.Writable…
Browse files Browse the repository at this point in the history
… 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: nodejs#44188
  • Loading branch information
zeazad-hub committed Dec 8, 2022
1 parent b3bf07e commit 7f40107
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 11 deletions.
27 changes: 16 additions & 11 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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;
Expand Down Expand Up @@ -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', {
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/streams/writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const {
getHighWaterMark,
getDefaultHighWaterMark
} = require('internal/streams/state');

const {
ERR_INVALID_ARG_TYPE,
ERR_METHOD_NOT_IMPLEMENTED,
Expand Down Expand Up @@ -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;
},
Expand Down
13 changes: 13 additions & 0 deletions script.sh
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions test/parallel/test-http-writable-true-after-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-stream-writable-acceptance.js
Original file line number Diff line number Diff line change
@@ -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();

0 comments on commit 7f40107

Please sign in to comment.