Skip to content

Commit 63c66ad

Browse files
dinwwwhxia-chao
andauthored
fix(node): parse absolute-form request targets (#85)
HTTP/1.1 clients that treat the server as a forward proxy (`curl -x <server> <url>`, `HTTP_PROXY` pointed at the app, some intermediaries) send the request target in absolute-form, `GET http://host:port/ping HTTP/1.1`, and Node hands that string through as `req.url` verbatim. `toStandardUrl` prefixed it with `/`, so routers saw `/http://host:port/ping` and answered 404. Absolute-form targets are now parsed with `URL` and reduced to pathname + search + hash through the Fetch adapter's `toStandardUrl`, so the Node and Fetch adapters yield the same `StandardUrl` for the same request. Supersedes #81. Thanks @javascript-unsafe for the report and the initial fix. ## Fixes - `GET http://127.0.0.1:3000/ping?x=1` against a real `node:http` server now yields `/ping?x=1`. The Fastify adapter inherits this through `req.raw`. - Origin-form paths are still passed through untouched: no normalization, and `//evil.com/x` stays a path rather than a host. - Unparseable absolute-form input (`http://`, `http://[::1`, an out-of-range port) keeps the previous `/${url}` fallback instead of throwing. ## Testing - Unit tests cover origin-form, `originalUrl` precedence, absolute-form, normalization parity with the Fetch adapter, non-http schemes, and malicious or malformed input ported from oRPC's `standard-server-node` tests. - One test sends absolute-form over a raw socket to a live `node:http` server. - Full suite, eslint, and `tsc -b` pass. Co-authored-by: Xia Chao <236466140+bun-unsafe@users.noreply.github.com>
1 parent f13e415 commit 63c66ad

2 files changed

Lines changed: 99 additions & 7 deletions

File tree

‎packages/node/src/url.test.ts‎

Lines changed: 86 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,89 @@
1+
import type { AddressInfo } from 'node:net'
2+
import http from 'node:http'
3+
import net from 'node:net'
14
import { toStandardUrl } from './url'
25

3-
it('toStandardUrl', () => {
4-
expect(toStandardUrl({ } as any)).toBe('/')
5-
expect(toStandardUrl({ url: '/foo' } as any)).toBe('/foo')
6-
expect(toStandardUrl({ url: '/foo?bar=1#baz' } as any)).toBe('/foo?bar=1#baz')
7-
expect(toStandardUrl({ url: '/', originalUrl: '/foo?bar=2#baz' } as any)).toBe('/foo?bar=2#baz')
8-
expect(toStandardUrl({ url: 'base' } as any)).toBe('/base')
6+
describe('toStandardUrl', () => {
7+
it('origin-form', () => {
8+
expect(toStandardUrl({ } as any)).toBe('/')
9+
expect(toStandardUrl({ url: '/' } as any)).toBe('/')
10+
expect(toStandardUrl({ url: '/foo' } as any)).toBe('/foo')
11+
expect(toStandardUrl({ url: '/foo?bar=1#baz' } as any)).toBe('/foo?bar=1#baz')
12+
// asterisk-form (`OPTIONS *`)
13+
expect(toStandardUrl({ url: '*' } as any)).toBe('/*')
14+
})
15+
16+
it('prefers originalUrl over url', () => {
17+
expect(toStandardUrl({ url: '/', originalUrl: '/foo?bar=2#baz' } as any)).toBe('/foo?bar=2#baz')
18+
expect(toStandardUrl({ url: '/', originalUrl: 'http://127.0.0.1:80/foo?x=1' } as any)).toBe('/foo?x=1')
19+
})
20+
21+
it('absolute-form (RFC 9112 §3.2.2, sent by clients that treat the server as a proxy)', () => {
22+
expect(toStandardUrl({ url: 'http://127.0.0.1:3000/ping' } as any)).toBe('/ping')
23+
expect(toStandardUrl({ url: 'http://example.com/foo?bar=1' } as any)).toBe('/foo?bar=1')
24+
expect(toStandardUrl({ url: 'https://example.com/foo#h' } as any)).toBe('/foo#h')
25+
expect(toStandardUrl({ url: 'HTTP://EXAMPLE.COM/Foo' } as any)).toBe('/Foo')
26+
expect(toStandardUrl({ url: 'http://example.com' } as any)).toBe('/')
27+
expect(toStandardUrl({ url: 'http://example.com?x=1' } as any)).toBe('/?x=1')
28+
expect(toStandardUrl({ url: 'http://example.com#f' } as any)).toBe('/#f')
29+
expect(toStandardUrl({ url: 'http://user:pw@example.com:8080/p' } as any)).toBe('/p')
30+
expect(toStandardUrl({ url: 'http://[::1]:3000/p' } as any)).toBe('/p')
31+
})
32+
33+
it('reduces non-http schemes to their path', () => {
34+
expect(toStandardUrl({ url: 'ws://example.com/socket' } as any)).toBe('/socket')
35+
expect(toStandardUrl({ url: 'ftp://example.com/file' } as any)).toBe('/file')
36+
})
37+
38+
it('treats anything else as a relative path', () => {
39+
expect(toStandardUrl({ url: 'base' } as any)).toBe('/base')
40+
expect(toStandardUrl({ url: '' } as any)).toBe('/')
41+
expect(toStandardUrl({ url: '?x=1' } as any)).toBe('/?x=1')
42+
expect(toStandardUrl({ url: '#frag' } as any)).toBe('/#frag')
43+
})
44+
45+
it('malicious or malformed input', () => {
46+
// origin-form is passed through untouched: never resolved as a host, never normalized
47+
expect(toStandardUrl({ url: '//evil.com/ping' } as any)).toBe('//evil.com/ping')
48+
expect(toStandardUrl({ url: '////' } as any)).toBe('////')
49+
expect(toStandardUrl({ url: '/../../etc/passwd' } as any)).toBe('/../../etc/passwd')
50+
expect(toStandardUrl({ url: '/%2e%2e/x' } as any)).toBe('/%2e%2e/x')
51+
expect(toStandardUrl({ url: ':::' } as any)).toBe('/:::')
52+
53+
// authority tricks in absolute-form never leak into the path
54+
expect(toStandardUrl({ url: 'http://good.com@evil.com/x' } as any)).toBe('/x')
55+
expect(toStandardUrl({ url: 'http://evil.com\\@good.com/x' } as any)).toBe('/@good.com/x')
56+
expect(toStandardUrl({ url: 'http://example.com/../../etc/passwd' } as any)).toBe('/etc/passwd')
57+
expect(toStandardUrl({ url: 'http://example.com/a\tb\n' } as any)).toBe('/ab')
58+
expect(toStandardUrl({ url: 'javascript:alert(1)' } as any)).toBe('/alert(1)')
59+
60+
// unparseable absolute-form keeps the legacy `/${url}` behavior instead of throwing
61+
expect(toStandardUrl({ url: 'http://' } as any)).toBe('/http://')
62+
expect(toStandardUrl({ url: 'http://[::1' } as any)).toBe('/http://[::1')
63+
expect(toStandardUrl({ url: 'http://example.com:99999/x' } as any)).toBe('/http://example.com:99999/x')
64+
})
65+
66+
it('absolute-form from a real node:http server (what `curl -x <server> <url>` sends)', async ({ onTestFinished }) => {
67+
let url: string | undefined
68+
69+
const server = http.createServer((req, res) => {
70+
url = toStandardUrl(req)
71+
res.end()
72+
})
73+
onTestFinished(() => new Promise<any>(r => server.close(r)))
74+
75+
await new Promise<void>(r => server.listen(0, r))
76+
const { port } = server.address() as AddressInfo
77+
78+
await new Promise<void>((resolve, reject) => {
79+
const socket = net.connect(port, '127.0.0.1', () => {
80+
socket.end(`GET http://127.0.0.1:${port}/ping?x=1 HTTP/1.1\r\nHost: 127.0.0.1:${port}\r\nConnection: close\r\n\r\n`)
81+
})
82+
socket.resume()
83+
socket.on('close', resolve)
84+
socket.on('error', reject)
85+
})
86+
87+
expect(url).toBe('/ping?x=1')
88+
})
989
})

‎packages/node/src/url.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
11
import type { StandardUrl } from '@standardserver/core'
22
import type { NodeHttpRequest } from './types'
3+
import { toStandardUrl as toStandardUrlFetch } from '@standardserver/fetch'
34

45
export function toStandardUrl(req: NodeHttpRequest): StandardUrl {
56
// prefer originalUrl over url, especially useful in express.js middleware
67
const url = req.originalUrl ?? req.url ?? '/'
7-
return `${url.startsWith('/') ? '' : '/'}${url}` as `/${string}`
8+
9+
if (url.startsWith('/')) {
10+
return url as `/${string}`
11+
}
12+
13+
try {
14+
const parsed = new URL(url, 'http://localhost')
15+
return toStandardUrlFetch(parsed)
16+
}
17+
catch {
18+
return `/${url}`
19+
}
820
}

0 commit comments

Comments
 (0)