FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: make HTTP2ServerResponse more streams compliant · nodejs/node@f63e440 · GitHub

/ node Public

Commit f63e440

Browse files
ronag
authored and
MylesBorins
committed
http2: make HTTP2ServerResponse more streams compliant
HTTP2ServerResponse.write would behave differently than both http1 and streams. This PR makes it more compliant with stream.Writable behaviour. Backport-PR-URL: #31444 PR-URL: #30964 Refs: #29529 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 02d7283 commit f63e440

2 files changed

Lines changed: 74 additions & 40 deletions

File tree

‎lib/internal/http2/compat.js‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ const {
4040
ERR_HTTP2_STATUS_INVALID,
4141
ERR_INVALID_ARG_VALUE,
4242
ERR_INVALID_CALLBACK,
43-
ERR_INVALID_HTTP_TOKEN
43+
ERR_INVALID_HTTP_TOKEN,
44+
ERR_STREAM_WRITE_AFTER_END
4445
},
4546
hideStackFrames
4647
} = require('internal/errors');
@@ -439,6 +440,7 @@ class Http2ServerResponse extends Stream {
439440
this[kState] = {
440441
closed: false,
441442
ending: false,
443+
destroyed: false,
442444
headRequest: false,
443445
sendDate: true,
444446
statusCode: HTTP_STATUS_OK,
@@ -649,23 +651,32 @@ class Http2ServerResponse extends Stream {
649651
}
650652

651653
write(chunk, encoding, cb) {
654+
const state = this[kState];
655+
652656
if (typeof encoding === 'function') {
653657
cb = encoding;
654658
encoding = 'utf8';
655659
}
656660

657-
if (this[kState].closed) {
658-
const err = new ERR_HTTP2_INVALID_STREAM();
661+
let err;
662+
if (state.ending) {
663+
err = new ERR_STREAM_WRITE_AFTER_END();
664+
} else if (state.closed) {
665+
err = new ERR_HTTP2_INVALID_STREAM();
666+
} else if (state.destroyed) {
667+
return false;
668+
}
669+
670+
if (err) {
659671
if (typeof cb === 'function')
660672
process.nextTick(cb, err);
661-
else
662-
throw err;
663-
return;
673+
this.destroy(err);
674+
return false;
664675
}
665676

666677
const stream = this[kStream];
667678
if (!stream.headersSent)
668-
this.writeHead(this[kState].statusCode);
679+
this.writeHead(state.statusCode);
669680
return stream.write(chunk, encoding, cb);
670681
}
671682

@@ -712,8 +723,10 @@ class Http2ServerResponse extends Stream {
712723
}
713724

714725
destroy(err) {
715-
if (this[kState].closed)
726+
if (this[kState].destroyed)
716727
return;
728+
729+
this[kState].destroyed = true;
717730
this[kStream].destroy(err);
718731
}
719732

‎test/parallel/test-http2-compat-serverresponse-write.js‎

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,50 +3,71 @@
33
const {
44
mustCall,
55
mustNotCall,
6-
expectsError,
76
hasCrypto,
87
skip
98
} = require('../common');
109
if (!hasCrypto)
1110
skip('missing crypto');
1211
const { createServer, connect } = require('http2');
1312
const assert = require('assert');
14-
15-
const server = createServer();
16-
server.listen(0, mustCall(() => {
17-
const port = server.address().port;
18-
const url = `http://localhost:${port}`;
19-
const client = connect(url, mustCall(() => {
20-
const request = client.request();
21-
request.resume();
22-
request.on('end', mustCall());
23-
request.on('close', mustCall(() => {
24-
client.close();
13+
{
14+
const server = createServer();
15+
server.listen(0, mustCall(() => {
16+
const port = server.address().port;
17+
const url = `http://localhost:${port}`;
18+
const client = connect(url, mustCall(() => {
19+
const request = client.request();
20+
request.resume();
21+
request.on('end', mustCall());
22+
request.on('close', mustCall(() => {
23+
client.close();
24+
}));
2525
}));
26-
}));
2726

28-
server.once('request', mustCall((request, response) => {
29-
// response.write() returns true
30-
assert(response.write('muahaha', 'utf8', mustCall()));
27+
server.once('request', mustCall((request, response) => {
28+
// response.write() returns true
29+
assert(response.write('muahaha', 'utf8', mustCall()));
3130

32-
response.stream.close(0, mustCall(() => {
33-
response.on('error', mustNotCall());
31+
response.stream.close(0, mustCall(() => {
32+
response.on('error', mustNotCall());
3433

35-
// response.write() without cb returns error
36-
expectsError(
37-
() => { response.write('muahaha'); },
38-
{
39-
type: Error,
40-
code: 'ERR_HTTP2_INVALID_STREAM',
41-
message: 'The stream has been destroyed'
42-
}
43-
);
34+
// response.write() without cb returns error
35+
response.write('muahaha', mustCall((err) => {
36+
assert.strictEqual(err.code, 'ERR_HTTP2_INVALID_STREAM');
4437

45-
// response.write() with cb returns falsy value
46-
assert(!response.write('muahaha', mustCall()));
38+
// response.write() with cb returns falsy value
39+
assert(!response.write('muahaha', mustCall()));
40+
41+
client.destroy();
42+
server.close();
43+
}));
44+
}));
45+
}));
46+
}));
47+
}
48+
49+
{
50+
// Http2ServerResponse.write ERR_STREAM_WRITE_AFTER_END
51+
const server = createServer();
52+
server.listen(0, mustCall(() => {
53+
const port = server.address().port;
54+
const url = `http://localhost:${port}`;
55+
const client = connect(url, mustCall(() => {
56+
const request = client.request();
57+
request.resume();
58+
request.on('end', mustCall());
59+
request.on('close', mustCall(() => {
60+
client.close();
61+
}));
62+
}));
4763

48-
client.destroy();
49-
server.close();
64+
server.once('request', mustCall((request, response) => {
65+
response.end();
66+
response.write('asd', mustCall((err) => {
67+
assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
68+
client.destroy();
69+
server.close();
70+
}));
5071
}));
5172
}));
52-
}));
73+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL