Repository: unjs/h3
Pull request #846 (2024-07-23 15:43:54)

fix(getRequestURL): consistent behavior between node and web
Followup on #844

Make `getRequestURL` consistent between adapters. Now it respects URL behavior for node, which that double slashes are preserved and backslashes normalized to slash but preserved when more than one is in one segment.


Patch:
diff --git a/src/adapters/node/internal/url.ts b/src/adapters/node/internal/url.ts
index 0155677..6c1d571 100644
--- a/src/adapters/node/internal/url.ts
+++ b/src/adapters/node/internal/url.ts
@@ -34,7 +34,7 @@ export const NodeReqURLProxy = /* @__PURE__ */ (() =>
     // hostname
     get hostname() {
       if (this._hostname === undefined) {
-        const [hostname, port] = _parseHost(this[kNodeReq].headers.host);
+        const [hostname, port] = parseHost(this[kNodeReq].headers.host);
         if (this._port === undefined && port) {
           this._port = String(Number.parseInt(port) || "");
         }
@@ -49,7 +49,7 @@ export const NodeReqURLProxy = /* @__PURE__ */ (() =>
     // port
     get port() {
       if (this._port === undefined) {
-        const [hostname, port] = _parseHost(this[kNodeReq].headers.host);
+        const [hostname, port] = parseHost(this[kNodeReq].headers.host);
         if (this._hostname === undefined && hostname) {
           this._hostname = hostname;
         }
@@ -64,7 +64,7 @@ export const NodeReqURLProxy = /* @__PURE__ */ (() =>
     // pathname
     get pathname() {
       if (this._pathname === undefined) {
-        const [pathname, search] = _parsePath(this[kNodeReq].url || "/");
+        const [pathname, search] = parsePath(this[kNodeReq].url || "/");
         this._pathname = pathname;
         if (this._search === undefined) {
           this._search = search;
@@ -86,7 +86,7 @@ export const NodeReqURLProxy = /* @__PURE__ */ (() =>
     // search
     get search() {
       if (this._search === undefined) {
-        const [pathname, search] = _parsePath(this[kNodeReq].url || "/");
+        const [pathname, search] = parsePath(this[kNodeReq].url || "/");
         this._search = search;
         if (this._pathname === undefined) {
           this._pathname = pathname;
@@ -176,31 +176,16 @@ export const NodeReqURLProxy = /* @__PURE__ */ (() =>
     }
   })();
 
-function _parsePath(input: string) {
-  let pIndex: number = -1;
-  let qIndex: number = -1;
-
-  const len = input.length;
-  for (let i = 0; i < len; i++) {
-    const c = input[i];
-    if (pIndex === -1 && c !== "/" && c !== "\\") {
-      pIndex = i;
-    }
-    if (qIndex === -1 && c === "?") {
-      qIndex = i;
-    }
+function parsePath(input: string): [pathname: string, search: string] {
+  const url = (input || "/").replace(/\\/g, "/");
+  const qIndex = url.indexOf("?");
+  if (qIndex === -1) {
+    return [url, ""];
   }
-
-  const search = qIndex === -1 || qIndex >= len - 1 ? "" : input.slice(qIndex);
-  const path =
-    pIndex === -1
-      ? "/"
-      : "/" + input.slice(pIndex, qIndex === -1 ? undefined : qIndex);
-
-  return [path, search];
+  return [url.slice(0, qIndex), url.slice(qIndex)];
 }
 
-function _parseHost(host: string | undefined) {
+function parseHost(host: string | undefined): [hostname: string, port: string] {
   const s = (host || "").split(":");
   return [s[0], String(Number.parseInt(s[1]) || "")];
 }
diff --git a/src/utils/request.ts b/src/utils/request.ts
index 77178dd..a5c1a0a 100644
--- a/src/utils/request.ts
+++ b/src/utils/request.ts
@@ -220,7 +220,7 @@ export function getRequestHost(
       return xForwardedHost;
     }
   }
-  return event.request.headers.get("host") || "localhost";
+  return event.request.headers.get("host") || "";
 }
 
 /**
@@ -239,11 +239,14 @@ export function getRequestProtocol(
   event: H3Event,
   opts: { xForwardedProto?: boolean } = {},
 ) {
-  if (
-    opts.xForwardedProto !== false &&
-    event.request.headers.get("x-forwarded-proto") === "https"
-  ) {
-    return "https";
+  if (opts.xForwardedProto !== false) {
+    const forwardedProto = event.request.headers.get("x-forwarded-proto");
+    if (forwardedProto === "https") {
+      return "https";
+    }
+    if (forwardedProto === "http") {
+      return "http";
+    }
   }
   return event.url.protocol.slice(0, -1);
 }
@@ -264,13 +267,18 @@ export function getRequestURL(
   event: H3Event,
   opts: { xForwardedHost?: boolean; xForwardedProto?: boolean } = {},
 ) {
-  if (opts.xForwardedHost === undefined && opts.xForwardedProto === undefined) {
-    return event.url;
+  const url = new URL(event.url);
+  url.protocol = getRequestProtocol(event, opts);
+  if (opts.xForwardedHost) {
+    const host = getRequestHost(event, opts);
+    if (host) {
+      url.host = host;
+      if (!host.includes(":")) {
+        url.port = "";
+      }
+    }
   }
-  const host = getRequestHost(event, opts);
-  const protocol = getRequestProtocol(event, opts);
-  const path = event.url.pathname + event.url.search;
-  return new URL(path, `${protocol}://${host}`);
+  return url;
 }
 
 /**


Test patch:
diff --git a/test/_setup.ts b/test/_setup.ts
index 2108261..1426958 100644
--- a/test/_setup.ts
+++ b/test/_setup.ts
@@ -66,12 +66,13 @@ function setupNodeTest(opts: TestOptions = {}): TestContext {
     ctx.fetch = (input, init = {}) => {
       const url = new URL(input, ctx.url);
       const headers = new Headers(init.headers);
-      // undici/node fetch() does not allow host header
-      // https://github.com/nodejs/undici/issues/2369
-      // TODO: find a workaround to intercept
+      // Emulate a reverse proxy
       if (!headers.has("x-forwarded-host")) {
         headers.set("x-forwarded-host", url.host);
       }
+      if (url.protocol === "https:" && !headers.has("x-forwarded-proto")) {
+        headers.set("x-forwarded-proto", "https");
+      }
       return fetch(`${ctx.url}${url.pathname}${url.search}`, {
         redirect: "manual",
         // @ts-expect-error undici/node specific
diff --git a/test/utils.test.ts b/test/utils.test.ts
index afe72ce..f7b2e30 100644
--- a/test/utils.test.ts
+++ b/test/utils.test.ts
@@ -69,6 +69,20 @@ describeMatrix("utils", (t, { it, describe, expect }) => {
   });
 
   describe("getRequestURL", () => {
+    const tests = [
+      "http://localhost/foo?bar=baz",
+      "http://localhost\\foo",
+      "http://localhost//foo",
+      "http://localhost//foo//bar",
+      "http://localhost//foo\\bar\\",
+      "http://localhost///foo",
+      "http://localhost\\\\foo",
+      "http://localhost\\/foo",
+      "http://localhost/\\foo",
+      "http://example.com/test",
+      "http://localhost:8080/test",
+    ];
+
     beforeEach(() => {
       t.app.get("/**", (event) => {
         return getRequestURL(event, {
@@ -78,27 +92,10 @@ describeMatrix("utils", (t, { it, describe, expect }) => {
       });
     });
 
-    const tests = [
-      ["http://localhost/foo?bar=baz", "http://localhost/foo?bar=baz"],
-      ["http://localhost\\foo", "http://localhost/foo"],
-      // TODO: Fix issues with web normalizing URLs
-      ...(t.target === "web"
-        ? []
-        : [
-            ["http://localhost//foo", "http://localhost/foo"],
-            ["http://localhost//foo//bar", "http://localhost/foo//bar"],
-            ["http://localhost///foo", "http://localhost/foo"],
-            ["http://localhost\\\\foo", "http://localhost/foo"],
-            ["http://localhost\\/foo", "http://localhost/foo"],
-            ["http://localhost/\\foo", "http://localhost/foo"],
-            ["http://example.com/test", "http://example.com/test"],
-            ["http://localhost:8080/test", "http://localhost:8080/test"],
-          ]),
-    ];
-    for (const test of tests) {
-      it(`getRequestURL(${JSON.stringify(test[0])})`, async () => {
-        const res = await t.fetch(test[0]);
-        expect(await res.text()).toMatch(test[1]);
+    for (const c of tests) {
+      it(`getRequestURL(${JSON.stringify(c)})`, async () => {
+        const res = await t.fetch(c);
+        expect(await res.text()).toMatch(new URL(c).href);
       });
     }