Skip to content

Commit

Permalink
remove http2 status pseudo header from headers (#2438)
Browse files Browse the repository at this point in the history
* remove http2 status pseudo header from headers

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

fixup

* add test

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

---------

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>
  • Loading branch information
KhafraDev and tsctx committed Nov 19, 2023
1 parent 6631655 commit 90831ae
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 4 deletions.
4 changes: 3 additions & 1 deletion lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,9 @@ function writeH2 (client, session, request) {
++h2State.openStreams

stream.once('response', headers => {
if (request.onHeaders(Number(headers[HTTP2_HEADER_STATUS]), headers, stream.resume.bind(stream), '') === false) {
const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers

if (request.onHeaders(Number(statusCode), realHeaders, stream.resume.bind(stream), '') === false) {
stream.pause()
}
})
Expand Down
42 changes: 40 additions & 2 deletions test/fetch/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ const { Readable } = require('node:stream')
const { test, plan } = require('tap')
const pem = require('https-pem')

const { Client, fetch } = require('../..')
const { Client, fetch, Headers } = require('../..')

const nodeVersion = Number(process.version.split('v')[1].split('.')[0])

plan(6)
plan(7)

test('[Fetch] Issue#2311', async t => {
const expectedBody = 'hello from client!'
Expand Down Expand Up @@ -375,3 +375,41 @@ test(
t.equal(Buffer.concat(requestChunks).toString('utf-8'), expectedBody)
}
)

test('Issue#2415', async (t) => {
t.plan(1)
const server = createSecureServer(pem)

server.on('stream', async (stream, headers) => {
stream.respond({
':status': 200
})
stream.end('test')
})

server.listen()
await once(server, 'listening')

const client = new Client(`https://localhost:${server.address().port}`, {
connect: {
rejectUnauthorized: false
},
allowH2: true
})

const response = await fetch(
`https://localhost:${server.address().port}/`,
// Needs to be passed to disable the reject unauthorized
{
method: 'GET',
dispatcher: client
}
)

await response.text()

t.teardown(server.close.bind(server))
t.teardown(client.close.bind(client))

t.doesNotThrow(() => new Headers(response.headers))
})
37 changes: 36 additions & 1 deletion test/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const isGreaterThanv20 = gte(process.version.slice(1), '20.0.0')
// https://github.com/nodejs/node/pull/41735
const hasPseudoHeadersOrderFix = gte(process.version.slice(1), '16.14.1')

plan(22)
plan(23)

test('Should support H2 connection', async t => {
const body = []
Expand Down Expand Up @@ -1154,3 +1154,38 @@ test(
t.equal(response.statusCode, 200)
}
)

test('The h2 pseudo-headers is not included in the headers', async t => {
const server = createSecureServer(pem)

server.on('stream', (stream, headers) => {
stream.respond({
':status': 200
})
stream.end('hello h2!')
})

server.listen(0)
await once(server, 'listening')

const client = new Client(`https://localhost:${server.address().port}`, {
connect: {
rejectUnauthorized: false
},
allowH2: true
})

t.plan(2)
t.teardown(server.close.bind(server))
t.teardown(client.close.bind(client))

const response = await client.request({
path: '/',
method: 'GET'
})

await response.body.text()

t.equal(response.statusCode, 200)
t.equal(response.headers[':status'], undefined)
})

0 comments on commit 90831ae

Please sign in to comment.