Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d005f49

Browse filesBrowse files
ronagronag
committed
http: cleanup end argument handling
PR-URL: #31818 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent e50ae7a commit d005f49
Copy full SHA for d005f49

File tree

Expand file treeCollapse file tree

5 files changed

+86
-51
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+86
-51
lines changed
Open diff view settings
Collapse file

‎lib/_http_outgoing.js‎

Copy file name to clipboardExpand all lines: lib/_http_outgoing.js
+46-39Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ const {
5656
ERR_METHOD_NOT_IMPLEMENTED,
5757
ERR_STREAM_CANNOT_PIPE,
5858
ERR_STREAM_ALREADY_FINISHED,
59-
ERR_STREAM_WRITE_AFTER_END
59+
ERR_STREAM_WRITE_AFTER_END,
60+
ERR_STREAM_NULL_VALUES,
61+
ERR_STREAM_DESTROYED
6062
},
6163
hideStackFrames
6264
} = require('internal/errors');
@@ -67,6 +69,8 @@ const { CRLF, debug } = common;
6769

6870
const kCorked = Symbol('corked');
6971

72+
function nop() {}
73+
7074
const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
7175
const RE_TE_CHUNKED = common.chunkExpression;
7276

@@ -633,58 +637,81 @@ ObjectDefineProperty(OutgoingMessage.prototype, 'writableEnded', {
633637

634638
const crlf_buf = Buffer.from('\r\n');
635639
OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
640+
if (typeof encoding === 'function') {
641+
callback = encoding;
642+
encoding = null;
643+
}
644+
636645
const ret = write_(this, chunk, encoding, callback, false);
637646
if (!ret)
638647
this[kNeedDrain] = true;
639648
return ret;
640649
};
641650

642-
function writeAfterEnd(msg, callback) {
643-
const err = new ERR_STREAM_WRITE_AFTER_END();
651+
function onError(msg, err, callback) {
644652
const triggerAsyncId = msg.socket ? msg.socket[async_id_symbol] : undefined;
645653
defaultTriggerAsyncIdScope(triggerAsyncId,
646654
process.nextTick,
647-
writeAfterEndNT,
655+
emitErrorNt,
648656
msg,
649657
err,
650658
callback);
651659
}
652660

661+
function emitErrorNt(msg, err, callback) {
662+
callback(err);
663+
if (typeof msg.emit === 'function') msg.emit('error', err);
664+
}
665+
653666
function write_(msg, chunk, encoding, callback, fromEnd) {
667+
if (typeof callback !== 'function')
668+
callback = nop;
669+
670+
let len;
671+
if (chunk === null) {
672+
throw new ERR_STREAM_NULL_VALUES();
673+
} else if (typeof chunk === 'string') {
674+
len = Buffer.byteLength(chunk, encoding);
675+
} else if (chunk instanceof Buffer) {
676+
len = chunk.length;
677+
} else {
678+
throw new ERR_INVALID_ARG_TYPE(
679+
'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
680+
}
681+
682+
let err;
654683
if (msg.finished) {
655-
writeAfterEnd(msg, callback);
656-
return true;
684+
err = new ERR_STREAM_WRITE_AFTER_END();
685+
} else if (msg.destroyed) {
686+
err = new ERR_STREAM_DESTROYED('write');
687+
}
688+
689+
if (err) {
690+
onError(msg, err, callback);
691+
return false;
657692
}
658693

659694
if (!msg._header) {
695+
if (fromEnd) {
696+
msg._contentLength = len;
697+
}
660698
msg._implicitHeader();
661699
}
662700

663701
if (!msg._hasBody) {
664702
debug('This type of response MUST NOT have a body. ' +
665703
'Ignoring write() calls.');
666-
if (callback) process.nextTick(callback);
704+
process.nextTick(callback);
667705
return true;
668706
}
669707

670-
if (!fromEnd && typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
671-
throw new ERR_INVALID_ARG_TYPE('first argument',
672-
['string', 'Buffer'], chunk);
673-
}
674-
675708
if (!fromEnd && msg.socket && !msg.socket.writableCorked) {
676709
msg.socket.cork();
677710
process.nextTick(connectionCorkNT, msg.socket);
678711
}
679712

680713
let ret;
681714
if (msg.chunkedEncoding && chunk.length !== 0) {
682-
let len;
683-
if (typeof chunk === 'string')
684-
len = Buffer.byteLength(chunk, encoding);
685-
else
686-
len = chunk.length;
687-
688715
msg._send(len.toString(16), 'latin1', null);
689716
msg._send(crlf_buf, null, null);
690717
msg._send(chunk, encoding, null);
@@ -698,12 +725,6 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
698725
}
699726

700727

701-
function writeAfterEndNT(msg, err, callback) {
702-
msg.emit('error', err);
703-
if (callback) callback(err);
704-
}
705-
706-
707728
function connectionCorkNT(conn) {
708729
conn.uncork();
709730
}
@@ -745,6 +766,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
745766
if (typeof chunk === 'function') {
746767
callback = chunk;
747768
chunk = null;
769+
encoding = null;
748770
} else if (typeof encoding === 'function') {
749771
callback = encoding;
750772
encoding = null;
@@ -755,21 +777,6 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
755777
}
756778

757779
if (chunk) {
758-
if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
759-
throw new ERR_INVALID_ARG_TYPE('chunk', ['string', 'Buffer'], chunk);
760-
}
761-
762-
if (this.finished) {
763-
writeAfterEnd(this, callback);
764-
return this;
765-
}
766-
767-
if (!this._header) {
768-
if (typeof chunk === 'string')
769-
this._contentLength = Buffer.byteLength(chunk, encoding);
770-
else
771-
this._contentLength = chunk.length;
772-
}
773780
write_(this, chunk, encoding, null, true);
774781
} else if (this.finished) {
775782
if (typeof callback === 'function') {
Collapse file

‎test/parallel/test-http-outgoing-destroy.js‎

Copy file name to clipboard
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
5+
const http = require('http');
6+
const OutgoingMessage = http.OutgoingMessage;
7+
8+
{
9+
const msg = new OutgoingMessage();
10+
assert.strictEqual(msg.destroyed, false);
11+
msg.destroy();
12+
assert.strictEqual(msg.destroyed, true);
13+
let callbackCalled = false;
14+
msg.write('asd', common.mustCall((err) => {
15+
assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
16+
callbackCalled = true;
17+
}));
18+
msg.on('error', common.mustCall((err) => {
19+
assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
20+
assert.strictEqual(callbackCalled, true);
21+
}));
22+
}
Collapse file

‎test/parallel/test-http-outgoing-finish.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http-outgoing-finish.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

2222
'use strict';
23-
require('../common');
23+
const common = require('../common');
2424
const assert = require('assert');
2525

2626
const http = require('http');
@@ -49,7 +49,7 @@ function write(out) {
4949
let endCb = false;
5050

5151
// First, write until it gets some backpressure
52-
while (out.write(buf)) {}
52+
while (out.write(buf, common.mustCall())) {}
5353

5454
// Now end, and make sure that we don't get the 'finish' event
5555
// before the tick where the cb gets called. We give it until
@@ -65,12 +65,12 @@ function write(out) {
6565
});
6666
});
6767

68-
out.end(buf, function() {
68+
out.end(buf, common.mustCall(function() {
6969
endCb = true;
7070
console.error(`${name} endCb`);
7171
process.nextTick(function() {
7272
assert(finishEvent, `${name} got endCb event before finishEvent!`);
7373
console.log(`ok - ${name} endCb`);
7474
});
75-
});
75+
}));
7676
}
Collapse file

‎test/parallel/test-http-outgoing-proto.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http-outgoing-proto.js
+12-6Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,14 @@ assert.throws(() => {
7474
);
7575
}
7676

77-
assert(OutgoingMessage.prototype.write.call({ _header: 'test' }));
78-
7977
assert.throws(() => {
8078
const outgoingMessage = new OutgoingMessage();
8179
outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' });
8280
}, {
8381
code: 'ERR_INVALID_ARG_TYPE',
8482
name: 'TypeError',
85-
message: 'The first argument must be of type string or an instance of ' +
86-
'Buffer. Received undefined'
83+
message: 'The "chunk" argument must be of type string or an instance of ' +
84+
'Buffer or Uint8Array. Received undefined'
8785
});
8886

8987
assert.throws(() => {
@@ -92,8 +90,16 @@ assert.throws(() => {
9290
}, {
9391
code: 'ERR_INVALID_ARG_TYPE',
9492
name: 'TypeError',
95-
message: 'The first argument must be of type string or an instance of ' +
96-
'Buffer. Received type number (1)'
93+
message: 'The "chunk" argument must be of type string or an instance of ' +
94+
'Buffer or Uint8Array. Received type number (1)'
95+
});
96+
97+
assert.throws(() => {
98+
const outgoingMessage = new OutgoingMessage();
99+
outgoingMessage.write.call({ _header: 'test', _hasBody: 'test' }, null);
100+
}, {
101+
code: 'ERR_STREAM_NULL_VALUES',
102+
name: 'TypeError'
97103
});
98104

99105
// addTrailers()
Collapse file

‎test/parallel/test-http-res-write-after-end.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http-res-write-after-end.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ const server = http.Server(common.mustCall(function(req, res) {
3434
res.end();
3535

3636
const r = res.write('This should raise an error.');
37-
// Write after end should return true
38-
assert.strictEqual(r, true);
37+
// Write after end should return false
38+
assert.strictEqual(r, false);
3939
}));
4040

4141
server.listen(0, function() {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.