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

This commit is contained in:
Samy Pessé
2025-03-24 10:21:26 +01:00
committed by GitHub
parent 54ee0149e1
commit bba2e52e24
12 changed files with 122 additions and 41 deletions
+6
View File
@@ -0,0 +1,6 @@
---
"gitbook-v2": patch
"gitbook": patch
---
Fix site redirects when it includes a section/variant path
+11
View File
@@ -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', () => { describe('toAbsoluteURL', () => {
it('should return the correct path', () => { it('should return the correct path', () => {
expect(root.toAbsoluteURL('some/path')).toBe('https://docs.company.com/some/path'); expect(root.toAbsoluteURL('some/path')).toBe('https://docs.company.com/some/path');
+21 -2
View File
@@ -1,4 +1,5 @@
import { getPagePath } from '@/lib/pages'; import { getPagePath } from '@/lib/pages';
import { withLeadingSlash, withTrailingSlash } from '@/lib/paths';
import type { RevisionPage, RevisionPageDocument, RevisionPageGroup } from '@gitbook/api'; import type { RevisionPage, RevisionPageDocument, RevisionPageGroup } from '@gitbook/api';
import warnOnce from 'warn-once'; import warnOnce from 'warn-once';
@@ -25,6 +26,11 @@ export interface GitBookLinker {
*/ */
toPathInSite(relativePath: string): string; 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. * Generate an absolute path for a page in the current content.
* The result should NOT be passed to `toPathInContent`. * The result should NOT be passed to `toPathInContent`.
@@ -64,13 +70,26 @@ export function createLinker(
): GitBookLinker { ): GitBookLinker {
warnOnce(!servedOn.host, 'No host provided to createLinker. It can lead to issues with links.'); 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 = { const linker: GitBookLinker = {
toPathInSpace(relativePath: string): string { toPathInSpace(relativePath: string): string {
return joinPaths(servedOn.spaceBasePath, relativePath); return joinPaths(spaceBasePath, relativePath);
}, },
toPathInSite(relativePath: string): string { 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 { toAbsoluteURL(absolutePath: string): string {
+15
View File
@@ -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', name: 'Share links',
contentBaseURL: 'https://gitbook.gitbook.io/gbo-tests-share-links/', contentBaseURL: 'https://gitbook.gitbook.io/gbo-tests-share-links/',
@@ -12,7 +12,7 @@ import { isPageIndexable, isSiteIndexable } from '@/lib/seo';
import { getResizedImageURL } from '@v2/lib/images'; import { getResizedImageURL } from '@v2/lib/images';
import { PageClientLayout } from './PageClientLayout'; 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 runtime = 'edge';
export const dynamic = 'force-dynamic'; export const dynamic = 'force-dynamic';
@@ -33,7 +33,7 @@ export async function SitePage(props: SitePageProps) {
const rawPathname = getPathnameParam(props.pageParams); const rawPathname = getPathnameParam(props.pageParams);
if (!pageTarget) { if (!pageTarget) {
const pathname = normalizePathname(rawPathname); const pathname = rawPathname.toLowerCase();
if (pathname !== rawPathname) { if (pathname !== rawPathname) {
// If the pathname was not normalized, redirect to the normalized version // If the pathname was not normalized, redirect to the normalized version
// before trying to resolve the page again // before trying to resolve the page again
@@ -2,6 +2,7 @@ import type { GitBookSiteContext } from '@v2/lib/context';
import { redirect } from 'next/navigation'; import { redirect } from 'next/navigation';
import { resolvePageId, resolvePagePath } from '@/lib/pages'; import { resolvePageId, resolvePagePath } from '@/lib/pages';
import { withLeadingSlash } from '@/lib/paths';
import { getDataOrNull } from '@v2/lib/data'; import { getDataOrNull } from '@v2/lib/data';
export interface PagePathParams { 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. * 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) { 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) { if ('pageId' in params) {
return resolvePageId(pages, params.pageId); return resolvePageId(pages, params.pageId);
} }
const rawPathname = getPathnameParam(params); const rawPathname = getPathnameParam(params);
const pathname = normalizePathname(rawPathname); const pathname = rawPathname.toLowerCase();
// When resolving a page, we use the lowercased pathname // When resolving a page, we use the lowercased pathname
const page = resolvePagePath(pages, 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. // If a page still can't be found, we try with the API, in case we have a redirect at site level.
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( const resolvedSiteRedirect = await getDataOrNull(
context.dataFetcher.getSiteRedirectBySource({ context.dataFetcher.getSiteRedirectBySource({
organizationId, organizationId,
siteId: site.id, siteId: site.id,
source: rawPathname.startsWith('/') ? rawPathname : `/${rawPathname}`, source,
siteShareKey: shareKey, siteShareKey: shareKey,
}) })
); );
if (resolvedSiteRedirect) { if (resolvedSiteRedirect) {
return redirect(resolvedSiteRedirect.target); return redirect(linker.toLinkForContent(resolvedSiteRedirect.target));
}
}
} }
} }
@@ -99,10 +115,3 @@ export function getPathnameParam(params: PagePathParams): string {
return pathname.map((part) => decodeURIComponent(part)).join('/'); 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();
}
+1 -1
View File
@@ -153,7 +153,7 @@ export function withAPI<T>(client: GitBookAPIContext, fn: () => Promise<T>): Pro
type SpaceContentLookup = Pick< type SpaceContentLookup = Pick<
PublishedSiteContent, PublishedSiteContent,
'space' | 'changeRequest' | 'revision' | 'pathname' | 'basePath' | 'apiToken' 'space' | 'changeRequest' | 'revision' | 'pathname' | 'basePath' | 'siteBasePath' | 'apiToken'
> & { kind: 'space' }; > & { kind: 'space' };
export type PublishedContentWithCache = export type PublishedContentWithCache =
+13 -8
View File
@@ -10,6 +10,7 @@ import { headers } from 'next/headers';
import { GITBOOK_APP_URL } from '@v2/lib/env'; import { GITBOOK_APP_URL } from '@v2/lib/env';
import { getPagePath } from './pages'; import { getPagePath } from './pages';
import { withLeadingSlash, withTrailingSlash } from './paths';
import { assertIsNotV2 } from './v2'; import { assertIsNotV2 } from './v2';
export interface PageHrefContext { export interface PageHrefContext {
@@ -27,17 +28,21 @@ export interface PageHrefContext {
export async function getBasePath(): Promise<string> { export async function getBasePath(): Promise<string> {
assertIsNotV2(); assertIsNotV2();
const headersList = await headers(); const headersList = await headers();
let path = headersList.get('x-gitbook-basepath') ?? '/'; const path = headersList.get('x-gitbook-basepath') ?? '/';
if (!path.startsWith('/')) { return withTrailingSlash(withLeadingSlash(path));
path = `/${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));
} }
/** /**
+12 -1
View File
@@ -22,10 +22,21 @@ export function removeLeadingSlash(path: string): string {
/** /**
* Normalize a pathname to make it start with a slash * 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('/')) { if (!pathname.startsWith('/')) {
pathname = `/${pathname}`; pathname = `/${pathname}`;
} }
return 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;
}
+2 -2
View File
@@ -1,5 +1,5 @@
import type { PublishedSiteContent } from '@gitbook/api'; 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. * 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.pathname), '')
.replace(removeTrailingSlash(resolved.basePath), ''); .replace(removeTrailingSlash(resolved.basePath), '');
const result = joinPath(normalizePathname(proxySitePath), resolved.basePath); const result = joinPath(withLeadingSlash(proxySitePath), resolved.basePath);
return result.endsWith('/') ? result : `${result}/`; return result.endsWith('/') ? result : `${result}/`;
} }
+3 -2
View File
@@ -31,7 +31,7 @@ import {
searchSiteContent, searchSiteContent,
} from './api'; } from './api';
import { getDynamicCustomizationSettings } from './customization'; 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. * 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> { export async function getV1BaseContext(): Promise<GitBookBaseContext> {
const host = await getHost(); const host = await getHost();
const basePath = await getBasePath(); const basePath = await getBasePath();
const siteBasePath = await getSiteBasePath();
const linker = createLinker({ const linker = createLinker({
host, host,
spaceBasePath: basePath, spaceBasePath: basePath,
siteBasePath: basePath, siteBasePath: siteBasePath,
}); });
// On V1, we use hard-navigation between different spaces because of layout issues // On V1, we use hard-navigation between different spaces because of layout issues
+8 -4
View File
@@ -28,7 +28,7 @@ import {
normalizeVisitorAuthURL, normalizeVisitorAuthURL,
} from '@/lib/visitor-token'; } from '@/lib/visitor-token';
import { joinPath, normalizePathname } from '@/lib/paths'; import { joinPath, withLeadingSlash } from '@/lib/paths';
import { getProxyModeBasePath } from '@/lib/proxy'; import { getProxyModeBasePath } from '@/lib/proxy';
import { MiddlewareHeaders } from '@v2/lib/middleware'; import { MiddlewareHeaders } from '@v2/lib/middleware';
import { addResponseCacheTag } from './lib/cache/response'; 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. // 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 // Resolution might have changed the API endpoint
apiEndpoint = resolved.apiEndpoint ?? apiEndpoint; apiEndpoint = resolved.apiEndpoint ?? apiEndpoint;
@@ -164,6 +164,7 @@ export async function middleware(request: NextRequest) {
? getProxyModeBasePath(inputURL, resolved) ? getProxyModeBasePath(inputURL, resolved)
: joinPath(originBasePath, resolved.basePath) : joinPath(originBasePath, resolved.basePath)
); );
headers.set('x-gitbook-site-basepath', joinPath(originBasePath, resolved.siteBasePath));
headers.set('x-gitbook-content-space', resolved.space); headers.set('x-gitbook-content-space', resolved.space);
if ('site' in resolved) { if ('site' in resolved) {
headers.set('x-gitbook-content-organization', resolved.organization); headers.set('x-gitbook-content-organization', resolved.organization);
@@ -371,6 +372,7 @@ async function lookupSiteInSingleMode(url: URL): Promise<LookupResult> {
kind: 'space', kind: 'space',
space: spaceId, space: spaceId,
basePath: '', basePath: '',
siteBasePath: '',
pathname: url.pathname, pathname: url.pathname,
apiToken, apiToken,
visitorToken: undefined, visitorToken: undefined,
@@ -549,7 +551,7 @@ async function lookupSiteOrSpaceInMultiIdMode(
}; };
} }
const basePath = normalizePathname(basePathParts.join('/')); const basePath = withLeadingSlash(basePathParts.join('/'));
return { return {
// In multi-id mode, complete is always considered true because there is no URL to resolve // In multi-id mode, complete is always considered true because there is no URL to resolve
...(decoded.kind === 'site' ? { ...decoded, complete: true } : decoded), ...(decoded.kind === 'site' ? { ...decoded, complete: true } : decoded),
@@ -557,7 +559,7 @@ async function lookupSiteOrSpaceInMultiIdMode(
revision: revisionId, revision: revisionId,
siteBasePath: basePath, siteBasePath: basePath,
basePath, basePath,
pathname: normalizePathname(pathSegments.join('/')), pathname: withLeadingSlash(pathSegments.join('/')),
apiToken, apiToken,
apiEndpoint, apiEndpoint,
contextId, contextId,
@@ -636,6 +638,7 @@ async function lookupSiteInMultiPathMode(request: NextRequest, url: URL): Promis
return { return {
...lookup, ...lookup,
siteBasePath: joinPath(target.host, lookup.siteBasePath),
basePath: joinPath(target.host, lookup.basePath), basePath: joinPath(target.host, lookup.basePath),
...('basePath' in lookup && visitorAuthToken ...('basePath' in lookup && visitorAuthToken
? getLookupResultForVisitorAuth(lookup.basePath, visitorAuthToken) ? getLookupResultForVisitorAuth(lookup.basePath, visitorAuthToken)
@@ -722,6 +725,7 @@ async function lookupSiteByAPI(
space: data.space, space: data.space,
changeRequest, changeRequest,
revision: data.revision ?? lookup.revision, revision: data.revision ?? lookup.revision,
siteBasePath: data.siteBasePath,
basePath: joinPath(data.basePath, lookup.basePath ?? ''), basePath: joinPath(data.basePath, lookup.basePath ?? ''),
pathname: joinPath(data.pathname, alternative.extraPath), pathname: joinPath(data.pathname, alternative.extraPath),
apiToken: data.apiToken, apiToken: data.apiToken,