Skip to content

Commit

Permalink
chore: change order of the pseudo-headers (#2308)
Browse files Browse the repository at this point in the history
  • Loading branch information
kyrylodolynskyi committed Oct 5, 2023
1 parent fcc1e39 commit 898040d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 6 deletions.
11 changes: 8 additions & 3 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
56 changes: 53 additions & 3 deletions test/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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)
}
)

0 comments on commit 898040d

Please sign in to comment.