Skip to content

Commit bfddcb1

Browse files
ofirruyadorno
ofir
authored and
ruyadorno
committed
http2: fix pseudo-headers order
Keep pseudo-headers order same as sent order Fixes: #38797 PR-URL: #41735 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 145a3b8 commit bfddcb1

File tree

4 files changed

+156
-90
lines changed

4 files changed

+156
-90
lines changed

‎lib/internal/http2/util.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,8 @@ const kNeverIndexFlag = StringFromCharCode(NGHTTP2_NV_FLAG_NO_INDEX);
472472
constkNoHeaderFlags=StringFromCharCode(NGHTTP2_NV_FLAG_NONE);
473473
functionmapToHeaders(map,
474474
assertValuePseudoHeader=assertValidPseudoHeader){
475-
letret='';
475+
letheaders='';
476+
letpseudoHeaders='';
476477
letcount=0;
477478
constkeys=ObjectKeys(map);
478479
constsingles=newSafeSet();
@@ -520,7 +521,7 @@ function mapToHeaders(map,
520521
err=assertValuePseudoHeader(key);
521522
if(err!==undefined)
522523
throwerr;
523-
ret=`${key}\0${value}\0${flags}${ret}`;
524+
pseudoHeaders+=`${key}\0${value}\0${flags}`;
524525
count++;
525526
continue;
526527
}
@@ -533,16 +534,16 @@ function mapToHeaders(map,
533534
if(isArray){
534535
for(j=0;j<value.length;++j){
535536
constval=String(value[j]);
536-
ret+=`${key}\0${val}\0${flags}`;
537+
headers+=`${key}\0${val}\0${flags}`;
537538
}
538539
count+=value.length;
539540
continue;
540541
}
541-
ret+=`${key}\0${value}\0${flags}`;
542+
headers+=`${key}\0${value}\0${flags}`;
542543
count++;
543544
}
544545

545-
return[ret,count];
546+
return[pseudoHeaders+headers,count];
546547
}
547548

