GeezerVison is showing github.com — http2: fix pseudo-headers order · nodejs/node@e747ef5
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/http2/util.js +6-5Lines changed: 6 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -472,7 +472,8 @@ const kNeverIndexFlag = StringFromCharCode(NGHTTP2_NV_FLAG_NO_INDEX); | 472 | 472 | const kNoHeaderFlags = StringFromCharCode(NGHTTP2_NV_FLAG_NONE); | 473 | 473 | function mapToHeaders(map, | 474 | 474 | assertValuePseudoHeader = assertValidPseudoHeader) { | 475 | | - let ret = ''; | | 475 | + let headers = ''; | | 476 | + let pseudoHeaders = ''; | 476 | 477 | let count = 0; | 477 | 478 | const keys = ObjectKeys(map); | 478 | 479 | const singles = new SafeSet(); | @@ -520,7 +521,7 @@ function mapToHeaders(map, | 520 | 521 | err = assertValuePseudoHeader(key); | 521 | 522 | if (err !== undefined) | 522 | 523 | throw err; | 523 | | - ret = `${key}\0${value}\0${flags}${ret}`; | | 524 | + pseudoHeaders += `${key}\0${value}\0${flags}`; | 524 | 525 | count++; | 525 | 526 | continue; | 526 | 527 | } | @@ -533,16 +534,16 @@ function mapToHeaders(map, | 533 | 534 | if (isArray) { | 534 | 535 | for (j = 0; j < value.length; ++j) { | 535 | 536 | const val = String(value[j]); | 536 | | - ret += `${key}\0${val}\0${flags}`; | | 537 | + headers += `${key}\0${val}\0${flags}`; | 537 | 538 | } | 538 | 539 | count += value.length; | 539 | 540 | continue; | 540 | 541 | } | 541 | | - ret += `${key}\0${value}\0${flags}`; | | 542 | + headers += `${key}\0${value}\0${flags}`; | 542 | 543 | count++; | 543 | 544 | } | 544 | 545 |
| 545 | | - return [ret, count]; | | 546 | + return [pseudoHeaders + headers, count]; | 546 | 547 | } | 547 | 548 |
| 548 | 549 | class NghttpError extends Error { | |
Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -6,83 +6,148 @@ if (!common.hasCrypto) | 6 | 6 | const assert = require('assert'); | 7 | 7 | const h2 = require('http2'); | 8 | 8 |
| 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); | 54 | 33 | } | 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); | 63 | 40 | } | 64 | | - ); | 65 | 41 |
| 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(); | 68 | 88 | })); | 69 | | - response.end(); | 70 | 89 | })); | | 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 | + })); | 71 | 134 |
| 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(); | 84 | 151 | })); | 85 | | - request.end(); | 86 | | - request.resume(); | 87 | 152 | })); | 88 | | -})); | | 153 | +} |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-http2-multiheaders-raw.js +6-6Lines changed: 6 additions & 6 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -16,14 +16,14 @@ src.test = 'foo, bar, baz'; | 16 | 16 |
| 17 | 17 | server.on('stream', common.mustCall((stream, headers, flags, rawHeaders) => { | 18 | 18 | const expected = [ | 19 | | - ':path', | 20 | | - '/', | 21 | | - ':scheme', | 22 | | - 'http', | 23 | | - ':authority', | 24 | | - `localhost:${server.address().port}`, | 25 | 19 | ':method', | 26 | 20 | 'GET', | | 21 | + ':authority', | | 22 | + `localhost:${server.address().port}`, | | 23 | + ':scheme', | | 24 | + 'http', | | 25 | + ':path', | | 26 | + '/', | 27 | 27 | 'www-authenticate', | 28 | 28 | 'foo', | 29 | 29 | 'www-authenticate', | |
Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -98,8 +98,8 @@ const { | 98 | 98 | { | 99 | 99 | const headers = { | 100 | 100 | 'abc': 1, | 101 | | - ':status': 200, | 102 | 101 | ':path': 'abc', | | 102 | + ':status': 200, | 103 | 103 | 'xyz': [1, '2', { toString() { return '3'; } }, 4], | 104 | 104 | 'foo': [], | 105 | 105 | 'BAR': [1] | @@ -116,8 +116,8 @@ const { | 116 | 116 | { | 117 | 117 | const headers = { | 118 | 118 | 'abc': 1, | 119 | | - ':path': 'abc', | 120 | 119 | ':status': [200], | | 120 | + ':path': 'abc', | 121 | 121 | ':authority': [], | 122 | 122 | 'xyz': [1, 2, 3, 4] | 123 | 123 | }; | @@ -132,10 +132,10 @@ const { | 132 | 132 | { | 133 | 133 | const headers = { | 134 | 134 | 'abc': 1, | 135 | | - ':path': 'abc', | | 135 | + ':status': 200, | 136 | 136 | 'xyz': [1, 2, 3, 4], | 137 | 137 | '': 1, | 138 | | - ':status': 200, | | 138 | + ':path': 'abc', | 139 | 139 | [Symbol('test')]: 1 // Symbol keys are ignored | 140 | 140 | }; | 141 | 141 |
| @@ -150,10 +150,10 @@ const { | 150 | 150 | // Only own properties are used | 151 | 151 | const base = { 'abc': 1 }; | 152 | 152 | const headers = Object.create(base); | 153 | | - headers[':path'] = 'abc'; | | 153 | + headers[':status'] = 200; | 154 | 154 | headers.xyz = [1, 2, 3, 4]; | 155 | 155 | headers.foo = []; | 156 | | - headers[':status'] = 200; | | 156 | + headers[':path'] = 'abc'; | 157 | 157 |
| 158 | 158 | assert.deepStrictEqual( | 159 | 159 | mapToHeaders(headers), | @@ -191,8 +191,8 @@ const { | 191 | 191 | { | 192 | 192 | const headers = { | 193 | 193 | 'abc': 1, | 194 | | - ':path': 'abc', | 195 | 194 | ':status': [200], | | 195 | + ':path': 'abc', | 196 | 196 | ':authority': [], | 197 | 197 | 'xyz': [1, 2, 3, 4], | 198 | 198 | [sensitiveHeaders]: ['xyz'] | |
|