Skip to content

Commit 8dea99e

Browse files
daquinoaldodaquinoaldo
authored
fix: crash and RFC compliance edge cases (#121)
## Summary Fixes all ten crash, hang, RFC-compliance, proxy, and response-quality bugs found in the review. Each fix is a separate commit; the edge-case test suite lands first. - **Malformed percent-encoding crashed the server**: `/%zz` threw `URIError`; now rejected safely. (`904a911`) - **Directory without `index.html` crashed the server**: now serves 404. (`f6209f7`) - **`If-Modified-Since` alone never produced 304**: now honored per RFC 9110. (`f407c96`) - **Upstream dying mid-response hung clients**: client socket now closes. (`9db8e12`) - **Proxy CORS preflight hit the upstream**: OPTIONS now returns 204 directly. (`98305ef`) - **Packaged update check could raise an uncaught callback error**: callback failures are contained. (`b3f8b15`) - **Multi-range requests incorrectly returned 416**: first range is served. (`1f0ab67`) - **WebSocket upgrades were unsupported**: HTTP and HTTPS upstream upgrade tunneling added. (`28183b9`) - **Existing `X-Forwarded-For` was overwritten**: client address is appended. (`330e42d`) - **Static 400/403/405/416 responses had empty bodies**: descriptive text bodies added. (`e998c05`) ## Verification - Edge-case suite and full tests pass where ports are available. - `pnpm lint` passes. - The two redirect tests are blocked locally by an unrelated `node server.js` process occupying port 8080. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
1 parent 9718252 commit 8dea99e

7 files changed

Lines changed: 276 additions & 32 deletions

File tree

‎src/certs.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,17 @@ function checkUpdates(): void {
3737
body += chunk.toString("utf8")
3838
})
3939
res.on("end", () => {
40-
const currentVersion: unknown = JSON.parse(
41-
fs.readFileSync(path.resolve(__dirname, "../package.json"), "utf8"),
42-
)
43-
const latestVersion: unknown = JSON.parse(body)
44-
if (!isRelease(latestVersion)) return
45-
const current = isVersioned(currentVersion) ? (currentVersion.version ?? "") : ""
46-
if (current !== latestVersion.tag_name.replace("v", "")) {
47-
console.warn("[https-localhost] New update available.")
48-
}
40+
try {
41+
const currentVersion: unknown = JSON.parse(
42+
fs.readFileSync(path.resolve(__dirname, "../package.json"), "utf8"),
43+
)
44+
const latestVersion: unknown = JSON.parse(body)
45+
if (!isRelease(latestVersion)) return
46+
const current = isVersioned(currentVersion) ? (currentVersion.version ?? "") : ""
47+
if (current !== latestVersion.tag_name.replace("v", "")) {
48+
console.warn("[https-localhost] New update available.")
49+
}
50+
} catch {}
4951
})
5052
})
5153
.end()

‎src/index.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { Server } from "node:http"
33
import https from "node:https"
44

55
import { getCerts } from "./certs.ts"
6-
import { createProxyHandler } from "./proxy.ts"
6+
import { createProxyHandler, createProxyUpgradeHandler } from "./proxy.ts"
77
import { createRouter } from "./router.ts"
88
import { createStaticHandler } from "./static.ts"
99

