Skip to content

Commit 2cc0482

Browse files
mcollinatargos
authored andcommitted
http2: implement capture rection for 'request' and 'stream' events
PR-URL: #27867 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 48fcd76 commit 2cc0482

2 files changed

Lines changed: 197 additions & 0 deletions

File tree

‎lib/internal/http2/core.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,6 +1414,17 @@ class Http2Session extends EventEmitter {
14141414
this[kMaybeDestroy]();
14151415
}
14161416

1417+
[EventEmitter.captureRejectionSymbol](err,event, ...args){
1418+
switch(event){
1419+
case'stream':
1420+
const[stream]=args;
1421+
stream.destroy(err);
1422+
break;
1423+
default:
1424+
this.destroy(err);
1425+
}
1426+
}
1427+
14171428
// Destroy the session if:
14181429
// * error is not undefined/null
14191430
// * session is closed and there are no more pending or open streams
@@ -2905,6 +2916,40 @@ class Http2Server extends NETServer {
29052916
}
29062917
}
29072918

2919+
Http2Server.prototype[EventEmitter.captureRejectionSymbol]=function(
2920+
err,event, ...args){
2921+
2922+
switch(event){
2923+
case'stream':
2924+
// TODO(mcollina): we might want to match this with what we do on
2925+
// the compat side.
2926+
const[stream]=args;
2927+
if(stream.sentHeaders){
2928+
stream.destroy(err);
2929+
}else{
2930+
stream.respond({[HTTP2_HEADER_STATUS]: 500});
2931+
stream.end();
2932+
}
2933+
break;
2934+
case'request':
2935+
const[,res]=args;
2936+
if(!res.headersSent&&!res.finished){
2937+
// Don't leak headers.
2938+
for(constnameofres.getHeaderNames()){
2939+
res.removeHeader(name);
2940+
}
2941+
res.statusCode=500;
2942+
res.end(http.STATUS_CODES[500]);
2943+
}else{
2944+
res.destroy();
2945+
}
2946+
break;
2947+
default:
2948+
net.Server.prototype[EventEmitter.captureRejectionSymbol]
2949+
.call(this,err,event, ...args);
2950+
}
2951+
};
2952+
29082953
functionsetupCompat(ev){
29092954
if(ev==='request'){
29102955
this.removeListener('newListener',setupCompat);
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
if(!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
constassert=require('assert');
8+
constevents=require('events');
9+
const{ createServer, connect }=require('http2');
10+
11+
events.captureRejections=true;
12+
13+
{
14+
// Test error thrown in the server 'stream' event,
15+
// after a respond()
16+
17+
constserver=createServer();
18+
server.on('stream',common.mustCall(async(stream)=>{
19+
server.close();
20+
21+
stream.respond({':status': 200});
22+
23+
const_err=newError('kaboom');
24+
stream.on('error',common.mustCall((err)=>{
25+
assert.strictEqual(err,_err);
26+
}));
27+
throw_err;
28+
}));
29+
30+
server.listen(0,common.mustCall(()=>{
31+
const{ port }=server.address();
32+
constsession=connect(`http://localhost:${port}`);
33+
34+
constreq=session.request();
35+
36+
req.on('error',common.mustCall((err)=>{
37+
assert.strictEqual(err.code,'ERR_HTTP2_STREAM_ERROR');
38+
}));
39+
40+
req.on('close',common.mustCall(()=>{
41+
session.close();
42+
}));
43+
}));
44+
}
45+
46+
{
47+
// Test error thrown in the server 'stream' event,
48+
// before a respond().
49+
50+
constserver=createServer();
51+
server.on('stream',common.mustCall(async(stream)=>{
52+
server.close();
53+
54+
stream.on('error',common.mustNotCall());
55+
56+
thrownewError('kaboom');
57+
}));
58+
59+
server.listen(0,common.mustCall(()=>{
60+
const{ port }=server.address();
61+
constsession=connect(`http://localhost:${port}`);
62+
63+
constreq=session.request();
64+
65+
req.on('response',common.mustCall((headers)=>{
66+
assert.strictEqual(headers[':status'],500);
67+
}));
68+
69+
req.on('close',common.mustCall(()=>{
70+
session.close();
71+
}));
72+
}));
73+
}
74+
75+
76+
{
77+
// Test error thrown in 'request' event
78+
79+
constserver=createServer(common.mustCall(async(req,res)=>{
80+
server.close();
81+
res.setHeader('content-type','application/json');
82+
const_err=newError('kaboom');
83+
throw_err;
84+
}));
85+
86+
server.listen(0,common.mustCall(()=>{
87+
const{ port }=server.address();
88+
constsession=connect(`http://localhost:${port}`);
89+
90+
constreq=session.request();
91+
92+
req.on('response',common.mustCall((headers)=>{
93+
assert.strictEqual(headers[':status'],500);
94+
assert.strictEqual(Object.hasOwnProperty.call(headers,'content-type'),
95+
false);
96+
}));
97+
98+
req.on('close',common.mustCall(()=>{
99+
session.close();
100+
}));
101+
102+
req.resume();
103+
}));
104+
}
105+
106+
{
107+
// Test error thrown in the client 'stream' event
108+
109+
constserver=createServer();
110+
server.on('stream',common.mustCall(async(stream)=>{
111+
const{ port }=server.address();
112+
113+
server.close();
114+
115+
stream.pushStream({
116+
':scheme': 'http',
117+
':path': '/foobar',
118+
':authority': `localhost:${port}`,
119+
},common.mustCall((err,push)=>{
120+
push.respond({
121+
'content-type': 'text/html',
122+
':status': 200
123+
});
124+
push.end('pushed by the server');
125+
126+
stream.end('test');
127+
}));
128+
129+
stream.respond({
130+
':status': 200
131+
});
132+
}));
133+
134+
server.listen(0,common.mustCall(()=>{
135+
const{ port }=server.address();
136+
constsession=connect(`http://localhost:${port}`);
137+
138+
constreq=session.request();
139+
140+
session.on('stream',common.mustCall(async(stream)=>{
141+
session.close();
142+
143+
const_err=newError('kaboom');
144+
stream.on('error',common.mustCall((err)=>{
145+
assert.strictEqual(err,_err);
146+
}));
147+
throw_err;
148+
}));
149+
150+
req.end();
151+
}));
152+
}

0 commit comments

Comments
 (0)