Fix normalization of URL causing invalid redirections. (#4130)

.changeset/mighty-ghosts-invent.md

@@ -0,0 +1,5 @@
+---
+"gitbook": patch
+---
+
+Fix normalization of URL causing invalid redirections.

packages/gitbook/src/lib/data/urls.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'bun:test';
 
-import { decodeURLPath, getURLLookupAlternatives, normalizeURL } from './urls';
+import { getURLLookupAlternatives, normalizeURL } from './urls';
 
 describe('getURLLookupAlternatives', () => {
     it('should return all URLs up to the root', () => {
@@ -405,12 +405,20 @@ describe('normalizeURL', () => {
             new URL('https://docs.mycompany.com/hello/there')
         );
     });
+
+    it('should throw for URL paths exceeding 2048 characters', () => {
+        const longPath = '/a'.repeat(1025); // 2050 chars
+        const url = new URL(`https://docs.mycompany.com${longPath}`);
+        expect(() => {
+            normalizeURL(url);
+        }).toThrow('URL path is too long');
+    });
 });
 
-describe('decodeURLPath', () => {
+describe('normalizeURL with encoded paths', () => {
     it('should decode encoded path components', () => {
         const url = new URL('https://docs.mycompany.com/helloworld/tes%74');
-        const result = decodeURLPath(url);
+        const result = normalizeURL(url);
         expect(result.pathname).toBe('/helloworld/test');
         expect(result.toString()).toBe('https://docs.mycompany.com/helloworld/test');
     });
@@ -419,48 +427,49 @@ describe('decodeURLPath', () => {
         // Double encoded: tes%2574 → tes%74 → test
         // %2574 decodes as: %25 → %, leaving %74, which then decodes to t
         const url = new URL('https://docs.mycompany.com/helloworld/tes%2574');
-        const result = decodeURLPath(url);
+        const result = normalizeURL(url);
         expect(result.pathname).toBe('/helloworld/test');
 
-        // Triple encoding (tes%252574) exceeds the 2-pass limit and is rejected
-        expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/helloworld/tes%252574'));
-        }).toThrow('URL path is malformed');
+        // Triple encoding is also normalized through the nested normalizeURL flow.
+        const tripleEncoded = normalizeURL(
+            new URL('https://docs.mycompany.com/helloworld/tes%252574')
+        );
+        expect(tripleEncoded.pathname).toBe('/helloworld/test');
     });
 
     it('should throw for malformed percent-encoding in the path', () => {
         // Invalid hex digits in percent-encoding
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/helloworld/%ZZ'));
+            normalizeURL(new URL('https://docs.mycompany.com/helloworld/%ZZ'));
         }).toThrow('URL path is malformed');
 
         // Incomplete or invalid UTF-8 sequence
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/helloworld/%E0%A4%A'));
+            normalizeURL(new URL('https://docs.mycompany.com/helloworld/%E0%A4%A'));
         }).toThrow('URL path is malformed');
 
         // Trailing '%' without two following hex digits
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/helloworld/trailing%'));
+            normalizeURL(new URL('https://docs.mycompany.com/helloworld/trailing%'));
         }).toThrow('URL path is malformed');
     });
 
     it.skip('should throw an error for invalid characters in the path', () => {
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/hello:world'));
+            normalizeURL(new URL('https://docs.mycompany.com/hello:world'));
         }).toThrow('URL path contains invalid characters');
 
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/hello%3Btest'));
+            normalizeURL(new URL('https://docs.mycompany.com/hello%3Btest'));
         }).toThrow('URL path contains invalid characters');
 
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/hello%40anchor'));
+            normalizeURL(new URL('https://docs.mycompany.com/hello%40anchor'));
         }).toThrow('URL path contains invalid characters');
 
         // %20 (space) re-encodes to %20 after decoding, so the path is stable
         // and considered fully decoded — it should not throw.
-        expect(decodeURLPath(new URL('https://docs.mycompany.com/hello%20world')).pathname).toBe(
+        expect(normalizeURL(new URL('https://docs.mycompany.com/hello%20world')).pathname).toBe(
             '/hello%20world'
         );
     });
