diff --git a/.changeset/quiet-forks-occur.md b/.changeset/quiet-forks-occur.md new file mode 100644 index 000000000..1f117db15 --- /dev/null +++ b/.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 --git a/packages/gitbook-v2/src/lib/links.test.ts b/packages/gitbook-v2/src/lib/links.test.ts index cfbfcbced..26a2464f5 100644 --- a/packages/gitbook-v2/src/lib/links.test.ts +++ b/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 --git a/packages/gitbook-v2/src/lib/links.ts b/packages/gitbook-v2/src/lib/links.ts index 4ff67379d..857345703 100644 --- a/packages/gitbook-v2/src/lib/links.ts +++ b/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 --git a/packages/gitbook/e2e/internal.spec.ts b/packages/gitbook/e2e/internal.spec.ts index e29bcb2fa..5b524b5f5 100644 --- a/packages/gitbook/e2e/internal.spec.ts +++ b/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 --git a/packages/gitbook/src/components/SitePage/SitePage.tsx b/packages/gitbook/src/components/SitePage/SitePage.tsx index c60b541d4..6339688c3 100644 --- a/packages/gitbook/src/components/SitePage/SitePage.tsx +++ b/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 --git a/packages/gitbook/src/components/SitePage/fetch.ts b/packages/gitbook/src/components/SitePage/fetch.ts index 63180d003..6ca4e5529 100644 --- a/packages/gitbook/src/components/SitePage/fetch.ts +++ b/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([ + // 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 --git a/packages/gitbook/src/lib/api.ts b/packages/gitbook/src/lib/api.ts index e2c048aff..6694a8dd2 100644 --- a/packages/gitbook/src/lib/api.ts +++ b/packages/gitbook/src/lib/api.ts @@ -153,7 +153,7 @@ export function withAPI(client: GitBookAPIContext, fn: () => Promise): Pro type SpaceContentLookup = Pick< PublishedSiteContent, - 'space' | 'changeRequest' | 'revision' | 'pathname' | 'basePath' | 'apiToken' + 'space' | 'changeRequest' | 'revision' | 'pathname' | 'basePath' | 'siteBasePath' | 'apiToken' > & { kind: 'space' }; export type PublishedContentWithCache = diff --git a/packages/gitbook/src/lib/links.ts b/packages/gitbook/src/lib/links.ts index 9f6576e7b..056e48b20 100644 --- a/packages/gitbook/src/lib/links.ts +++ b/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 { 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 { + assertIsNotV2(); + const headersList = await headers(); + const path = headersList.get('x-gitbook-site-basepath') ?? '/'; - return path; + return withTrailingSlash(withLeadingSlash(path)); } /** diff --git a/packages/gitbook/src/lib/paths.ts b/packages/gitbook/src/lib/paths.ts index 1b4edc24d..0d1c1b4c3 100644 --- a/packages/gitbook/src/lib/paths.ts +++ b/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 --git a/packages/gitbook/src/lib/proxy.ts b/packages/gitbook/src/lib/proxy.ts index c915426c1..4417ab7e2 100644 --- a/packages/gitbook/src/lib/proxy.ts +++ b/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 --git a/packages/gitbook/src/lib/v1.ts b/packages/gitbook/src/lib/v1.ts index 9713ad58b..4c78d3bcd 100644 --- a/packages/gitbook/src/lib/v1.ts +++ b/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 { 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 --git a/packages/gitbook/src/middleware.ts b/packages/gitbook/src/middleware.ts index 20d978333..dc2ddd35d 100644 --- a/packages/gitbook/src/middleware.ts +++ b/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 { kind: 'space', space: spaceId, basePath: '', + siteBasePath: '', pathname: url.pathname, apiToken, visitorToken: undefined, @@ -549,7 +551,7 @@ 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), @@ -557,7 +559,7 @@ async function lookupSiteOrSpaceInMultiIdMode( 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,