Fix support for site redirects that include section/variant paths (#3024)

Author: SamyPesse

Diff for: .changeset/quiet-forks-occur.md

@@ -0,0 +1,6 @@
+---
+"gitbook-v2": patch
+"gitbook": patch
+---
+
+Fix site redirects when it includes a section/variant path

Diff for: packages/gitbook-v2/src/lib/links.test.ts

@@ -38,6 +38,17 @@ describe('toPathInSite', () => {
     });
 });
 
+describe('toRelativePathInSite', () => {
+    it('should return the correct path', () => {
+        expect(root.toRelativePathInSite('/some/path')).toBe('some/path');
+        expect(siteGitBookIO.toRelativePathInSite('/sitename/some/path')).toBe('some/path');
+    });
+
+    it('should preserve absolute paths outside of the site', () => {
+        expect(siteGitBookIO.toRelativePathInSite('/outside/some/path')).toBe('/outside/some/path');
+    });
+});
+
 describe('toAbsoluteURL', () => {
     it('should return the correct path', () => {
         expect(root.toAbsoluteURL('some/path')).toBe('https://docs.company.com/some/path');

Diff for: packages/gitbook-v2/src/lib/links.ts

@@ -1,4 +1,5 @@
 import { getPagePath } from '@/lib/pages';
+import { withLeadingSlash, withTrailingSlash } from '@/lib/paths';
 import type { RevisionPage, RevisionPageDocument, RevisionPageGroup } from '@gitbook/api';
 import warnOnce from 'warn-once';
 
@@ -25,6 +26,11 @@ export interface GitBookLinker {
      */
     toPathInSite(relativePath: string): string;
 
+    /**
+     * Transform an absolute path in a site, to a relative path from the root of the site.
+     */
+    toRelativePathInSite(absolutePath: string): string;
+
     /**
      * Generate an absolute path for a page in the current content.
      * The result should NOT be passed to `toPathInContent`.
@@ -64,13 +70,26 @@ export function createLinker(
 ): GitBookLinker {
     warnOnce(!servedOn.host, 'No host provided to createLinker. It can lead to issues with links.');
 
+    const siteBasePath = withTrailingSlash(withLeadingSlash(servedOn.siteBasePath));
+    const spaceBasePath = withTrailingSlash(withLeadingSlash(servedOn.spaceBasePath));
+
     const linker: GitBookLinker = {
         toPathInSpace(relativePath: string): string {
-            return joinPaths(servedOn.spaceBasePath, relativePath);
+            return joinPaths(spaceBasePath, relativePath);
         },
 
         toPathInSite(relativePath: string): string {
-            return joinPaths(servedOn.siteBasePath, relativePath);
+            return joinPaths(siteBasePath, relativePath);
+        },
+
+        toRelativePathInSite(absolutePath: string): string {
+            const normalizedPath = withLeadingSlash(absolutePath);
+
+            if (!normalizedPath.startsWith(servedOn.siteBasePath)) {
+                return normalizedPath;
+            }
+
+            return normalizedPath.slice(servedOn.siteBasePath.length);
         },
 
         toAbsoluteURL(absolutePath: string): string {

Diff for: packages/gitbook/e2e/internal.spec.ts

@@ -808,6 +808,21 @@ const testCases: TestsCase[] = [
             },
         ],
     },
+    {
+        name: 'Site Redirects with sections',
+        contentBaseURL: 'https://gitbook-open-e2e-sites.gitbook.io/sections/',
+        tests: [
+            {
+                // This test that a redirect that incudes a section path works
+                name: 'Redirect to Quickstart page',
+                url: 'sections-2/redirect-test',
+                run: async (page) => {
+                    await expect(page.locator('h1')).toHaveText('Quickstart');
+                },
+                screenshot: false,
+            },
+        ],
+    },
     {
         name: 'Share links',
         contentBaseURL: 'https://gitbook.gitbook.io/gbo-tests-share-links/',

Diff for: packages/gitbook/src/components/SitePage/SitePage.tsx

@@ -12,7 +12,7 @@ import { isPageIndexable, isSiteIndexable } from '@/lib/seo';
 
 import { getResizedImageURL } from '@v2/lib/images';
 import { PageClientLayout } from './PageClientLayout';
-import { type PagePathParams, fetchPageData, getPathnameParam, normalizePathname } from './fetch';
+import { type PagePathParams, fetchPageData, getPathnameParam } from './fetch';
 
 export const runtime = 'edge';
 export const dynamic = 'force-dynamic';
@@ -33,7 +33,7 @@ export async function SitePage(props: SitePageProps) {
 
     const rawPathname = getPathnameParam(props.pageParams);
     if (!pageTarget) {
-        const pathname = normalizePathname(rawPathname);
+        const pathname = rawPathname.toLowerCase();
         if (pathname !== rawPathname) {
             // If the pathname was not normalized, redirect to the normalized version
             // before trying to resolve the page again

Diff for: packages/gitbook/src/components/SitePage/fetch.ts

@@ -2,6 +2,7 @@ import type { GitBookSiteContext } from '@v2/lib/context';
 import { redirect } from 'next/navigation';
 
 import { resolvePageId, resolvePagePath } from '@/lib/pages';
+import { withLeadingSlash } from '@/lib/paths';
 import { getDataOrNull } from '@v2/lib/data';
 
 export interface PagePathParams {
@@ -35,14 +36,14 @@ export async function fetchPageData(context: GitBookSiteContext, params: PagePar
  * If the path can't be found, we try to resolve it from the API to handle redirects.
  */
 async function resolvePage(context: GitBookSiteContext, params: PagePathParams | PageIdParams) {
-    const { organizationId, site, space, revisionId, pages, shareKey } = context;
+    const { organizationId, site, space, revisionId, pages, shareKey, linker } = context;
 
     if ('pageId' in params) {
         return resolvePageId(pages, params.pageId);
     }
 
     const rawPathname = getPathnameParam(params);
-    const pathname = normalizePathname(rawPathname);
+    const pathname = rawPathname.toLowerCase();
 
     // When resolving a page, we use the lowercased pathname
     const page = resolvePagePath(pages, pathname);
@@ -67,16 +68,31 @@ async function resolvePage(context: GitBookSiteContext, params: PagePathParams |
         }
 
         // If a page still can't be found, we try with the API, in case we have a redirect at site level.
-        const resolvedSiteRedirect = await getDataOrNull(
-            context.dataFetcher.getSiteRedirectBySource({
-                organizationId,
-                siteId: site.id,
-                source: rawPathname.startsWith('/') ? rawPathname : `/${rawPathname}`,
-                siteShareKey: shareKey,
-            })
-        );
-        if (resolvedSiteRedirect) {
-            return redirect(resolvedSiteRedirect.target);
+        const redirectPathname = withLeadingSlash(rawPathname);
+        if (/^\/[a-zA-Z0-9-_.\/]+[a-zA-Z0-9-_.]$/.test(redirectPathname)) {
+            const redirectSources = new Set<string>([
+                // Test the pathname relative to the root
+                // For example hello/world -> section/variant/hello/world
+                withLeadingSlash(
+                    linker.toRelativePathInSite(linker.toPathInSpace(redirectPathname))
+                ),
+                // Test the pathname relative to the content/space
+                // For example hello/world -> /hello/world
+                redirectPathname,
+            ]);
+            for (const source of redirectSources) {
+                const resolvedSiteRedirect = await getDataOrNull(
+                    context.dataFetcher.getSiteRedirectBySource({
+                        organizationId,
+                        siteId: site.id,
+                        source,
+                        siteShareKey: shareKey,
+                    })
+                );
+                if (resolvedSiteRedirect) {
+                    return redirect(linker.toLinkForContent(resolvedSiteRedirect.target));
+                }
+            }
         }
     }
 
@@ -99,10 +115,3 @@ export function getPathnameParam(params: PagePathParams): string {
 
     return pathname.map((part) => decodeURIComponent(part)).join('/');
 }
-
-/**
- * Normalize the URL pathname into the format used in the revision page path.
- */
-export function normalizePathname(pathname: string) {
-    return pathname.toLowerCase();
-}

Diff for: packages/gitbook/src/lib/api.ts

@@ -153,7 +153,7 @@ export function withAPI<T>(client: GitBookAPIContext, fn: () => Promise<T>): Pro
 
 type SpaceContentLookup = Pick<
     PublishedSiteContent,
-    'space' | 'changeRequest' | 'revision' | 'pathname' | 'basePath' | 'apiToken'
+    'space' | 'changeRequest' | 'revision' | 'pathname' | 'basePath' | 'siteBasePath' | 'apiToken'
 > & { kind: 'space' };
 
 export type PublishedContentWithCache =

Diff for: packages/gitbook/src/lib/links.ts

@@ -10,6 +10,7 @@ import { headers } from 'next/headers';
 
 import { GITBOOK_APP_URL } from '@v2/lib/env';
 import { getPagePath } from './pages';
+import { withLeadingSlash, withTrailingSlash } from './paths';
 import { assertIsNotV2 } from './v2';
 
 export interface PageHrefContext {
@@ -27,17 +28,21 @@ export interface PageHrefContext {
 export async function getBasePath(): Promise<string> {
     assertIsNotV2();
     const headersList = await headers();
-    let path = headersList.get('x-gitbook-basepath') ?? '/';
+    const path = headersList.get('x-gitbook-basepath') ?? '/';
 
-    if (!path.startsWith('/')) {
-        path = `/${path}`;
-    }
+    return withTrailingSlash(withLeadingSlash(path));
+}
 
-    if (!path.endsWith('/')) {
-        path = `${path}/`;
-    }
+/**
+ * Return the site base path for the current request.
+ * The value will start and finish with /
+ */
+export async function getSiteBasePath(): Promise<string> {
+    assertIsNotV2();
+    const headersList = await headers();
+    const path = headersList.get('x-gitbook-site-basepath') ?? '/';
 
-    return path;
+    return withTrailingSlash(withLeadingSlash(path));
 }
 
 /**

Diff for: packages/gitbook/src/lib/paths.ts

@@ -22,10 +22,21 @@ export function removeLeadingSlash(path: string): string {
 /**
  * Normalize a pathname to make it start with a slash
  */
-export function normalizePathname(pathname: string): string {
+export function withLeadingSlash(pathname: string): string {
     if (!pathname.startsWith('/')) {
         pathname = `/${pathname}`;
     }
 
     return pathname;
 }
+
+/**
+ * Normalize a pathname to make it end with a slash
+ */
+export function withTrailingSlash(pathname: string): string {
+    if (!pathname.endsWith('/')) {
+        pathname = `${pathname}/`;
+    }
+
+    return pathname;
+}

Diff for: packages/gitbook/src/lib/proxy.ts

@@ -1,5 +1,5 @@
 import type { PublishedSiteContent } from '@gitbook/api';
-import { joinPath, normalizePathname, removeTrailingSlash } from './paths';
+import { joinPath, removeTrailingSlash, withLeadingSlash } from './paths';
 
 /**
  * Compute the final base path for a site served in proxy mode.
@@ -16,6 +16,6 @@ export function getProxyModeBasePath(
         .replace(removeTrailingSlash(resolved.pathname), '')
         .replace(removeTrailingSlash(resolved.basePath), '');
 
-    const result = joinPath(normalizePathname(proxySitePath), resolved.basePath);
+    const result = joinPath(withLeadingSlash(proxySitePath), resolved.basePath);
     return result.endsWith('/') ? result : `${result}/`;
 }

Diff for: packages/gitbook/src/lib/v1.ts

@@ -31,7 +31,7 @@ import {
     searchSiteContent,
 } from './api';
 import { getDynamicCustomizationSettings } from './customization';
-import { getBasePath, getHost } from './links';
+import { getBasePath, getHost, getSiteBasePath } from './links';
 
 /*
  * Code that will be used until the migration to v2 is complete.
@@ -43,11 +43,12 @@ import { getBasePath, getHost } from './links';
 export async function getV1BaseContext(): Promise<GitBookBaseContext> {
     const host = await getHost();
     const basePath = await getBasePath();
+    const siteBasePath = await getSiteBasePath();
 
     const linker = createLinker({
         host,
         spaceBasePath: basePath,
-        siteBasePath: basePath,
+        siteBasePath: siteBasePath,
     });
 
     // On V1, we use hard-navigation between different spaces because of layout issues

Diff for: packages/gitbook/src/middleware.ts

@@ -28,7 +28,7 @@ import {
     normalizeVisitorAuthURL,
 } from '@/lib/visitor-token';
 
-import { joinPath, normalizePathname } from '@/lib/paths';
+import { joinPath, withLeadingSlash } from '@/lib/paths';
 import { getProxyModeBasePath } from '@/lib/proxy';
 import { MiddlewareHeaders } from '@v2/lib/middleware';
 import { addResponseCacheTag } from './lib/cache/response';
@@ -139,7 +139,7 @@ export async function middleware(request: NextRequest) {
     }
 
     // Because of how Next will encode, we need to encode ourselves the pathname before rewriting to it.
-    const rewritePathname = normalizePathname(encodePathname(resolved.pathname));
+    const rewritePathname = withLeadingSlash(encodePathname(resolved.pathname));
 
     // Resolution might have changed the API endpoint
     apiEndpoint = resolved.apiEndpoint ?? apiEndpoint;
@@ -164,6 +164,7 @@ export async function middleware(request: NextRequest) {
             ? getProxyModeBasePath(inputURL, resolved)
             : joinPath(originBasePath, resolved.basePath)
     );
+    headers.set('x-gitbook-site-basepath', joinPath(originBasePath, resolved.siteBasePath));
     headers.set('x-gitbook-content-space', resolved.space);
     if ('site' in resolved) {
         headers.set('x-gitbook-content-organization', resolved.organization);
@@ -371,6 +372,7 @@ async function lookupSiteInSingleMode(url: URL): Promise<LookupResult> {
         kind: 'space',
         space: spaceId,
         basePath: '',
+        siteBasePath: '',
         pathname: url.pathname,
         apiToken,
         visitorToken: undefined,
@@ -549,15 +551,15 @@ async function lookupSiteOrSpaceInMultiIdMode(
         };
     }
 
-    const basePath = normalizePathname(basePathParts.join('/'));
+    const basePath = withLeadingSlash(basePathParts.join('/'));
     return {
         // In multi-id mode, complete is always considered true because there is no URL to resolve
         ...(decoded.kind === 'site' ? { ...decoded, complete: true } : decoded),
         changeRequest: changeRequestId,
         revision: revisionId,
         siteBasePath: basePath,
         basePath,
-        pathname: normalizePathname(pathSegments.join('/')),
+        pathname: withLeadingSlash(pathSegments.join('/')),
         apiToken,
         apiEndpoint,
         contextId,
@@ -636,6 +638,7 @@ async function lookupSiteInMultiPathMode(request: NextRequest, url: URL): Promis
 
     return {
         ...lookup,
+        siteBasePath: joinPath(target.host, lookup.siteBasePath),
         basePath: joinPath(target.host, lookup.basePath),
         ...('basePath' in lookup && visitorAuthToken
             ? getLookupResultForVisitorAuth(lookup.basePath, visitorAuthToken)
@@ -722,6 +725,7 @@ async function lookupSiteByAPI(
                 space: data.space,
                 changeRequest,
                 revision: data.revision ?? lookup.revision,
+                siteBasePath: data.siteBasePath,
                 basePath: joinPath(data.basePath, lookup.basePath ?? ''),
                 pathname: joinPath(data.pathname, alternative.extraPath),
                 apiToken: data.apiToken,