Skip to content

Commit c32bab2

Browse files
authored
feat: accept absolute-form URI for request-target (#232)
* feat: accept absolute-form for request-target * refactor: use :scheme for request URL's scheme if it's HTTP/2 * Update tests
1 parent 1ebb26e commit c32bab2

File tree

3 files changed

+147
-10
lines changed

3 files changed

+147
-10
lines changed

src/request.ts

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,20 +160,46 @@ export const newRequest = (
160160
const req = Object.create(requestPrototype)
161161
req[incomingKey] = incoming
162162

163+
const incomingUrl = incoming.url || ''
164+
165+
// handle absolute URL in request.url
166+
if (
167+
incomingUrl[0] !== '/' && // short-circuit for performance. most requests are relative URL.
168+
(incomingUrl.startsWith('http://') || incomingUrl.startsWith('https://'))
169+
) {
170+
if (incoming instanceof Http2ServerRequest) {
171+
throw new RequestError('Absolute URL for :path is not allowed in HTTP/2') // RFC 9113 8.3.1.
172+
}
173+
174+
try {
175+
const url = new URL(incomingUrl)
176+
req[urlKey] = url.href
177+
} catch (e) {
178+
throw new RequestError('Invalid absolute URL', { cause: e })
179+
}
180+
181+
return req
182+
}
183+
184+
// Otherwise, relative URL
163185
const host =
164186
(incoming instanceof Http2ServerRequest ? incoming.authority : incoming.headers.host) ||
165187
defaultHostname
166188
if (!host) {
167189
throw new RequestError('Missing host header')
168190
}
169-
const url = new URL(
170-
`${
171-
incoming instanceof Http2ServerRequest ||
172-
(incoming.socket && (incoming.socket as TLSSocket).encrypted)
173-
? 'https'
174-
: 'http'
175-
}://${host}${incoming.url}`
176-
)
191+
192+
let scheme: string
193+
if (incoming instanceof Http2ServerRequest) {
194+
scheme = incoming.scheme
195+
if (!(scheme === 'http' || scheme === 'https')) {
196+
throw new RequestError('Unsupported scheme')
197+
}
198+
} else {
199+
scheme = incoming.socket && (incoming.socket as TLSSocket).encrypted ? 'https' : 'http'
200+
}
201+
202+
const url = new URL(`${scheme}://${host}${incomingUrl}`)
177203

178204
// check by length for performance.
179205
// if suspicious, check by host. host header sometimes contains port.

test/request.test.ts

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { IncomingMessage } from 'node:http'
2+
import type { ServerHttp2Stream } from 'node:http2'
3+
import { Http2ServerRequest } from 'node:http2'
24
import { Socket } from 'node:net'
5+
import { Duplex } from 'node:stream'
36
import {
47
newRequest,
58
Request as LightweightRequest,
@@ -130,7 +133,7 @@ describe('Request', () => {
130133
}).toThrow(RequestError)
131134
})
132135

133-
it('Should be create request body from `req.rawBody` if it exists', async () => {
136+
it('Should be created request body from `req.rawBody` if it exists', async () => {
134137
const rawBody = Buffer.from('foo')
135138
const socket = new Socket()
136139
const incomingMessage = new IncomingMessage(socket)
@@ -152,6 +155,110 @@ describe('Request', () => {
152155
const text = await req.text()
153156
expect(text).toBe('foo')
154157
})
158+
159+
describe('absolute-form for request-target', () => {
160+
it('should be created from valid absolute URL', async () => {
161+
const req = newRequest({
162+
url: 'http://localhost/path/to/file.html',
163+
} as IncomingMessage)
164+
expect(req).toBeInstanceOf(GlobalRequest)
165+
expect(req.url).toBe('http://localhost/path/to/file.html')
166+
})
167+
168+
it('should throw error if host header is invalid', async () => {
169+
expect(() => {
170+
newRequest({
171+
url: 'http://',
172+
} as IncomingMessage)
173+
}).toThrow(RequestError)
174+
})
175+
176+
it('should throw error if absolute-form is specified via HTTP/2', async () => {
177+
expect(() => {
178+
newRequest(
179+
new Http2ServerRequest(
180+
new Duplex() as ServerHttp2Stream,
181+
{
182+
':scheme': 'http',
183+
':authority': 'localhost',
184+
':path': 'http://localhost/foo.txt',
185+
},
186+
{},
187+
[]
188+
)
189+
)
190+
}).toThrow(RequestError)
191+
})
192+
})
193+
194+
describe('HTTP/2', () => {
195+
it('should be created from "http" scheme', async () => {
196+
const req = newRequest(
197+
new Http2ServerRequest(
198+
new Duplex() as ServerHttp2Stream,
199+
{
200+
':scheme': 'http',
201+
':authority': 'localhost',
202+
':path': '/foo.txt',
203+
},
204+
{},
205+
[]
206+
)
207+
)
208+
expect(req).toBeInstanceOf(GlobalRequest)
209+
expect(req.url).toBe('http://localhost/foo.txt')
210+
})
211+
212+
it('should be created from "https" scheme', async () => {
213+
const req = newRequest(
214+
new Http2ServerRequest(
215+
new Duplex() as ServerHttp2Stream,
216+
{
217+
':scheme': 'https',
218+
':authority': 'localhost',
219+
':path': '/foo.txt',
220+
},
221+
{},
222+
[]
223+
)
224+
)
225+
expect(req).toBeInstanceOf(GlobalRequest)
226+
expect(req.url).toBe('https://localhost/foo.txt')
227+
})
228+
229+
it('should throw error if scheme is missing', async () => {
230+
expect(() => {
231+
newRequest(
232+
new Http2ServerRequest(
233+
new Duplex() as ServerHttp2Stream,
234+
{
235+
':authority': 'localhost',
236+
':path': '/foo.txt',
237+
},
238+
{},
239+
[]
240+
)
241+
)
242+
}).toThrow(RequestError)
243+
})
244+
245+
it('should throw error if unsupported scheme is specified', async () => {
246+
expect(() => {
247+
newRequest(
248+
new Http2ServerRequest(
249+
new Duplex() as ServerHttp2Stream,
250+
{
251+
':scheme': 'ftp',
252+
':authority': 'localhost',
253+
':path': '/foo.txt',
254+
},
255+
{},
256+
[]
257+
)
258+
)
259+
}).toThrow(RequestError)
260+
})
261+
})
155262
})
156263

157264
describe('GlobalRequest', () => {

test/server.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,11 @@ describe('HTTP2', () => {
640640

641641
// Use :authority as the host for the url.
642642
it('Should return 200 response - GET /url', async () => {
643-
const res = await request(server, { http2: true }).get('/url').trustLocalhost()
643+
const res = await request(server, { http2: true })
644+
.get('/url')
645+
.set(':scheme', 'https')
646+
.set(':authority', '127.0.0.1')
647+
.trustLocalhost()
644648
expect(res.status).toBe(200)
645649
expect(res.headers['content-type']).toMatch('text/plain')
646650
const url = new URL(res.text)

0 commit comments

Comments
 (0)