chore: change order of the pseudo-headers (nodejs#2308)

lib/client.js

@@ -93,6 +93,7 @@ const {
     HTTP2_HEADER_AUTHORITY,
     HTTP2_HEADER_METHOD,
     HTTP2_HEADER_PATH,
+    HTTP2_HEADER_SCHEME,
     HTTP2_HEADER_CONTENT_LENGTH,
     HTTP2_HEADER_EXPECT,
     HTTP2_HEADER_STATUS
@@ -1689,7 +1690,7 @@ function writeH2 (client, session, request) {
   const h2State = client[kHTTP2SessionState]
 
   headers[HTTP2_HEADER_AUTHORITY] = host || client[kHost]
-  headers[HTTP2_HEADER_PATH] = path
+  headers[HTTP2_HEADER_METHOD] = method
 
   if (method === 'CONNECT') {
     session.ref()
@@ -1716,10 +1717,14 @@ function writeH2 (client, session, request) {
     })
 
     return true
-  } else {
-    headers[HTTP2_HEADER_METHOD] = method
   }
 
+  // https://tools.ietf.org/html/rfc7540#section-8.3
+  // :path and :scheme headers must be omited when sending CONNECT
+
+  headers[HTTP2_HEADER_PATH] = path
+  headers[HTTP2_HEADER_SCHEME] = 'https'
+
   // https://tools.ietf.org/html/rfc7231#section-4.3.1
   // https://tools.ietf.org/html/rfc7231#section-4.3.2
   // https://tools.ietf.org/html/rfc7231#section-4.3.5

package.json

@@ -122,6 +122,7 @@
     "pre-commit": "^1.2.2",
     "proxy": "^1.0.2",
     "proxyquire": "^2.1.3",
+    "semver": "^7.5.4",
     "sinon": "^15.0.0",
     "snazzy": "^9.0.0",
     "standard": "^17.0.0",

test/http2.js

@@ -7,19 +7,23 @@ const { Blob } = require('node:buffer')
 const { Writable, pipeline, PassThrough, Readable } = require('node:stream')
 
 const { test, plan } = require('tap')
+const { gte } = require('semver')
 const pem = require('https-pem')
 
 const { Client, Agent } = require('..')
 
-const isGreaterThanv20 = Number(process.version.slice(1).split('.')[0]) >= 20
+const isGreaterThanv20 = gte(process.version.slice(1), '20.0.0')
+// NOTE: node versions <16.14.1 have a bug which changes the order of pseudo-headers
+// https://github.com/nodejs/node/pull/41735
+const hasPseudoHeadersOrderFix = gte(process.version.slice(1), '16.14.1')
 
-plan(19)
+plan(20)
 
 test('Should support H2 connection', async t => {
   const body = []
   const server = createSecureServer(pem)
 
-  server.on('stream', (stream, headers) => {
+  server.on('stream', (stream, headers, _flags, rawHeaders) => {
     t.equal(headers['x-my-header'], 'foo')
     t.equal(headers[':method'], 'GET')
     stream.respond({
@@ -996,3 +1000,49 @@ test('Agent should support H2 connection', async t => {
   t.equal(response.headers['x-custom-h2'], 'hello')
   t.equal(Buffer.concat(body).toString('utf8'), 'hello h2!')
 })
+
+test(
+  'Should provide pseudo-headers in proper order',
+  { skip: !hasPseudoHeadersOrderFix },
+  async t => {
+    const server = createSecureServer(pem)
+    server.on('stream', (stream, _headers, _flags, rawHeaders) => {
+      t.same(rawHeaders, [
+        ':authority',
+        `localhost:${server.address().port}`,
+        ':method',
+        'GET',
+        ':path',
+        '/',
+        ':scheme',
+        'https'
+      ])
+
+      stream.respond({
+        'content-type': 'text/plain; charset=utf-8',
+        ':status': 200
+      })
+      stream.end()
+    })
+
+    server.listen(0)
+    await once(server, 'listening')
+
+    const client = new Client(`https://localhost:${server.address().port}`, {
+      connect: {
+        rejectUnauthorized: false
+      },
+      allowH2: true
+    })
+
+    t.teardown(server.close.bind(server))
+    t.teardown(client.close.bind(client))
+
+    const response = await client.request({
+      path: '/',
+      method: 'GET'
+    })
+
+    t.equal(response.statusCode, 200)
+  }
+)