Skip to content

Commit e4e2173

Browse files
committed
fix #6 -- Multiple forward slashes in request URL cause unexpected issues
- now it is possible to proxy url's with any number of slashes with no mangling
1 parent 19aba46 commit e4e2173

File tree

4 files changed

+327
-14
lines changed

4 files changed

+327
-14
lines changed

lib/http-proxy/common.ts

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ export function urlJoin(...args: string[]): string {
169169
// join url and merge all query string.
170170
const queryParams: string[] = [];
171171
let queryParamRaw = "";
172-
let retSegs;
173172

174173
args.forEach((url, index) => {
175174
const qpStart = url.indexOf("?");
@@ -181,13 +180,34 @@ export function urlJoin(...args: string[]): string {
181180
queryParamRaw = queryParams.filter(Boolean).join("&");
182181

183182
// Join all strings, but remove empty strings so we don't get extra slashes from
184-
// joining e.g. ['', 'am']
185-
retSegs = args
186-
.filter(Boolean)
187-
.join("/")
188-
.replace(/\/+/g, "/")
189-
.replace("http:/", "http://")
190-
.replace("https:/", "https://");
183+
// joining e.g. ['', 'am'].
184+
// Also we respect strings that start and end in multiple slashes, e.g., so
185+
// ['/', '//test', '///foo'] --> '//test'
186+
// since e.g., http://localhost//test///foo is a valid URL. See
187+
// lib/test/http/double-slashes.test.ts
188+
// The algorithm for joining is just straightforward and simple, instead
189+
// of the complicated "too clever" code from http-proxy. This just concats
190+
// the strings together, not adding any slashes, and also combining adjacent
191+
// slashes in two segments, e.g., ['/foo/','/bar'] --> '/foo/bar'
192+
let retSegs = "";
193+
for (const seg of args) {
194+
if (!seg) {
195+
continue;
196+
}
197+
if (retSegs.endsWith("/")) {
198+
if (seg.startsWith("/")) {
199+
retSegs += seg.slice(1);
200+
} else {
201+
retSegs += seg;
202+
}
203+
} else {
204+
if (seg.startsWith("/")) {
205+
retSegs += seg;
206+
} else {
207+
retSegs += "/" + seg;
208+
}
209+
}
210+
}
191211

192212
// Only join the query string if it exists so we don't have trailing a '?'
193213
// on every request
@@ -244,11 +264,22 @@ export function toURL(url: URL | urllib.Url | string | undefined): URL {
244264
if (url instanceof URL) {
245265
return url;
246266
} else if (typeof url === "object" && typeof url.href === "string") {
247-
// urllib.Url is deprecated but we support it by converting to URL
248-
return new URL(url.href, "http://dummy.org");
249-
} else {
250-
return new URL(url ?? "", "http://dummy.org");
267+
url = url.href;
268+
}
269+
if (!url) {
270+
url = "";
271+
}
272+
if (typeof url != "string") {
273+
// it has to be a string at this point, but to keep typescript happy:
274+
url = `${url}`;
275+
}
276+
if (url.startsWith("//")) {
277+
// special case -- this would be viewed as a this is a "network-path reference",
278+
// so we explicitly prefix with our http schema. See
279+
url = `http://dummy.org${url}`;
251280
}
281+
// urllib.Url is deprecated but we support it by converting to URL
282+
return new URL(url, "http://dummy.org");
252283
}
253284

254285
// vendor simplified version of https://www.npmjs.com/package/requires-port to

lib/test/http/double-slashes.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import express from "express";
2+
import getPort from "../get-port";
3+
import { createServer } from "../..";
4+
import http from "http";
5+
6+
// See https://github.com/sagemathinc/http-proxy-3/issues/6
7+
8+
describe("test multiple forward slashes in a URL", () => {
9+
let port, server;
10+
11+
it("create a simple http server", async () => {
12+
port = await getPort();
13+
const app = express();
14+
15+
app.get("/", (_req, res) => {
16+
res.send("Hello World!");
17+
});
18+
19+
app.get("/test", (_req, res) => {
20+
res.send("Test Page!");
21+
});
22+
23+
app.get("/test/foo", (_req, res) => {
24+
res.send("Test Foo Page!");
25+
});
26+
27+
app.get("//test", (_req, res) => {
28+
res.send("double slash test Page!");
29+
});
30+
31+
app.get("///test/crazy//slasher", (_req, res) => {
32+
res.send("crazy slasher");
33+
});
34+
35+
server = app.listen(port);
36+
});
37+
38+
const get = async (url) => {
39+
return await (await fetch(`http://localhost:${port}${url}`)).text();
40+
};
41+
42+
it("establish what the correct behavior is", async () => {
43+
expect(await get("")).toBe("Hello World!");
44+
expect(await get("/test")).toBe("Test Page!");
45+
expect(await get("//test")).toBe("double slash test Page!");
46+
expect(await get("/test/foo")).toContain("Test Foo Page!");
47+
expect(await get("/test//foo")).toContain("Cannot GET /test//foo");
48+
expect(await get("///test/crazy//slasher")).toBe("crazy slasher");
49+
});
50+
51+
let proxy, httpServer;
52+
let proxyPort;
53+
it("create a proxy server", async () => {
54+
proxy = createServer();
55+
proxy.on("error", (err, _req, res) => {
56+
console.error("Proxy error:", err);
57+
res.end("Something went wrong.");
58+
});
59+
httpServer = http.createServer((req, res) => {
60+
const target = `http://localhost:${port}`;
61+
proxy.web(req, res, { target });
62+
});
63+
64+
proxyPort = await getPort();
65+
httpServer.listen(proxyPort);
66+
});
67+
68+
const getProxy = async (url) => {
69+
return await (await fetch(`http://localhost:${proxyPort}${url}`)).text();
70+
};
71+
72+
it("get using the proxy instead -- the behavior is identical to directly using the server", async () => {
73+
expect(await getProxy("")).toBe("Hello World!");
74+
expect(await getProxy("/test")).toBe("Test Page!");
75+
expect(await getProxy("//test")).toBe("double slash test Page!");
76+
expect(await getProxy("/test/foo")).toContain("Test Foo Page!");
77+
expect(await getProxy("/test//foo")).toContain("Cannot GET /test//foo");
78+
expect(await getProxy("///test/crazy//slasher")).toBe("crazy slasher");
79+
});
80+
81+
it("clean up", () => {
82+
server.close();
83+
httpServer.close();
84+
});
85+
});

package.json

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "http-proxy-3",
3-
"version": "1.20.5",
3+
"version": "1.20.6",
44
"repository": {
55
"type": "git",
66
"url": "https://github.com/sagemathinc/http-proxy-3.git"
@@ -13,7 +13,11 @@
1313
"jcrugzz <jcrugzz@gmail.com>"
1414
],
1515
"main": "dist/lib/index.js",
16-
"files": ["dist/lib/http-proxy", "dist/lib/index.js", "dist/lib/index.d.ts"],
16+
"files": [
17+
"dist/lib/http-proxy",
18+
"dist/lib/index.js",
19+
"dist/lib/index.d.ts"
20+
],
1721
"dependencies": {
1822
"debug": "^4.4.0",
1923
"follow-redirects": "^1.15.9"
@@ -32,6 +36,7 @@
3236
"connect": "^3.7.0",
3337
"eventsource": "^3.0.7",
3438
"expect.js": "~0.3.1",
39+
"express": "^5.1.0",
3540
"get-port": "^7.1.0",
3641
"https-proxy-agent": "^7.0.6",
3742
"jest": "^29.7.0",

0 commit comments

Comments
 (0)