refactor: update fetch handling to allow selective redirects including different paths, query parameters, and hash fragments on the same domain

Author: jakejarvis

lib/fetch.test.ts

@@ -271,64 +271,102 @@ describe("lib/fetch", () => {
       expect(globalThis.fetch).toHaveBeenCalledTimes(1);
     });
 
-    it("does NOT follow redirect to different path", async () => {
+    it("follows redirect to different path on same domain", async () => {
       const redirectRes = createResponse({
         status: 301,
         headers: new Headers({ location: "https://example.com/other-path" }),
       });
+      const finalRes = createResponse({ ok: true, status: 200 });
 
-      globalThis.fetch = vi.fn(
-        async () => redirectRes,
-      ) as unknown as typeof fetch;
+      const mock = vi
+        .fn()
+        .mockResolvedValueOnce(redirectRes)
+        .mockResolvedValueOnce(finalRes);
+      globalThis.fetch = mock as unknown as typeof fetch;
 
       const out = await fetchWithSelectiveRedirects(
         "https://example.com/",
         {},
         { timeoutMs: 50 },
       );
-      expect(out).toBe(redirectRes);
-      expect(globalThis.fetch).toHaveBeenCalledTimes(1);
+      expect(out).toBe(finalRes);
+      expect(mock).toHaveBeenCalledTimes(2);
     });
 
-    it("does NOT follow redirect with query params", async () => {
+    it("follows redirect with query params", async () => {
       const redirectRes = createResponse({
         status: 301,
         headers: new Headers({
           location: "https://example.com/?utm_source=test",
         }),
       });
+      const finalRes = createResponse({ ok: true, status: 200 });
 
-      globalThis.fetch = vi.fn(
-        async () => redirectRes,
-      ) as unknown as typeof fetch;
+      const mock = vi
+        .fn()
+        .mockResolvedValueOnce(redirectRes)
+        .mockResolvedValueOnce(finalRes);
+      globalThis.fetch = mock as unknown as typeof fetch;
 
       const out = await fetchWithSelectiveRedirects(
         "https://example.com/",
         {},
         { timeoutMs: 50 },
       );
-      expect(out).toBe(redirectRes);
-      expect(globalThis.fetch).toHaveBeenCalledTimes(1);
+      expect(out).toBe(finalRes);
+      expect(mock).toHaveBeenCalledTimes(2);
     });
 
-    it("handles relative redirect URLs", async () => {
+    it("follows redirect with hash fragment", async () => {
       const redirectRes = createResponse({
         status: 301,
-        headers: new Headers({ location: "/" }),
+        headers: new Headers({
+          location: "https://example.com/#section",
+        }),
       });
+      const finalRes = createResponse({ ok: true, status: 200 });
 
-      globalThis.fetch = vi.fn(
-        async () => redirectRes,
-      ) as unknown as typeof fetch;
+      const mock = vi
+        .fn()
+        .mockResolvedValueOnce(redirectRes)
+        .mockResolvedValueOnce(finalRes);
+      globalThis.fetch = mock as unknown as typeof fetch;
 
       const out = await fetchWithSelectiveRedirects(
-        "https://www.example.com/path",
+        "https://example.com/",
         {},
         { timeoutMs: 50 },
       );
-      // Relative redirect to "/" changes the path (/path -> /), so should NOT be followed
-      expect(out).toBe(redirectRes);
-      expect(globalThis.fetch).toHaveBeenCalledTimes(1);
+      expect(out).toBe(finalRes);
+      expect(mock).toHaveBeenCalledTimes(2);
+    });
+
+    it("follows relative redirect URLs to different path", async () => {
+      const redirectRes = createResponse({
+        status: 301,
+        headers: new Headers({ location: "/main" }),
+      });
+      const finalRes = createResponse({ ok: true, status: 200 });
+
+      const mock = vi
+        .fn()
+        .mockResolvedValueOnce(redirectRes)
+        .mockResolvedValueOnce(finalRes);
+      globalThis.fetch = mock as unknown as typeof fetch;
+
+      const out = await fetchWithSelectiveRedirects(
+        "https://www.example.com/",
+        {},
+        { timeoutMs: 50 },
+      );
+      expect(out).toBe(finalRes);
+      expect(mock).toHaveBeenCalledTimes(2);
+      // Second call should use the absolute URL
+      expect(mock).toHaveBeenNthCalledWith(
+        2,
+        "https://www.example.com/main",
+        expect.objectContaining({ redirect: "manual" }),
+      );
     });
 
     it("throws on too many redirects", async () => {

lib/fetch.ts

@@ -54,8 +54,16 @@ export async function headThenGet(
 }
 
 /**
- * Checks if a redirect from one URL to another is allowed.
- * Only allows redirects between apex/www or http/https versions of the same domain.
+ * Determines if a redirect should be followed based on selective redirect rules.
+ * Allows redirects:
+ * 1. Between apex/www versions (e.g., example.com -> www.example.com)
+ * 2. Between http/https schemes
+ * 3. To different paths on the same domain (e.g., www.example.com -> www.example.com/homepage)
+ * 4. With different query parameters (e.g., example.com -> example.com/?ref=social)
+ * 5. With different hash fragments (e.g., example.com -> example.com/#content)
+ *
+ * Blocks redirects:
+ * - To different domains (after normalizing www)
  */
 function isAllowedRedirect(fromUrl: string, toUrl: string): boolean {
   try {
@@ -72,25 +80,17 @@ function isAllowedRedirect(fromUrl: string, toUrl: string): boolean {
       return false;
     }
 
-    // Allow if path, search, and hash are the same (only scheme or www prefix changed)
-    const isSchemeLike =
-      from.pathname === to.pathname &&
-      from.search === to.search &&
-      from.hash === to.hash;
-    if (isSchemeLike) {
-      return true;
-    }
-
-    return false;
+    // Allow: same domain, any path, query params, or hash
+    return true;
   } catch {
     // If URL parsing fails, don't allow redirect
     return false;
   }
 }
 
 /**
- * Fetch with manual redirect handling that only follows redirects between
- * apex/www or http/https versions of the same domain.
+ * Fetch with manual redirect handling that only follows redirects to the same domain
+ * (allowing apex/www, http/https, path, query param, and hash changes).
  */
 export async function fetchWithSelectiveRedirects(
   input: RequestInfo | URL,