Skip to content

Commit d005f49

Browse files
ronagronag
committed
http: cleanup end argument handling
PR-URL: #31818 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent e50ae7a commit d005f49

File tree

5 files changed

+86
-51
lines changed

5 files changed

+86
-51
lines changed

‎lib/_http_outgoing.js‎

Lines 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
constkCorked=Symbol('corked');
6971

72+
functionnop(){}
73+
7074
constRE_CONN_CLOSE=/(?:^|\W)close(?:$|\W)/i;
7175
constRE_TE_CHUNKED=common.chunkExpression;
7276

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

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

642-
functionwriteAfterEnd(msg,callback){
643-
consterr=newERR_STREAM_WRITE_AFTER_END();
651+
functiononError(msg,err,callback){
644652
consttriggerAsyncId=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+
functionemitErrorNt(msg,err,callback){
662+
callback(err);
663+
if(typeofmsg.emit==='function')msg.emit('error',err);
664+
}
665+
653666
functionwrite_(msg,chunk,encoding,callback,fromEnd){
667+
if(typeofcallback!=='function')
668+
callback=nop;
669+
670+
letlen;
671+
if(chunk===null){
672+
thrownewERR_STREAM_NULL_VALUES();
673+
}elseif(typeofchunk==='string'){
674+
len=Buffer.byteLength(chunk,encoding);
675+
}elseif(chunkinstanceofBuffer){
676+
len=chunk.length;
677+
}else{
678+
thrownewERR_INVALID_ARG_TYPE(
679+
'chunk',['string','Buffer','Uint8Array'],chunk);
680+
}
681+
682+
leterr;
654683
if(msg.finished){
655-
writeAfterEnd(msg,callback);
656-
returntrue;
684+
err=newERR_STREAM_WRITE_AFTER_END();
685+
}elseif(msg.destroyed){
686+
err=newERR_STREAM_DESTROYED('write');
687+
}
688+
689+
if(err){
690+
onError(msg,err,callback);
691+
returnfalse;
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
returntrue;
668706
}
669707

670-
if(!fromEnd&&typeofchunk!=='string'&&!(chunkinstanceofBuffer)){
671-
thrownewERR_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
letret;
681714
if(msg.chunkedEncoding&&chunk.length!==0){
682-
letlen;
683-
if(typeofchunk==='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-
functionwriteAfterEndNT(msg,err,callback){
702-
msg.emit('error',err);
703-
if(callback)callback(err);
704-
}
705-
706-
707728
functionconnectionCorkNT(conn){
708729
conn.uncork();
709730
}
@@ -745,6 +766,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback){
745766
if(typeofchunk==='function'){
746767
callback=chunk;
747768
chunk=null;
769+
encoding=null;
748770
}elseif(typeofencoding==='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(typeofchunk!=='string'&&!(chunkinstanceofBuffer)){
759-
thrownewERR_INVALID_ARG_TYPE('chunk',['string','Buffer'],chunk);
760-
}
761-
762-
if(this.finished){
763-
writeAfterEnd(this,callback);
764-
returnthis;
765-
}
766-
767-
if(!this._header){
768-
if(typeofchunk==='string')
769-
this._contentLength=Buffer.byteLength(chunk,encoding);
770-
else
771-
this._contentLength=chunk.length;
772-
}
773780
write_(this,chunk,encoding,null,true);
774781
}elseif(this.finished){
775782
if(typeofcallback==='function'){

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
constassert=require('assert');
4+
5+
consthttp=require('http');
6+
constOutgoingMessage=http.OutgoingMessage;
7+
8+
{
9+
constmsg=newOutgoingMessage();
10+
assert.strictEqual(msg.destroyed,false);
11+
msg.destroy();
12+
assert.strictEqual(msg.destroyed,true);
13+
letcallbackCalled=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+
}

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

Lines 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+
constcommon=require('../common');
2424
constassert=require('assert');
2525

2626
consthttp=require('http');
@@ -49,7 +49,7 @@ function write(out){
4949
letendCb=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
}

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

Lines 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
constoutgoingMessage=newOutgoingMessage();
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+
constoutgoingMessage=newOutgoingMessage();
99+
outgoingMessage.write.call({_header: 'test',_hasBody: 'test'},null);
100+
},{
101+
code: 'ERR_STREAM_NULL_VALUES',
102+
name: 'TypeError'
97103
});
98104

99105
// addTrailers()

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

Lines 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
constr=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)