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 e747ef5

Browse filesBrowse files
ofirdanielleadams
ofir
authored and
danielleadams
committed
http2: fix pseudo-headers order
Keep pseudo-headers order same as sent order Fixes: #38797 PR-URL: #41735 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent bf99ef8 commit e747ef5
Copy full SHA for e747ef5

File tree

Expand file treeCollapse file tree

4 files changed

+156
-90
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+156
-90
lines changed
Open diff view settings
Collapse file

‎lib/internal/http2/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/util.js
+6-5Lines 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
const kNoHeaderFlags = StringFromCharCode(NGHTTP2_NV_FLAG_NONE);
473473
function mapToHeaders(map,
474474
assertValuePseudoHeader = assertValidPseudoHeader) {
475-
let ret = '';
475+
let headers = '';
476+
let pseudoHeaders = '';
476477
let count = 0;
477478
const keys = ObjectKeys(map);
478479
const singles = new SafeSet();
@@ -520,7 +521,7 @@ function mapToHeaders(map,
520521
err = assertValuePseudoHeader(key);
521522
if (err !== undefined)
522523
throw err;
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
const val = 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
class NghttpError extends Error {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http2-compat-serverrequest-headers.js
+137-72Lines changed: 137 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -6,83 +6,148 @@ if (!common.hasCrypto)
66
const assert = require('assert');
77
const h2 = require('http2');
88

9-
// Http2ServerRequest should have header helpers
10-
11-
const server = h2.createServer();
12-
server.listen(0, common.mustCall(function() {
13-
const port = server.address().port;
14-
server.once('request', common.mustCall(function(request, response) {
15-
const expected = {
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-
const headers = request.headers;
30-
for (const [name, value] of Object.entries(expected)) {
31-
assert.strictEqual(headers[name], value);
32-
}
33-
34-
const rawHeaders = request.rawHeaders;
35-
for (const [name, value] of Object.entries(expected)) {
36-
const position = 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+
const server = h2.createServer();
13+
server.listen(0, common.mustCall(function() {
14+
const port = server.address().port;
15+
server.once('request', common.mustCall(function(request, response) {
16+
const expected = {
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+
const headers = request.headers;
31+
for (const [name, value] of Object.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+
const rawHeaders = request.rawHeaders;
36+
for (const [name, value] of Object.entries(expected)) {
37+
const position = 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+
const url = `http://localhost:${port}`;
74+
const client = h2.connect(url, common.mustCall(function() {
75+
const headers = {
76+
':path': '/foobar',
77+
':method': 'GET',
78+
':scheme': 'http',
79+
':authority': `localhost:${port}`,
80+
'foo-bar': 'abc123'
81+
};
82+
const request = 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+
const server = h2.createServer();
97+
server.listen(0, common.mustCall(function() {
98+
const port = server.address().port;
99+
server.once('request', common.mustCall(function(request, response) {
100+
const expected = {
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+
const headers = request.headers;
116+
for (const [name, value] of Object.entries(expected)) {
117+
assert.strictEqual(headers[name], value);
118+
}
119+
120+
const rawHeaders = request.rawHeaders;
121+
let expectedPosition = 0;
122+
for (const [name, value] of Object.entries(expected)) {
123+
const position = 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-
const url = `http://localhost:${port}`;
73-
const client = h2.connect(url, common.mustCall(function() {
74-
const headers = {
75-
':path': '/foobar',
76-
':method': 'GET',
77-
':scheme': 'http',
78-
':authority': `localhost:${port}`,
79-
'foo-bar': 'abc123'
80-
};
81-
const request = client.request(headers);
82-
request.on('end', common.mustCall(function() {
83-
client.close();
135+
const url = `http://localhost:${port}`;
136+
const client = h2.connect(url, common.mustCall(function() {
137+
const headers = {
138+
':path': '/foobar',
139+
':method': 'GET',
140+
'foo1': 'abc1',
141+
':scheme': 'http',
142+
'foo2': 'abc2',
143+
':authority': `localhost:${port}`
144+
};
145+
const request = 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http2-multiheaders-raw.js
+6-6Lines 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
const expected = [
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',
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http2-util-headers-list.js
+7-7Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ const {
9898
{
9999
const headers = {
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
const headers = {
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
const headers = {
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
const base = { 'abc': 1 };
152152
const headers = 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
const headers = {
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.