@@ -31,6 +31,9 @@ export function createServer({
3131
async listen(port = 443) {
3232
const certs = await getCerts({ domain, certPath, reinstall })
3333
app.server = https.createServer(certs, router.handleRequest)
34+
if (router.proxyUpgradeHandler !== undefined) {
35+
app.server.on("upgrade", router.proxyUpgradeHandler)
36+
}
3437
await new Promise<void>(resolve => {
3538
app.server?.listen(port, resolve)
3639
})
@@ -39,6 +42,7 @@ export function createServer({
3942
},
4043
async proxy(target, port = 443) {
4144
router.setProxyHandler(createProxyHandler(target))
45+
router.setProxyUpgradeHandler(createProxyUpgradeHandler(target))
4246
console.info(`Proxying to ${target}`)
4347
await app.listen(port)
4448
return app

‎src/proxy.ts‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
ServerResponse,
77
} from "node:http"
88
import https from "node:https"
9+
import type { Duplex } from "node:stream"
910

1011
const HOP_BY_HOP_HEADERS: ReadonlySet<string> = new Set([
1112
"connection",
@@ -36,10 +37,20 @@ export function createProxyHandler(target: string): RequestListener {
3637
const port = url.port === "" ? (url.protocol === "https:" ? 443 : 80) : Number(url.port)
3738

3839
return function handleProxy(req: IncomingMessage, res: ServerResponse): void {
40+
if (req.method === "OPTIONS") {
41+
res.writeHead(204)
42+
res.end()
43+
return
44+
}
3945
const headers = filterHeaders(req.headers)
4046
headers.host = url.host
41-
if (req.socket.remoteAddress !== undefined)
42-
headers["x-forwarded-for"] = req.socket.remoteAddress
47+
if (req.socket.remoteAddress !== undefined) {
48+
const forwardedFor = req.headers["x-forwarded-for"]
49+
headers["x-forwarded-for"] =
50+
forwardedFor === undefined
51+
? req.socket.remoteAddress
52+
: `${forwardedFor}, ${req.socket.remoteAddress}`
53+
}
4354
headers["x-forwarded-proto"] =
4455
(req.socket as { encrypted?: boolean }).encrypted === true ? "https" : "http"
4556
if (req.headers.host !== undefined) headers["x-forwarded-host"] = req.headers.host
@@ -55,6 +66,9 @@ export function createProxyHandler(target: string): RequestListener {
5566
},
5667
upstreamRes => {
5768
res.writeHead(upstreamRes.statusCode ?? 502, filterHeaders(upstreamRes.headers))
69+
upstreamRes.on("error", () => {
70+
res.destroy()
71+
})
5872
upstreamRes.pipe(res)
5973
},
6074
)
@@ -72,3 +86,42 @@ export function createProxyHandler(target: string): RequestListener {
7286
req.pipe(upstream)
7387
}
7488
}
89+
90+
export function createProxyUpgradeHandler(
91+
target: string,
92+
): (req: IncomingMessage, socket: Duplex, head: Buffer) => void {
93+
const url = new URL(target)
94+
if (url.protocol !== "http:" && url.protocol !== "https:") {
95+
throw new Error(`Unsupported proxy protocol: ${url.protocol}. Use http or https.`)
96+
}
97+
const transport = url.protocol === "https:" ? https : http
98+
const port = url.port === "" ? (url.protocol === "https:" ? 443 : 80) : Number(url.port)
99+
return function handleUpgrade(req, clientSocket, head): void {
100+
const headers = filterHeaders(req.headers)
101+
headers.host = url.host
102+
const upstream = transport.request({
103+
protocol: url.protocol,
104+
hostname: url.hostname,
105+
port,
106+
path: req.url,
107+
method: req.method,
108+
headers,
109+
})
110+
upstream.on("upgrade", (upstreamRes, upstreamSocket, upstreamHead) => {
111+
clientSocket.write(
112+
`HTTP/1.1 ${upstreamRes.statusCode ?? 101} ${upstreamRes.statusMessage ?? "Switching Protocols"}\r\n`,
113+
)
114+
for (const [name, value] of Object.entries(upstreamRes.headers)) {
115+
if (value === undefined) continue
116+
const values = Array.isArray(value) ? value : [value]
117+
for (const item of values) clientSocket.write(`${name}: ${item}\r\n`)
118+
}
119+
clientSocket.write("\r\n")
120+
if (upstreamHead.length > 0) clientSocket.write(upstreamHead)
121+
if (head.length > 0) upstreamSocket.write(head)
122+
clientSocket.pipe(upstreamSocket).pipe(clientSocket)
123+
})
124+
upstream.on("error", () => clientSocket.destroy())
125+
upstream.end()
126+
}
127+
}

‎src/router.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
import type { IncomingMessage, RequestListener, ServerResponse } from "node:http"
2+
import type { Duplex } from "node:stream"
23

34
import { applyCors } from "./cors.ts"
45

56
export function createRouter(): {
67
handleRequest: RequestListener
78
setProxyHandler: (handler: RequestListener | undefined) => void
9+
setProxyUpgradeHandler: (
10+
handler: ((req: IncomingMessage, socket: Duplex, head: Buffer) => void) | undefined,
11+
) => void
12+
proxyUpgradeHandler?: (req: IncomingMessage, socket: Duplex, head: Buffer) => void
813
setStaticHandler: (handler: RequestListener | undefined) => void
914
} {
1015
let proxyHandler: RequestListener | undefined
@@ -28,9 +33,25 @@ export function createRouter(): {
2833
proxyHandler = handler
2934
}
3035

36+
function setProxyUpgradeHandler(
37+
handler: ((req: IncomingMessage, socket: Duplex, head: Buffer) => void) | undefined,
38+
): void {
39+
router.proxyUpgradeHandler = handler
40+
}
41+
3142
function setStaticHandler(handler: RequestListener | undefined): void {
3243
staticHandler = handler
3344
}
3445

35-
return { handleRequest, setProxyHandler, setStaticHandler }
46+
const router = {
47+
handleRequest,
48+
setProxyHandler,
49+
setProxyUpgradeHandler,
50+
proxyUpgradeHandler: undefined as
51+
| ((req: IncomingMessage, socket: Duplex, head: Buffer) => void)
52+
| undefined,
53+
setStaticHandler,
54+
}
55+
56+
return router
3657
}

‎src/static.ts‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,20 @@ function mime(filePath: string): string {
1616

1717
function sanitize(staticPath: string, urlPath: string): string | null {
1818
const base = path.resolve(staticPath)
19-
const decoded = decodeURIComponent(urlPath.split("?")[0]?.split("#")[0] ?? "/")
19+
let decoded: string
20+
try {
21+
decoded = decodeURIComponent(urlPath.split("?")[0]?.split("#")[0] ?? "/")
22+
} catch {
23+
return null
24+
}
2025
const resolved = path.resolve(base, `.${path.posix.normalize(`/${decoded}`)}`)
2126
if (resolved !== base && !resolved.startsWith(`${base}${path.sep}`)) return null
2227
return resolved
2328
}
2429

2530
function parseRange(header: string, size: number): { start: number; end: number } | null {
26-
const match = /^bytes=(\d*)-(\d*)$/u.exec(header.trim())
31+
const firstRange = header.trim().split(",", 1)[0]
32+
const match = /^bytes=(\d*)-(\d*)$/u.exec(firstRange ?? "")
2733
if (match === null || (match[1] === "" && match[2] === "")) return null
2834
if (match[1] === "") {
2935
const n = Number(match[2])
@@ -72,14 +78,12 @@ function serveFile(
7278
return
7379
}
7480
const ifModifiedSince = req.headers["if-modified-since"]
75-
if (
76-
ifNoneMatch !== undefined &&
77-
ifModifiedSince !== undefined &&
78-
stat.mtime <= new Date(ifModifiedSince)
79-
) {
80-
res.writeHead(304)
81-
res.end()
82-
return
81+
if (ifModifiedSince !== undefined && !Number.isNaN(new Date(ifModifiedSince).getTime())) {
82+
if (stat.mtime <= new Date(ifModifiedSince)) {
83+
res.writeHead(304)
84+
res.end()
85+
return
86+
}
8387
}
8488

8589
const { size } = stat
@@ -110,21 +114,24 @@ export function createStaticHandler(staticPath: string): RequestListener {
110114
return
111115
}
112116
if (req.method !== "GET" && req.method !== "HEAD") {
113-
res.writeHead(405, { Allow: "GET, HEAD, OPTIONS" })
114-
res.end()
117+
res.writeHead(405, {
118+
Allow: "GET, HEAD, OPTIONS",
119+
"Content-Type": "text/plain; charset=utf-8",
120+
})
121+
res.end("Method not allowed.")
115122
return
116123
}
117124
const url = req.url ?? "/"
118125
if (!url.startsWith("/") || url.startsWith("//")) {
119-
res.writeHead(400)
120-
res.end()
126+
res.writeHead(400, { "Content-Type": "text/plain; charset=utf-8" })
127+
res.end("Bad request.")
121128
return
122129
}
123130
const [urlPath, query = ""] = url.split("?")
124131
const filePath = sanitize(staticPath, urlPath ?? "/")
125132
if (filePath === null) {
126-
res.writeHead(403)
127-
res.end()
133+
res.writeHead(403, { "Content-Type": "text/plain; charset=utf-8" })
134+
res.end("Forbidden.")
128135
return
129136
}
130137
let target = filePath
@@ -145,13 +152,22 @@ export function createStaticHandler(staticPath: string): RequestListener {
145152
serve404(staticPath, req, res)
146153
return
147154
}
155+
try {
156+
fs.statSync(target)
157+
} catch {
158+
serve404(staticPath, req, res)
159+
return
160+
}
148161
let range: { start: number; end: number } | null = null
149162
const rangeHeader = req.headers.range
150163
if (rangeHeader !== undefined) {
151164
range = parseRange(rangeHeader, fs.statSync(target).size)
152165
if (range === null) {
153-
res.writeHead(416, { "Content-Range": `bytes */${fs.statSync(target).size}` })
154-
res.end()
166+
res.writeHead(416, {
167+
"Content-Range": `bytes */${fs.statSync(target).size}`,
168+
"Content-Type": "text/plain; charset=utf-8",
169+
})
170+
res.end("Range not satisfiable.")
155171
return
156172
}
157173
}

0 commit comments

Comments
 (0)