@@ -470,33 +479,25 @@ describe('decodeURLPath', () => {
         // %25252525 needs 4 passes: %25252525 → %252525 → %2525 → %25 → %
         const url = new URL('https://docs.mycompany.com/%25252525');
         expect(() => {
-            decodeURLPath(url);
+            normalizeURL(url);
         }).toThrow('URL path is malformed');
 
         const deepUrl = new URL('https://docs.mycompany.com/%2525252525252525');
         expect(() => {
-            decodeURLPath(deepUrl);
+            normalizeURL(deepUrl);
         }).toThrow('URL path is malformed');
     });
 
-    it('should throw for URL paths exceeding 2048 characters', () => {
-        const longPath = '/a'.repeat(1025); // 2050 chars
-        const url = new URL(`https://docs.mycompany.com${longPath}`);
-        expect(() => {
-            decodeURLPath(url);
-        }).toThrow('URL path is too long');
-    });
-
     // TODO: should we do that actually?
     it.skip('should throw an error if the encoded path contains /', () => {
         expect(() => {
-            decodeURLPath(new URL('https://docs.mycompany.com/hello%2Fworld'));
+            normalizeURL(new URL('https://docs.mycompany.com/hello%2Fworld'));
         }).toThrow('URL path contains invalid characters');
     });
 
     it('should not decode search params or hash fragments', () => {
         const url = new URL('https://docs.mycompany.com/helloworld/tes%74?query=%74est#sec%74ion');
-        const result = decodeURLPath(url);
+        const result = normalizeURL(url);
         expect(result.pathname).toBe('/helloworld/test');
         expect(result.search).toBe('?query=%74est');
         expect(result.hash).toBe('#sec%74ion');
@@ -508,7 +509,7 @@ describe('decodeURLPath', () => {
         const url = new URL(`https://docs.mycompany.com/short-path?jwt_token=${fakeJwt}`);
         // The path itself is well within the limit; only the query param is huge.
         expect(url.pathname.length).toBeLessThan(2048);
-        const result = decodeURLPath(url);
+        const result = normalizeURL(url);
         expect(result.pathname).toBe('/short-path');
         // The query string must pass through untouched.
         expect(result.searchParams.get('jwt_token')).toBe(fakeJwt);
@@ -521,9 +522,9 @@ describe('decodeURLPath', () => {
         const url = new URL(
             `https://docs.mycompany.com/some-page?filter=${encodeURIComponent(risonValue)}`
         );
-        const result = decodeURLPath(url);
+        const result = normalizeURL(url);
         expect(result.pathname).toBe('/some-page');
-        // The rison param must survive decodeURLPath intact.
+        // The rison param must survive normalizeURL intact.
         expect(result.searchParams.get('filter')).toBe(risonValue);
     });
 });

packages/gitbook/src/lib/data/urls.ts

@@ -1,5 +1,5 @@
 import { isProxyRootRequest } from '../proxy';
-import { DataFetcherError } from './errors';
+import { DataFetcherError, getExposableError } from './errors';
 
 /**
  * For a given GitBook URL, return a list of alternative URLs that could be matched against to lookup the content.
@@ -118,46 +118,39 @@ export function getURLLookupAlternatives(input: URL) {
     return { urls: alternatives, basePath, changeRequest, revision };
 }
 
+/**
+ * Normalize the URL in a request and redirect if the normalized URL is different from the original one.
+ */
+export function normalizeRequestURL(url: URL): Response | null {
+    try {
+        const normalizedURL = normalizeURL(url);
+
+        if (normalizedURL.toString() !== url.toString()) {
+            return Response.redirect(normalizedURL.toString(), 302);
+        }
+
+        return null;
+    } catch (error) {
+        const sanitized = getExposableError(error);
+        return new Response(sanitized.message, { status: sanitized.code });
+    }
+}
+
 /**
  * Normalize a URL to remove duplicate slashes and trailing slashes
  * and transform the pathname to lowercase.
  */
 export function normalizeURL(url: URL) {
     const result = new URL(url);
-    result.pathname = url.pathname.replace(/\/{2,}/g, '/').replace(/\/$/, '');
-    return result;
-}
 
-/**
- * This function checks if a decoded URL path segment contains characters that are not allowed
- * in GitBook content paths. These characters are valid in generic RFC 3986 URL paths, but are
- * rejected here as an application-level constraint for GitBook routing and security.
- * https://developer.mozilla.org/en-US/docs/Glossary/Percent-encoding
- * "%" itself is excluded because it could be part of percent-encoding and may require decoding.
- */
-// function containsInvalidURLCharacters(segment: string): boolean {
-//     const invalidCharacters = [
-//         ':',
-//         '/',
-//         '?',
-//         '#',
-//         '[',
-//         ']',
-//         '@',
-//         '!',
-//         '$',
-//         '&',
-//         "'",
-//         '(',
-//         ')',
-//         '*',
-//         '+',
-//         ',',
-//         ';',
-//         '=',
-//     ];
-//     return invalidCharacters.some((char) => segment.includes(char));
-// }
+    // Reject excessively long paths up-front to bound per-request work.
+    if (url.pathname.length > 2048) {
+        throw new DataFetcherError('URL path is too long.', 400);
+    }
+
+    result.pathname = url.pathname.replace(/\/{2,}/g, '/').replace(/\/$/, '');
+    return decodeURLPath(result);
+}
 
 /**
  * Decode the url path component, we redirect URLs with encoded path components
@@ -167,12 +160,7 @@ export function normalizeURL(url: URL) {
  * percent-encoding. Legitimate URLs are at most singly encoded; double-encoding covers
  * any reasonable proxy behaviour.
  */
-export function decodeURLPath(url: URL): URL {
-    // Reject excessively long paths up-front to bound per-request work.
-    if (url.pathname.length > 2048) {
-        throw new DataFetcherError(`URL path is too long: ${url.pathname}`, 400);
-    }
-
+function decodeURLPath(url: URL): URL {
     let current = url;
 
     for (let i = 0; i < 2; i++) {

packages/gitbook/src/middleware.ts

@@ -13,10 +13,9 @@ import { getContentSecurityPolicy } from '@/lib/csp';
 import { validateSerializedCustomization } from '@/lib/customization';
 import {
     DataFetcherError,
-    decodeURLPath,
     getVisitorAuthBasePath,
     lookupPublishedContentByUrl,
-    normalizeURL,
+    normalizeRequestURL,
     throwIfDataError,
 } from '@/lib/data';
 import { GITBOOK_OAUTH_SERVER_URL, isGitBookAssetsHostURL, isGitBookHostURL } from '@/lib/env';
@@ -60,18 +59,6 @@ export async function middleware(request: NextRequest) {
     try {
         const requestURL = new URL(request.url);
 
-        // Redirect to normalize the URL
-        const normalized = normalizeURL(requestURL);
-        if (normalized.toString() !== requestURL.toString()) {
-            return NextResponse.redirect(normalized.toString());
-        }
-
-        // If the URL path is encoded, decode it and redirect to the decoded URL
-        const decoded = decodeURLPath(requestURL);
-        if (decoded.toString() !== requestURL.toString()) {
-            return NextResponse.redirect(decoded.toString());
-        }
-
         // Reject malicious requests
         const rejectResponse = await validateServerActionRequest(request);
         if (rejectResponse) {
@@ -152,6 +139,13 @@ async function serveSiteRoutes(requestURL: URL, request: NextRequest) {
     }
 
     const { url: siteRequestURL, mode } = match;
+
+    // Normalize URL after extracting the URL from the request to make sure the client is redirected to the proper one
+    const normalizationResponse = normalizeRequestURL(siteRequestURL);
+    if (normalizationResponse) {
+        return normalizationResponse;
+    }
+
     const imagesContextId = getImageResizingContextId(siteRequestURL);
     /**
      * Serve image resizing requests (all requests containing `/~gitbook/image`).