Skip to content

Commit e23167b

Browse files
metcoder95Uzlopak
andauthored
fix(h2): adjust :scheme on h2 requests (#4454)
* fix(h2): adjust :scheme on h2 requests * fix: recurrent origin for client * Apply suggestion from @Uzlopak Co-authored-by: Aras Abbasi <aras.abbasi@googlemail.com> * Apply suggestion from @Uzlopak Co-authored-by: Aras Abbasi <aras.abbasi@googlemail.com> * fix: typo * refactor: implement custom protocol function --------- Co-authored-by: Aras Abbasi <aras.abbasi@googlemail.com>
1 parent ad4d547 commit e23167b

6 files changed

Lines changed: 49 additions & 11 deletions

File tree

lib/core/request.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ const {
1717
serializePathWithQuery,
1818
assertRequestHandler,
1919
getServerName,
20-
normalizedMethodRecords
20+
normalizedMethodRecords,
21+
getProtocolFromUrlString
2122
} = require('./util')
2223
const { channels } = require('./diagnostics.js')
2324
const { headerNameLowerCasedRecord } = require('./constants')
@@ -141,8 +142,11 @@ class Request {
141142

142143
this.path = query ? serializePathWithQuery(path, query) : path
143144

145+
// TODO: shall we maybe standardize it to an URL object?
144146
this.origin = origin
145147

148+
this.protocol = getProtocolFromUrlString(origin)
149+
146150
this.idempotent = idempotent == null
147151
? method === 'HEAD' || method === 'GET'
148152
: idempotent

lib/core/util.js

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,30 @@ function onConnectTimeout (socket, opts) {
876876
destroy(socket, new ConnectTimeoutError(message))
877877
}
878878

879+
/**
880+
* @param {string} urlString
881+
* @returns {string}
882+
*/
883+
function getProtocolFromUrlString (urlString) {
884+
if (
885+
urlString[0] === 'h' &&
886+
urlString[1] === 't' &&
887+
urlString[2] === 't' &&
888+
urlString[3] === 'p'
889+
) {
890+
switch (urlString[4]) {
891+
case ':':
892+
return 'http:'
893+
case 's':
894+
if (urlString[5] === ':') {
895+
return 'https:'
896+
}
897+
}
898+
}
899+
// fallback if none of the usual suspects
900+
return urlString.slice(0, urlString.indexOf(':') + 1)
901+
}
902+
879903
const kEnumerableProperty = Object.create(null)
880904
kEnumerableProperty.enumerable = true
881905

@@ -947,5 +971,6 @@ module.exports = {
947971
nodeMinor,
948972
safeHTTPMethods: Object.freeze(['GET', 'HEAD', 'OPTIONS', 'TRACE']),
949973
wrapRequestBody,
950-
setupConnectTimeout
974+
setupConnectTimeout,
975+
getProtocolFromUrlString
951976
}

lib/dispatcher/client-h2.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ function shouldSendContentLength (method) {
279279
function writeH2 (client, request) {
280280
const requestTimeout = request.bodyTimeout ?? client[kBodyTimeout]
281281
const session = client[kHTTP2Session]
282-
const { method, path, host, upgrade, expectContinue, signal, headers: reqHeaders } = request
282+
const { method, path, host, upgrade, expectContinue, signal, protocol, headers: reqHeaders } = request
283283
let { body } = request
284284

285285
if (upgrade) {
@@ -397,7 +397,7 @@ function writeH2 (client, request) {
397397
// :path and :scheme headers must be omitted when sending CONNECT
398398

399399
headers[HTTP2_HEADER_PATH] = path
400-
headers[HTTP2_HEADER_SCHEME] = 'https'
400+
headers[HTTP2_HEADER_SCHEME] = protocol === 'http:' ? 'http' : 'https'
401401

402402
// https://tools.ietf.org/html/rfc7231#section-4.3.1
403403
// https://tools.ietf.org/html/rfc7231#section-4.3.2

lib/dispatcher/client.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,7 @@ class Client extends DispatcherBase {
296296
}
297297

298298
[kDispatch] (opts, handler) {
299-
const origin = opts.origin || this[kUrl].origin
300-
const request = new Request(origin, opts, handler)
299+
const request = new Request(this[kUrl].origin, opts, handler)
301300

302301
this[kQueue].push(request)
303302
if (this[kResuming]) {

test/h2c-client.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,22 @@ test('Should throw if pipelining greather than concurrent streams', async t => {
2626
})
2727

2828
test('Should support h2c connection', async t => {
29-
const planner = tspl(t, { plan: 2 })
29+
const planner = tspl(t, { plan: 6 })
30+
let authority = ''
3031

3132
const server = createServer((req, res) => {
33+
planner.equal(req.headers[':authority'], authority)
34+
planner.equal(req.headers[':method'], 'GET')
35+
planner.equal(req.headers[':path'], '/')
36+
planner.equal(req.headers[':scheme'], 'http')
3237
res.writeHead(200)
3338
res.end('Hello, world!')
3439
})
3540

3641
server.listen()
3742
await once(server, 'listening')
38-
const client = new H2CClient(`http://localhost:${server.address().port}/`)
43+
authority = `localhost:${server.address().port}`
44+
const client = new H2CClient(`http://${authority}/`)
3945

4046
t.after(() => client.close())
4147
t.after(() => server.close())

test/http2.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,14 @@ const { Client, Agent, FormData, Response } = require('..')
1414
test('Should support H2 connection', async t => {
1515
const body = []
1616
const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))
17+
let authority = ''
1718

1819
server.on('stream', (stream, headers, _flags, rawHeaders) => {
1920
t.strictEqual(headers['x-my-header'], 'foo')
2021
t.strictEqual(headers[':method'], 'GET')
22+
t.strictEqual(headers[':scheme'], 'https')
23+
t.strictEqual(headers[':path'], '/')
24+
t.strictEqual(headers[':authority'], authority)
2125
stream.respond({
2226
'content-type': 'text/plain; charset=utf-8',
2327
'x-custom-h2': 'hello',
@@ -28,15 +32,15 @@ test('Should support H2 connection', async t => {
2832

2933
server.listen(0)
3034
await once(server, 'listening')
31-
32-
const client = new Client(`https://localhost:${server.address().port}`, {
35+
authority = `localhost:${server.address().port}`
36+
const client = new Client(`https://${authority}`, {
3337
connect: {
3438
rejectUnauthorized: false
3539
},
3640
allowH2: true
3741
})
3842

39-
t = tspl(t, { plan: 6 })
43+
t = tspl(t, { plan: 9 })
4044
after(() => server.close())
4145
after(() => client.close())
4246

0 commit comments

Comments
 (0)