548549
classNghttpErrorextendsError{

‎test/parallel/test-http2-compat-serverrequest-headers.js‎

Lines changed: 137 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -6,83 +6,148 @@ if (!common.hasCrypto)
66
constassert=require('assert');
77
consth2=require('http2');
88

9-
// Http2ServerRequest should have header helpers
10-
11-
constserver=h2.createServer();
12-
server.listen(0,common.mustCall(function(){
13-
constport=server.address().port;
14-
server.once('request',common.mustCall(function(request,response){
15-
constexpected={
16-
':path': '/foobar',
17-
':method': 'GET',
18-
':scheme': 'http',
19-
':authority': `localhost:${port}`,
20-
'foo-bar': 'abc123'
21-
};
22-
23-
assert.strictEqual(request.path,undefined);
24-
assert.strictEqual(request.method,expected[':method']);
25-
assert.strictEqual(request.scheme,expected[':scheme']);
26-
assert.strictEqual(request.url,expected[':path']);
27-
assert.strictEqual(request.authority,expected[':authority']);
28-
29-
constheaders=request.headers;
30-
for(const[name,value]ofObject.entries(expected)){
31-
assert.strictEqual(headers[name],value);
32-
}
33-
34-
constrawHeaders=request.rawHeaders;
35-
for(const[name,value]ofObject.entries(expected)){
36-
constposition=rawHeaders.indexOf(name);
37-
assert.notStrictEqual(position,-1);
38-
assert.strictEqual(rawHeaders[position+1],value);
39-
}
40-
41-
request.url='/one';
42-
assert.strictEqual(request.url,'/one');
43-
44-
// Third-party plugins for packages like express use query params to
45-
// change the request method
46-
request.method='POST';
47-
assert.strictEqual(request.method,'POST');
48-
assert.throws(
49-
()=>request.method=' ',
50-
{
51-
code: 'ERR_INVALID_ARG_VALUE',
52-
name: 'TypeError',
53-
message: "The argument 'method' is invalid. Received ' '"
9+
{
10+
// Http2ServerRequest should have header helpers
11+
12+
constserver=h2.createServer();
13+
server.listen(0,common.mustCall(function(){
14+
constport=server.address().port;
15+
server.once('request',common.mustCall(function(request,response){
16+
constexpected={
17+
':path': '/foobar',
18+
':method': 'GET',
19+
':scheme': 'http',
20+
':authority': `localhost:${port}`,
21+
'foo-bar': 'abc123'
22+
};
23+
24+
assert.strictEqual(request.path,undefined);
25+
assert.strictEqual(request.method,expected[':method']);
26+
assert.strictEqual(request.scheme,expected[':scheme']);
27+
assert.strictEqual(request.url,expected[':path']);
28+
assert.strictEqual(request.authority,expected[':authority']);
29+
30+
constheaders=request.headers;
31+
for(const[name,value]ofObject.entries(expected)){
32+
assert.strictEqual(headers[name],value);
5433
}
55-
);
56-
assert.throws(
57-
()=>request.method=true,
58-
{
59-
code: 'ERR_INVALID_ARG_TYPE',
60-
name: 'TypeError',
61-
message: 'The "method" argument must be of type string. '+
62-
'Received type boolean (true)'
34+
35+
constrawHeaders=request.rawHeaders;
36+
for(const[name,value]ofObject.entries(expected)){
37+
constposition=rawHeaders.indexOf(name);
38+
assert.notStrictEqual(position,-1);
39+
assert.strictEqual(rawHeaders[position+1],value);
6340
}
64-
);
6541

66-
response.on('finish',common.mustCall(function(){
67-
server.close();
42+
request.url='/one';
43+
assert.strictEqual(request.url,'/one');
44+
45+
// Third-party plugins for packages like express use query params to
46+
// change the request method
47+
request.method='POST';
48+
assert.strictEqual(request.method,'POST');
49+
assert.throws(
50+
()=>request.method=' ',
51+
{
52+
code: 'ERR_INVALID_ARG_VALUE',
53+
name: 'TypeError',
54+
message: "The argument 'method' is invalid. Received ' '"
55+
}
56+
);
57+
assert.throws(
58+
()=>request.method=true,
59+
{
60+
code: 'ERR_INVALID_ARG_TYPE',
61+
name: 'TypeError',
62+
message: 'The "method" argument must be of type string. '+
63+
'Received type boolean (true)'
64+
}
65+
);
66+
67+
response.on('finish',common.mustCall(function(){
68+
server.close();
69+
}));
70+
response.end();
71+
}));
72+
73+
consturl=`http://localhost:${port}`;
74+
constclient=h2.connect(url,common.mustCall(function(){
75+
constheaders={
76+
':path': '/foobar',
77+
':method': 'GET',
78+
':scheme': 'http',
79+
':authority': `localhost:${port}`,
80+
'foo-bar': 'abc123'
81+
};
82+
constrequest=client.request(headers);
83+
request.on('end',common.mustCall(function(){
84+
client.close();
85+
}));
86+
request.end();
87+
request.resume();
6888
}));
69-
response.end();
7089
}));
90+
}
91+
92+
{
93+
// Http2ServerRequest should keep pseudo-headers order and after them,
94+
// in the same order, the others headers
95+
96+
constserver=h2.createServer();
97+
server.listen(0,common.mustCall(function(){
98+
constport=server.address().port;
99+
server.once('request',common.mustCall(function(request,response){
100+
constexpected={
101+
':path': '/foobar',
102+
':method': 'GET',
103+
':scheme': 'http',
104+
':authority': `localhost:${port}`,
105+
'foo1': 'abc1',
106+
'foo2': 'abc2'
107+
};
108+
109+
assert.strictEqual(request.path,undefined);
110+
assert.strictEqual(request.method,expected[':method']);
111+
assert.strictEqual(request.scheme,expected[':scheme']);
112+
assert.strictEqual(request.url,expected[':path']);
113+
assert.strictEqual(request.authority,expected[':authority']);
114+
115+
constheaders=request.headers;
116+
for(const[name,value]ofObject.entries(expected)){
117+
assert.strictEqual(headers[name],value);
118+
}
119+
120+
constrawHeaders=request.rawHeaders;
121+
letexpectedPosition=0;
122+
for(const[name,value]ofObject.entries(expected)){
123+
constposition=rawHeaders.indexOf(name);
124+
assert.strictEqual(position/2,expectedPosition);
125+
assert.strictEqual(rawHeaders[position+1],value);
126+
expectedPosition++;
127+
}
128+
129+
response.on('finish',common.mustCall(function(){
130+
server.close();
131+
}));
132+
response.end();
133+
}));
71134

72-
consturl=`http://localhost:${port}`;
73-
constclient=h2.connect(url,common.mustCall(function(){
74-
constheaders={
75-
':path': '/foobar',
76-
':method': 'GET',
77-
':scheme': 'http',
78-
':authority': `localhost:${port}`,
79-
'foo-bar': 'abc123'
80-
};
81-
constrequest=client.request(headers);
82-
request.on('end',common.mustCall(function(){
83-
client.close();
135+
consturl=`http://localhost:${port}`;
136+
constclient=h2.connect(url,common.mustCall(function(){
137+
constheaders={
138+
':path': '/foobar',
139+
':method': 'GET',
140+
'foo1': 'abc1',
141+
':scheme': 'http',
142+
'foo2': 'abc2',
143+
':authority': `localhost:${port}`
144+
};
145+
constrequest=client.request(headers);
146+
request.on('end',common.mustCall(function(){
147+
client.close();
148+
}));
149+
request.end();
150+
request.resume();
84151
}));
85-
request.end();
86-
request.resume();
87152
}));
88-
}));
153+
}

‎test/parallel/test-http2-multiheaders-raw.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ src.test = 'foo, bar, baz'
1616

1717
server.on('stream',common.mustCall((stream,headers,flags,rawHeaders)=>{
1818
constexpected=[
19-
':path',
20-
'/',
21-
':scheme',
22-
'http',
23-
':authority',
24-
`localhost:${server.address().port}`,
2519
':method',
2620
'GET',
21+
':authority',
22+
`localhost:${server.address().port}`,
23+
':scheme',
24+
'http',
25+
':path',
26+
'/',
2727
'www-authenticate',
2828
'foo',
2929
'www-authenticate',

‎test/parallel/test-http2-util-headers-list.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ const{
9898
{
9999
constheaders={
100100
'abc': 1,
101-
':status': 200,
102101
':path': 'abc',
102+
':status': 200,
103103
'xyz': [1,'2',{toString(){return'3';}},4],
104104
'foo': [],
105105
'BAR': [1]
@@ -116,8 +116,8 @@ const{
116116
{
117117
constheaders={
118118
'abc': 1,
119-
':path': 'abc',
120119
':status': [200],
120+
':path': 'abc',
121121
':authority': [],
122122
'xyz': [1,2,3,4]
123123
};
@@ -132,10 +132,10 @@ const{
132132
{
133133
constheaders={
134134
'abc': 1,
135-
':path': 'abc',
135+
':status': 200,
136136
'xyz': [1,2,3,4],
137137
'': 1,
138-
':status': 200,
138+
':path': 'abc',
139139
[Symbol('test')]: 1// Symbol keys are ignored
140140
};
141141

@@ -150,10 +150,10 @@ const{
150150
// Only own properties are used
151151
constbase={'abc': 1};
152152
constheaders=Object.create(base);
153-
headers[':path']='abc';
153+
headers[':status']=200;
154154
headers.xyz=[1,2,3,4];
155155
headers.foo=[];
156-
headers[':status']=200;
156+
headers[':path']='abc';
157157

158158
assert.deepStrictEqual(
159159
mapToHeaders(headers),
@@ -191,8 +191,8 @@ const{
191191
{
192192
constheaders={
193193
'abc': 1,
194-
':path': 'abc',
195194
':status': [200],
195+
':path': 'abc',
196196
':authority': [],
197197
'xyz': [1,2,3,4],
198198
[sensitiveHeaders]: ['xyz']

0 commit comments

Comments
(0)