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 f63e440

Browse filesBrowse files
ronagMylesBorins
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
Copy full SHA for f63e440

File tree

Expand file treeCollapse file tree

2 files changed

+74
-40
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+74
-40
lines changed
Open diff view settings
Collapse file

‎lib/internal/http2/compat.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/compat.js
+21-8Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http2-compat-serverresponse-write.js
+53-32Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.