diff --git a/.changeset/visitor-country-hostnames-env.md b/.changeset/visitor-country-hostnames-env.md deleted file mode 100644 index 0d2a1a371..000000000 --- a/.changeset/visitor-country-hostnames-env.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"gitbook": patch ---- - -Read the production hostnames that receive the visitor country from the `GITBOOK_VISITOR_COUNTRY_HOSTNAMES` build-time env variable. diff --git a/.github/composite/deploy-cloudflare/action.yaml b/.github/composite/deploy-cloudflare/action.yaml index 346e295ac..a000568b1 100644 --- a/.github/composite/deploy-cloudflare/action.yaml +++ b/.github/composite/deploy-cloudflare/action.yaml @@ -70,7 +70,6 @@ runs: GITBOOK_BLOCK_SEARCH_INDEXATION: ${{ inputs.environment == 'preview' && 'true' || '' }} GITBOOK_ALLOW_CUSTOMIZATION_OVERRIDE: ${{ inputs.environment == 'preview' && 'true' || '' }} GITBOOK_DISABLE_INSIGHTS: ${{ inputs.environment == 'preview' && 'true' || '' }} - GITBOOK_DISABLE_LOOKUP_ALTERNATIVES: ${{ inputs.environment == 'staging' && 'true' || '' }} shell: bash - name: Upload the DO worker diff --git a/.github/composite/deploy-vercel/action.yaml b/.github/composite/deploy-vercel/action.yaml index aeb934fd9..76968b3e2 100644 --- a/.github/composite/deploy-vercel/action.yaml +++ b/.github/composite/deploy-vercel/action.yaml @@ -78,16 +78,6 @@ runs: echo "GITBOOK_DISABLE_INSIGHTS=true" >> .vercel/.env.${{ inputs.environment }}.local echo "--- .vercel/.env.${{ inputs.environment }}.local after inject ---" cat .vercel/.env.${{ inputs.environment }}.local - - name: Inject visitor country hostnames - if: ${{ env.GITBOOK_VISITOR_COUNTRY_HOSTNAMES != '' }} - shell: bash - run: | - echo "GITBOOK_VISITOR_COUNTRY_HOSTNAMES=$GITBOOK_VISITOR_COUNTRY_HOSTNAMES" >> .vercel/.env.${{ inputs.environment }}.local - - name: Inject staging build env vars - if: ${{ inputs.environment == 'staging' }} - shell: bash - run: | - echo "GITBOOK_DISABLE_LOOKUP_ALTERNATIVES=true" >> .vercel/.env.${{ inputs.environment }}.local - name: Build Project Artifacts run: bun run vercel build --target=${{ inputs.environment }} --token=${{ inputs.vercelToken }} shell: bash diff --git a/.github/workflows/deploy-production.yaml b/.github/workflows/deploy-production.yaml index 471ada6ba..dcf465a76 100644 --- a/.github/workflows/deploy-production.yaml +++ b/.github/workflows/deploy-production.yaml @@ -10,8 +10,6 @@ jobs: environment: name: 2v-production url: ${{ steps.deploy.outputs.deployment-url }} - env: - GITBOOK_VISITOR_COUNTRY_HOSTNAMES: ${{ vars.GITBOOK_VISITOR_COUNTRY_HOSTNAMES }} outputs: deployment-url: ${{ steps.deploy.outputs.deployment-url }} steps: @@ -33,8 +31,6 @@ jobs: environment: name: 2c-production url: ${{ steps.deploy.outputs.deployment-url }} - env: - GITBOOK_VISITOR_COUNTRY_HOSTNAMES: ${{ vars.GITBOOK_VISITOR_COUNTRY_HOSTNAMES }} outputs: deployment-url: ${{ steps.deploy.outputs.deployment-url }} steps: diff --git a/packages/gitbook/next.config.mjs b/packages/gitbook/next.config.mjs index 6523801da..ce1c0113b 100644 --- a/packages/gitbook/next.config.mjs +++ b/packages/gitbook/next.config.mjs @@ -95,7 +95,6 @@ const nextConfig = { GITBOOK_BLOCK_SEARCH_INDEXATION: process.env.GITBOOK_BLOCK_SEARCH_INDEXATION, GITBOOK_ALLOW_CUSTOMIZATION_OVERRIDE: process.env.GITBOOK_ALLOW_CUSTOMIZATION_OVERRIDE, GITBOOK_DISABLE_INSIGHTS: process.env.GITBOOK_DISABLE_INSIGHTS, - GITBOOK_VISITOR_COUNTRY_HOSTNAMES: process.env.GITBOOK_VISITOR_COUNTRY_HOSTNAMES, VERCEL_TARGET_ENV: process.env.VERCEL_TARGET_ENV, // Next.js envs diff --git a/packages/gitbook/src/app/~gitbook/env/route.ts b/packages/gitbook/src/app/~gitbook/env/route.ts index 4d4382f4e..80eec2e02 100644 --- a/packages/gitbook/src/app/~gitbook/env/route.ts +++ b/packages/gitbook/src/app/~gitbook/env/route.ts @@ -7,7 +7,6 @@ import { GITBOOK_APP_URL, GITBOOK_ASSETS_URL, GITBOOK_DISABLE_INSIGHTS, - GITBOOK_DISABLE_LOOKUP_ALTERNATIVES, GITBOOK_DISABLE_TRACKING, GITBOOK_FONTS_URL, GITBOOK_ICONS_URL, @@ -38,7 +37,6 @@ export async function GET(_req: NextRequest) { GITBOOK_INTEGRATIONS_CONTENT_HOST, GITBOOK_DISABLE_TRACKING, GITBOOK_DISABLE_INSIGHTS, - GITBOOK_DISABLE_LOOKUP_ALTERNATIVES, // Secret envs GITBOOK_SECRET: !!GITBOOK_SECRET, diff --git a/packages/gitbook/src/lib/api-token.test.ts b/packages/gitbook/src/lib/api-token.test.ts deleted file mode 100644 index 370ab3d4e..000000000 --- a/packages/gitbook/src/lib/api-token.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { describe, expect, it } from 'bun:test'; -import jwt from 'jsonwebtoken'; - -import { isAPITokenExpired } from './api-token'; - -function signToken(exp: number | undefined) { - return jwt.sign(exp === undefined ? {} : { exp }, 'secret'); -} - -describe('isAPITokenExpired', () => { - const now = Math.floor(Date.now() / 1000); - - it('returns false for a token valid for a while', () => { - expect(isAPITokenExpired(signToken(now + 3600))).toBe(false); - }); - - it('returns true for an expired token', () => { - expect(isAPITokenExpired(signToken(now - 60))).toBe(true); - }); - - it('returns true for a token expiring within the margin', () => { - expect(isAPITokenExpired(signToken(now + 60))).toBe(true); - }); - - it('returns false for a token without an expiration', () => { - expect(isAPITokenExpired(signToken(undefined))).toBe(false); - }); - - it('returns false for a token that cannot be decoded', () => { - expect(isAPITokenExpired('not-a-jwt')).toBe(false); - }); -}); diff --git a/packages/gitbook/src/lib/api-token.ts b/packages/gitbook/src/lib/api-token.ts deleted file mode 100644 index 4bf1d1d87..000000000 --- a/packages/gitbook/src/lib/api-token.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { jwtDecode } from 'jwt-decode'; - -import type { SiteAPIToken } from '@gitbook/api'; - -/** - * Refresh slightly ahead of the real expiry so the token survives the rest of the request. - * The API mints tokens on a UTC-midnight bucket that stay valid until 00:05 the next day, so this - * margin must stay under that 5min overlap: outside it, a refresh returns the very same token. - */ -const EXPIRATION_MARGIN_SECONDS = 120; - -/** - * Check if a site API token is expired, or about to expire. - */ -export function isAPITokenExpired(apiToken: string): boolean { - try { - const decoded = jwtDecode(apiToken); - return ( - typeof decoded.exp === 'number' && - decoded.exp < Date.now() / 1000 + EXPIRATION_MARGIN_SECONDS - ); - } catch { - // A token we cannot decode is not one we can refresh. - return false; - } -} diff --git a/packages/gitbook/src/lib/async.test.ts b/packages/gitbook/src/lib/async.test.ts deleted file mode 100644 index 8d371e88a..000000000 --- a/packages/gitbook/src/lib/async.test.ts +++ /dev/null @@ -1,382 +0,0 @@ -import { describe, expect, it } from 'bun:test'; - -import { race } from './async'; -import { flushWaitUntil } from './waitUntil'; - -describe('race', () => { - it('should return on the first result and delay the others', async () => { - const result = await race( - [ - [2, 20], - [1, 10], - [3, 30], - ], - async ([value, timeout]) => { - await new Promise((resolve) => setTimeout(resolve, timeout)); - return value; - } - ); - - expect(result).toBe(1); - expect(await flushWaitUntil()).toHaveLength(3); - }); - - it('should ignore null resolutions', async () => { - const result = await race( - [ - [2, 20], - [null, 10], - [3, 30], - ] as const, - async ([value, timeout]) => { - await new Promise((resolve) => setTimeout(resolve, timeout)); - return value; - } - ); - - expect(result).toBe(2); - expect(await flushWaitUntil()).toHaveLength(2); - }); - - it('should ignore errors', async () => { - const result = await race( - [ - [new Error('test'), 20], - [null, 10], - [3, 30], - ] as const, - async ([value, timeout]) => { - await new Promise((resolve) => setTimeout(resolve, timeout)); - - if (value instanceof Error) { - throw value; - } - - return value; - } - ); - - expect(result).toBe(3); - expect(await flushWaitUntil()).toHaveLength(1); - }); - - it('should pass an abort signal to each input', async () => { - const result = await race( - [ - [2, 20], - [1, 10], - [3, 30], - ], - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - return value; - } - ); - - expect(result).toBe(1); - expect(await flushWaitUntil()).toHaveLength(3); - }); - - it('should respect timeout', async () => { - const result = await race( - [ - [1, 10], - [2, 20], - [3, 30], - ], - async ([value, timeout]) => { - await new Promise((resolve) => setTimeout(resolve, timeout)); - return value; - }, - { - timeout: 5, - } - ); - - expect(result).toBe(null); - expect(await flushWaitUntil()).toHaveLength(3); - }); - - describe('signal', () => { - it('should abort all inputs', async () => { - let abortted = 0; - - const abort = new AbortController(); - - setTimeout(() => { - abort.abort(); - }, 5); - - await expect( - race( - [ - [2, 20], - [1, 10], - [3, 30], - ], - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - abortted += 1; - - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - return value; - }, - { - signal: abort.signal, - } - ) - ).rejects.toThrow('The operation was aborted.'); - - expect(abortted).toBe(3); - }); - }); - - describe('blockTimeout', () => { - it('should fallback if blockTimeout is reached', async () => { - const result = await race( - [ - [null, 10], - [2, 200], - [1, 100], - [3, 300], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - return value; - }, - { - blockTimeout: 20, - blockFallback: async () => { - return 4; - }, - } - ); - - expect(result).toBe(4); - expect(await flushWaitUntil()).toHaveLength(3); - }); - - it('should resolve only if fallback if blockTimeout is reached', async () => { - const result = await race( - [ - [null, 10], - [2, 15], - [1, 100], - [3, 300], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - return value; - }, - { - blockTimeout: 20, - blockFallback: async () => { - return 4; - }, - } - ); - - expect(result).toBe(2); - expect(await flushWaitUntil()).toHaveLength(3); - }); - - it('should resolve with fallback if all inputs resolved to null', async () => { - const result = await race( - [ - [null, 20], - [null, 25], - [null, 30], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - return value; - }, - { - blockTimeout: 15, - blockFallback: async () => { - // Wait for all inputs to resolve - await new Promise((resolve) => setTimeout(resolve, 30)); - return 4; - }, - } - ); - - expect(result).toBe(4); - - const pendings = await flushWaitUntil(); - expect(pendings).toHaveLength(0); - }); - - it('should throw error from fallback if fallbackOnNull is set to true after inputs finished', async () => { - await expect( - race( - [ - [null, 3], - [null, 4], - [null, 5], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - - return value; - }, - { - blockTimeout: 15, - blockFallback: async () => { - // Wait for all inputs to resolve - throw new Error('Fallback error'); - }, - fallbackOnNull: true, - } - ) - ).rejects.toThrow('Fallback error'); - - const pendings = await flushWaitUntil(); - expect(pendings).toHaveLength(0); - }); - - it('should throw error from fallback if fallbackOnNull is set to true before inputs finished with null', async () => { - await expect( - race( - [ - [null, 30], - [null, 40], - [null, 50], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - - return value; - }, - { - blockTimeout: 5, - blockFallback: async () => { - throw new Error('Fallback error'); - }, - fallbackOnNull: true, - } - ) - ).rejects.toThrow('Fallback error'); - - const pendings = await flushWaitUntil(); - expect(pendings).toHaveLength(0); - }); - - it('should not throw error from fallback if fallbackOnNull is set to true before inputs finished with non-null null', async () => { - const result = await race( - [ - [null, 10], - ['done', 15], - [null, 20], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - - return value; - }, - { - blockTimeout: 5, - blockFallback: async () => { - throw new Error('Fallback error'); - }, - fallbackOnNull: true, - } - ); - expect(result).toBe('done'); - - const pendings = await flushWaitUntil(); - expect(pendings).toHaveLength(2); - }); - - it('should throw the error if the blockFallback fails after the inputs with fallbackOnNull=false', async () => { - await expect( - race( - [ - [null, 10], - [null, 15], - [null, 20], - ] as const, - async ([value, timeout], { signal }) => { - await new Promise((resolve, reject) => { - const timeoutId = setTimeout(resolve, timeout); - - signal.addEventListener('abort', () => { - clearTimeout(timeoutId); - reject(new Error('Aborted')); - }); - }); - - return value; - }, - { - blockTimeout: 5, - blockFallback: async () => { - // Sleep 30ms to make sure all inputs are resolved - await new Promise((resolve) => setTimeout(resolve, 30)); - throw new Error('Fallback error'); - }, - fallbackOnNull: false, - } - ) - ).rejects.toThrow('Fallback error'); - - const pendings = await flushWaitUntil(); - expect(pendings).toHaveLength(0); - }); - }); -}); diff --git a/packages/gitbook/src/lib/async.ts b/packages/gitbook/src/lib/async.ts index 4e702f34e..35d2ae0e4 100644 --- a/packages/gitbook/src/lib/async.ts +++ b/packages/gitbook/src/lib/async.ts @@ -1,222 +1,3 @@ -import { waitUntil } from './waitUntil'; - -/** - * Execute a function for each input in parallel and return the first result. - * It is designed to support executing in a worker environment where async execution outside the request lifecycle is not allowed. - * And also to support promise cancellation and optional result. - * - * Pattern 1: it waits for the first input to resolve with a non-null result. - * I1: |-----------------| (r1) - * I2: |---| (null) - * I3: |-------| (r3) <- The function will return r3. - * - * Pattern 2: it returns `null` if the timeout is reached. - * T: |------------| (timeout) <- The function will return `null`. - * I1: |-----------------| (r1) - * I2: |--------------| (null) - * I3: |------------------| (r3) - * - * Pattern 3: it sends a signal to stop pending operations as soon as one resolves. - * I1: |-------x cancelled - * I2: |---| (null) - * I3: |-------| (r3) <- The function will return r3. - * - * Pattern 4: it starts the blockFallback function if the blockTimeout is reached before any input resolves. - * BT: |-----------------| (blockTimeout) - * F: |---| (fallback) <- The function will return the result of the fallback. - * I1: |---------------------x cancelled - * I2: |---------------------x cancelled - * I3: |---------------------x cancelled - * - * Pattern 5: it resolves with the first resolution even after fallback was started - * BT: |-----------------| (blockTimeout) - * F: |---x fallback cancelled - * I1: |---------------------| (r1) <- The function will return r1. - * I2: |-------------------| (null) - * I3: |---------------------x cancelled - */ -export async function race( - inputs: I[], - execute: (input: I, options: { signal: AbortSignal }) => Promise, - options: { - /** - * Signal to cancel all operations. - */ - signal?: AbortSignal; - - /** - * Timeout in milliseconds before all operations are cancelled. - */ - timeout?: number; - - /** - * Timeout in milliseconds. - * if the timeout is reached, the function will start calling the `blockFallback` function. - */ - blockTimeout?: number; - - /** - * Fallback to call when the timeout is reached. - * If an input resolve before the timeout, the fallback will be ignored. - * If an input resolve while the fallback is running, the fallback will be cancelled. - */ - blockFallback?: (options: { signal: AbortSignal }) => Promise; - - /** - * If true, the `blockFallback` will be called if all inputs resolved with `null`. - * @default false - */ - fallbackOnNull?: boolean; - } = {} -): Promise { - const { signal, timeout, blockTimeout, blockFallback, fallbackOnNull = false } = options; - const result = await new Promise((resolve, reject) => { - let resolved = false; - let pending = inputs.length; - let timeoutId: ReturnType | null = null; - let blockFallbackStarted = false; - let blockFallbackRunning = false; - let blockFallbackError: Error | null = null; - let blockTimeoutId: ReturnType | null = null; - const abort = new AbortController(); - - const done = () => { - resolved = true; - if (timeoutId) { - clearTimeout(timeoutId); - } - if (blockTimeoutId) { - clearTimeout(blockTimeoutId); - } - abort.abort(); - }; - - const rejectWith = (error: Error) => { - if (!resolved) { - done(); - reject(error); - } - }; - - const resolveWith = (value: R | null) => { - if (!resolved) { - done(); - resolve(value); - } - }; - - if (signal) { - // If top function is cancelling, we should cancel all pending operations - signal.addEventListener('abort', () => { - resolveWith(null); - }); - } - - const runFallback = () => { - if (!blockFallback) { - throw new Error('blockFallback is required'); - } - - if (blockFallbackRunning || blockFallbackStarted) { - throw new Error('blockFallback already started'); - } - - if (blockTimeoutId) { - clearTimeout(blockTimeoutId); - } - - blockFallbackStarted = true; - blockFallbackRunning = true; - waitUntil( - blockFallback({ - signal: abort.signal, - }).then( - (fallbackResult) => { - resolveWith(fallbackResult); - }, - (error) => { - logIgnoredError('blockFallback failed with', error); - - if (pending === 0) { - rejectWith(error); - } else { - blockFallbackError = error; - blockFallbackRunning = false; - } - } - ) - ); - }; - - if (timeout) { - timeoutId = setTimeout(() => { - resolveWith(null); - }, timeout); - } - - if (blockTimeout) { - if (!blockFallback) { - throw new Error('blockTimeout requires blockFallback'); - } - - blockTimeoutId = setTimeout(runFallback, blockTimeout); - } - - inputs.forEach((input, inputIndex) => { - waitUntil( - execute(input, { signal: abort.signal }) - .then( - (inputResult) => { - if (abort.signal.aborted) { - // Ignore input results if the blockFallback has started - return; - } - - if (inputResult !== null) { - // Only resolve if we actually got a non-null result - resolveWith(inputResult); - } - }, - (error) => { - // Ignore errors - logIgnoredError(`input ${inputIndex} failed with`, error); - } - ) - .finally(() => { - pending -= 1; - if (pending === 0 && !resolved && !blockFallbackRunning) { - if (fallbackOnNull && !blockFallbackStarted) { - runFallback(); - } else { - if (blockFallbackError) { - rejectWith(blockFallbackError); - } else { - resolveWith(null); - } - } - } - }) - ); - }); - }); - - // Since the cancellation of the signal will cause the promise to resolve to `null`, - // we need to throw an error to indicate that the operation was cancelled. - signal?.throwIfAborted(); - - return result; -} - -/** - * Log and ignore an error. - * It skips the error if it's an AbortError. - */ -function logIgnoredError(_message: string, error: Error) { - if (error.name === 'AbortError' || process.env.NODE_ENV === 'test') { - return; - } -} - /** * Try catch a promise and return the result or the error. */ diff --git a/packages/gitbook/src/lib/data/lookup.test.ts b/packages/gitbook/src/lib/data/lookup.test.ts index 878a3a5ce..b5492c15b 100644 --- a/packages/gitbook/src/lib/data/lookup.test.ts +++ b/packages/gitbook/src/lib/data/lookup.test.ts @@ -4,6 +4,7 @@ import type { PublishedSiteContentLookup } from '@gitbook/api'; import { GITBOOK_PREVIEW_BASE_URL } from '../env'; import * as api from './api'; +import { DataFetcherError } from './errors'; import { lookupPublishedContentByUrl } from './lookup'; describe('preview auth redirects', () => { @@ -106,6 +107,55 @@ describe('preview auth redirects', () => { ); }); +describe('lookup URL', () => { + it('resolves the full URL without search params in a single call', async () => { + const urls: string[] = []; + const apiClientSpy = spyOn(api, 'apiClient').mockReturnValue({ + urls: { + async resolvePublishedContentByUrl({ url }: { url: string }) { + urls.push(url); + return { data: { target: 'external', redirect: 'https://example.com' } }; + }, + }, + } as unknown as ReturnType); + + try { + await lookupPublishedContentByUrl({ + url: 'https://docs.example.com/v/variant/a/b/c/d/e?theme=dark', + apiToken: null, + redirectOnError: false, + visitorPayload: {}, + }); + } finally { + apiClientSpy.mockRestore(); + } + + expect(urls).toEqual(['https://docs.example.com/v/variant/a/b/c/d/e']); + }); + + it('returns the API error', async () => { + const apiClientSpy = spyOn(api, 'apiClient').mockReturnValue({ + urls: { + async resolvePublishedContentByUrl() { + throw new DataFetcherError('Not found', 404); + }, + }, + } as unknown as ReturnType); + + try { + const result = await lookupPublishedContentByUrl({ + url: 'https://docs.example.com/a', + apiToken: null, + redirectOnError: false, + visitorPayload: {}, + }); + expect(result.error?.code).toBe(404); + } finally { + apiClientSpy.mockRestore(); + } + }); +}); + describe('visitor payload', () => { it('forwards the visitor country to the API', async () => { const calls: { visitor?: unknown }[] = []; diff --git a/packages/gitbook/src/lib/data/lookup.ts b/packages/gitbook/src/lib/data/lookup.ts index 081d49162..76ec665bf 100644 --- a/packages/gitbook/src/lib/data/lookup.ts +++ b/packages/gitbook/src/lib/data/lookup.ts @@ -3,10 +3,8 @@ import type { GitBookAPI, PublishedSiteContentLookup, SiteVisitorPayload } from import { apiClient } from './api'; import { getExposableError } from './errors'; import type { DataFetcherResponse } from './types'; -import { getURLLookupAlternatives, getURLLookupPathname, stripURLSearch } from './urls'; -import { isAPITokenExpired } from '@/lib/api-token'; -import { race, tryCatch } from '@/lib/async'; -import { getLogger } from '@/lib/logger'; +import { getURLLookup, getURLLookupPathname, stripURLSearch } from './urls'; +import { tryCatch } from '@/lib/async'; import { joinPath, joinPathWithBaseURL } from '@/lib/paths'; import { isPreviewRequest } from '@/lib/preview'; import { trace } from '@/lib/tracing'; @@ -22,137 +20,14 @@ interface LookupPublishedContentByUrlInput { /** * Lookup a content by its URL using the GitBook resolvePublishedContentByUrl API endpoint. - * To optimize caching, we try multiple lookup alternatives and return the first one that matches. */ export async function lookupPublishedContentByUrl( input: LookupPublishedContentByUrlInput ): Promise> { const lookupURL = new URL(input.url); - const url = stripURLSearch(lookupURL); - const lookup = getURLLookupAlternatives(url); + const lookup = getURLLookup(stripURLSearch(lookupURL)); - const result = await race(lookup.urls, async (alternative, { signal }) => { - const api = apiClient({ apiToken: input.apiToken }); - const resolveURL = (cacheBust?: string) => - tryCatch( - api.urls.resolvePublishedContentByUrl( - { - url: alternative.url, - ...(input.visitorPayload ? { visitor: input.visitorPayload } : {}), - redirectOnError: input.redirectOnError, - // Temporary: the API caches this POST by request body, so an unknown - // field is enough to miss the cache and get a freshly minted token. - ...(cacheBust ? { cacheBust } : {}), - } as ResolveBody, //TODO: remove cast when we are sure that everything is good - { signal } - ) - ); - - let callResult = await trace( - { - operation: 'resolvePublishedContentByUrl', - name: alternative.url, - }, - () => resolveURL() - ); - - const resolved = callResult.error ? null : callResult.data.data; - if ( - resolved && - !('redirect' in resolved) && - // Only alternatives we'd actually accept are worth a second round-trip. - (alternative.primary || resolved.complete) && - isAPITokenExpired(resolved.apiToken) - ) { - getLogger().warn( - 'resolvePublishedContentByUrl returned an expired API token, retrying without cache' - ); - callResult = await trace( - { - operation: 'resolvePublishedContentByUrlUncached', - name: alternative.url, - }, - // The value only needs to differ between calls, so `Math.random` is enough. - () => resolveURL(Math.random().toString(36).slice(2)) - ); - } - - if (callResult.error) { - if (alternative.primary) { - // We only return an error for the primary alternative (full URL), - // as other parts could result in errors due to the URL being incomplete (share links, etc). - return { error: callResult.error }; - } - return null; - } - - const { - data: { data }, - } = callResult; - - if ('redirect' in data) { - if (alternative.primary) { - if (data.target === 'application' && isPreviewRequest(lookupURL)) { - // The cached lookup omits content selectors and query params needed after login. - const redirect = new URL(data.redirect); - redirect.searchParams.set('redirect', lookupURL.toString()); - return { data: { ...data, redirect: redirect.toString() } }; - } - - // Append the path to the redirect URL - // because we might have matched a shorter path and the redirect is relative to it - if (alternative.extraPath) { - if (data.target === 'content') { - const redirect = new URL(data.redirect); - redirect.pathname = joinPath(redirect.pathname, alternative.extraPath); - data.redirect = redirect.toString(); - } else { - const redirect = new URL(data.redirect); - if (redirect.searchParams.has('location')) { - redirect.searchParams.set( - 'location', - joinPath( - redirect.searchParams.get('location') ?? '', - alternative.extraPath - ) - ); - data.redirect = redirect.toString(); - } - } - } - - return { data }; - } - - return null; - } - - /** - * We use the following criteria to determine if the lookup result is the right one: - * - the primary alternative was resolved (because that's the longest or most inclusive path) - * - the resolution of the site URL is complete (because we want to resolve the deepest path possible) - * - * In both cases, the idea is to use the deepest/longest/most inclusive path to resolve the content. - */ - if (alternative.primary || ('site' in data && data.complete)) { - const changeRequest = data.changeRequest ?? lookup.changeRequest; - const revision = data.revision ?? lookup.revision; - - const siteResult: PublishedSiteContentLookup = { - ...data, - canonicalUrl: joinPathWithBaseURL(data.canonicalUrl, alternative.extraPath), - basePath: joinPath(data.basePath, lookup.basePath ?? ''), - pathname: getURLLookupPathname(alternative, data), - ...(changeRequest ? { changeRequest } : {}), - ...(revision ? { revision } : {}), - }; - return { data: siteResult }; - } - - return null; - }); - - if (!result) { + if (!lookup) { return { error: { code: 404, @@ -161,13 +36,73 @@ export async function lookupPublishedContentByUrl( }; } - if (result.error) { + const api = apiClient({ apiToken: input.apiToken }); + const callResult = await trace( + { + operation: 'resolvePublishedContentByUrl', + name: lookup.url, + }, + () => + tryCatch( + api.urls.resolvePublishedContentByUrl({ + url: lookup.url, + ...(input.visitorPayload ? { visitor: input.visitorPayload } : {}), + redirectOnError: input.redirectOnError, + } as ResolveBody) //TODO: remove cast when we are sure that everything is good + ) + ); + + if (callResult.error) { return { - error: getExposableError(result.error), + error: getExposableError(callResult.error), }; } + const { + data: { data }, + } = callResult; + + if ('redirect' in data) { + if (data.target === 'application' && isPreviewRequest(lookupURL)) { + // The cached lookup omits content selectors and query params needed after login. + const redirect = new URL(data.redirect); + redirect.searchParams.set('redirect', lookupURL.toString()); + return { data: { ...data, redirect: redirect.toString() } }; + } + + // Append the path to the redirect URL + // because we resolved a shorter path and the redirect is relative to it + if (lookup.extraPath) { + if (data.target === 'content') { + const redirect = new URL(data.redirect); + redirect.pathname = joinPath(redirect.pathname, lookup.extraPath); + data.redirect = redirect.toString(); + } else { + const redirect = new URL(data.redirect); + if (redirect.searchParams.has('location')) { + redirect.searchParams.set( + 'location', + joinPath(redirect.searchParams.get('location') ?? '', lookup.extraPath) + ); + data.redirect = redirect.toString(); + } + } + } + + return { data }; + } + + const changeRequest = data.changeRequest ?? lookup.changeRequest; + const revision = data.revision ?? lookup.revision; + return { - data: result.data, + data: { + ...data, + canonicalUrl: joinPathWithBaseURL(data.canonicalUrl, lookup.extraPath), + basePath: joinPath(data.basePath, lookup.basePath ?? ''), + pathname: getURLLookupPathname(lookup, data), + ...(changeRequest ? { changeRequest } : {}), + ...(revision ? { revision } : {}), + }, }; } diff --git a/packages/gitbook/src/lib/data/urls.test.ts b/packages/gitbook/src/lib/data/urls.test.ts index 72dd5f840..b11b6eabf 100644 --- a/packages/gitbook/src/lib/data/urls.test.ts +++ b/packages/gitbook/src/lib/data/urls.test.ts @@ -1,11 +1,6 @@ import { describe, expect, it } from 'bun:test'; -import { - getURLLookupAlternatives, - getURLLookupPathname, - normalizeURL, - shouldBypassLookupAlternatives, -} from './urls'; +import { getURLLookup, getURLLookupPathname, normalizeURL } from './urls'; describe('getURLLookupPathname', () => { const previewRoot = 'https://sites.gitbook.com/preview/site_example/section'; @@ -15,13 +10,12 @@ describe('getURLLookupPathname', () => { it.each(['revisions', 'changes'])( 'resolves a page in a %s preview from the site-space root', (kind) => { - const lookup = getURLLookupAlternatives( + const lookup = getURLLookup( new URL(`${previewRoot}/~/${kind}/revision-id/${pagePath}`) - ); - const alternative = lookup.urls[0]!; + )!; expect( - getURLLookupPathname(alternative, { + getURLLookupPathname(lookup, { basePath: '/preview/site_example/section/', pathname: homepagePath, }) @@ -32,12 +26,10 @@ describe('getURLLookupPathname', () => { it.each(['revisions', 'changes'])( 'preserves the custom homepage at a %s preview root', (kind) => { - const lookup = getURLLookupAlternatives( - new URL(`${previewRoot}/~/${kind}/revision-id/`) - ); + const lookup = getURLLookup(new URL(`${previewRoot}/~/${kind}/revision-id/`))!; expect( - getURLLookupPathname(lookup.urls[0]!, { + getURLLookupPathname(lookup, { basePath: '/preview/site_example/section/', pathname: homepagePath, }) @@ -45,27 +37,22 @@ describe('getURLLookupPathname', () => { } ); - it.each(['', '/section/variant'])('resolves a published page below the %s root', (basePath) => { - const rootURL = `https://docs.example.com${basePath}`; - const lookup = getURLLookupAlternatives(new URL(`${rootURL}/${pagePath}`)); - const alternative = lookup.urls.find(({ url }) => url === new URL(rootURL).toString())!; + it('uses the resolved pathname for a full URL lookup', () => { + const lookup = getURLLookup(new URL(`https://docs.example.com/section/${pagePath}`))!; - expect(alternative).toBeDefined(); expect( - getURLLookupPathname(alternative, { - basePath: `${basePath}/`, - pathname: homepagePath, + getURLLookupPathname(lookup, { + basePath: '/section/', + pathname: `/${pagePath}`, }) - ).toBe(`/${pagePath}`); + ).toBe(`/${pagePath}/`); }); it('preserves page resolution without a custom homepage', () => { - const lookup = getURLLookupAlternatives( - new URL(`${previewRoot}/~/revisions/revision-id/${pagePath}`) - ); + const lookup = getURLLookup(new URL(`${previewRoot}/~/revisions/revision-id/${pagePath}`))!; expect( - getURLLookupPathname(lookup.urls[0]!, { + getURLLookupPathname(lookup, { basePath: '/preview/site_example/section/', pathname: '/', }) @@ -88,617 +75,90 @@ describe('getURLLookupPathname', () => { ); it('preserves a legitimate page prefix for a non-root lookup', () => { - const lookup = getURLLookupAlternatives( - new URL('https://docs.example.com/section/guide/page') - ); - const alternative = lookup.urls.find( - ({ url }) => url === 'https://docs.example.com/section/guide' - )!; - expect( - getURLLookupPathname(alternative, { - basePath: '/section/', - pathname: '/guide', - }) + getURLLookupPathname( + { url: 'https://docs.example.com/section/guide', extraPath: 'page' }, + { basePath: '/section/', pathname: '/guide' } + ) ).toBe('/guide/page'); }); }); -describe('getURLLookupAlternatives', () => { - it('should return all URLs up to the root', () => { - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/b/c'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - extraPath: 'a/b/c', - url: 'https://docs.mycompany.com/', - primary: false, - }, - { - extraPath: 'b/c', - url: 'https://docs.mycompany.com/a', - primary: false, - }, - { - extraPath: 'c', - url: 'https://docs.mycompany.com/a/b', - primary: false, - }, - { - extraPath: '', - url: 'https://docs.mycompany.com/a/b/c', - primary: true, - }, - ], - }); - - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/b/c/d'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - extraPath: 'a/b/c/d', - url: 'https://docs.mycompany.com/', - primary: false, - }, - { - extraPath: 'b/c/d', - url: 'https://docs.mycompany.com/a', - primary: false, - }, - { - extraPath: 'c/d', - url: 'https://docs.mycompany.com/a/b', - primary: false, - }, - { - extraPath: 'd', - url: 'https://docs.mycompany.com/a/b/c', - primary: false, - }, - { - extraPath: '', - url: 'https://docs.mycompany.com/a/b/c/d', - primary: true, - }, - ], - }); +describe('getURLLookup', () => { + it.each([ + 'https://docs.mycompany.com/', + 'https://docs.mycompany.com/a/b/c/d/e', + 'https://test.gitbook.io/space', + 'https://test.gitbook.io/v/variant/space', + 'https://test.gitbook.io/sharelink/v/variant/space', + 'https://docs.mycompany.com/a/~/b/c/d', + 'https://docs.mycompany.com/section/variant/~gitbook/ogimage/pageId', + 'https://org.gitbook.io/~gitbook/icon', + 'https://docs.mycompany.com/llms-full.txt/100', + 'https://docs.mycompany.com/section/blog/rss.xml', + 'https://docs.mycompany.com/a/b/intro.md', + 'https://proxy.gitbook.site/sites/site_foo/hello/world', + ])('looks up the full URL %s', (url) => { + expect(getURLLookup(new URL(url))).toEqual({ url, extraPath: '' }); }); - it('should not match before the variant for a variant url', () => { - expect( - getURLLookupAlternatives(new URL('https://test.gitbook.io/v/variant/space')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://test.gitbook.io/v/variant', - extraPath: 'space', - primary: true, - }, - ], - }); - }); - - it('should not match before the variant for a variant in a share link', () => { - expect( - getURLLookupAlternatives(new URL('https://test.gitbook.io/sharelink/v/variant/space')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://test.gitbook.io/sharelink/v/variant', - extraPath: 'space', - primary: true, - }, - ], - }); - }); - - it('should not match before a revision in a variant', () => { - expect( - getURLLookupAlternatives( - new URL('https://test.gitbook.io/v/variant/~/revisions/id/rest') - ) - ).toEqual({ - revision: 'id', - changeRequest: undefined, - basePath: '~/revisions/id', - urls: [ - { - url: 'https://test.gitbook.io/v/variant', - extraPath: 'rest', - primary: true, - }, - ], + it('looks up the normalized URL', () => { + expect(getURLLookup(new URL('https://docs.mycompany.com//hello/'))).toEqual({ + url: 'https://docs.mycompany.com/hello', + extraPath: '', }); }); it('should not match before a revision ID', () => { - expect( - getURLLookupAlternatives(new URL('https://docs.mycompany.com/~/revisions/id/a/b/c')) - ).toEqual({ - revision: 'id', - changeRequest: undefined, + expect(getURLLookup(new URL('https://docs.mycompany.com/~/revisions/id/a/b/c'))).toEqual({ + url: 'https://docs.mycompany.com/', + extraPath: 'a/b/c', basePath: '~/revisions/id', - urls: [ - { - extraPath: 'a/b/c', - url: 'https://docs.mycompany.com/', - primary: true, - }, - ], + revision: 'id', }); }); it('should not match before a change request ID', () => { - expect( - getURLLookupAlternatives(new URL('https://docs.mycompany.com/~/changes/id/hello')) - ).toEqual({ - revision: undefined, - changeRequest: 'id', + expect(getURLLookup(new URL('https://docs.mycompany.com/~/changes/id/hello'))).toEqual({ + url: 'https://docs.mycompany.com/', + extraPath: 'hello', basePath: '~/changes/id', - urls: [ - { - extraPath: 'hello', - url: 'https://docs.mycompany.com/', - primary: true, - }, - ], - }); - }); - - it('should normalize duplicated slashes', () => { - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com//hello'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - extraPath: 'hello', - url: 'https://docs.mycompany.com/', - primary: false, - }, - { - extraPath: '', - url: 'https://docs.mycompany.com/hello', - primary: true, - }, - ], - }); - }); - - it('should normalize trailing slash', () => { - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com/hello/'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - extraPath: 'hello', - url: 'https://docs.mycompany.com/', - primary: false, - }, - { - extraPath: '', - url: 'https://docs.mycompany.com/hello', - primary: true, - }, - ], - }); - }); - - it('should match a variant in a share-link', () => { - expect( - getURLLookupAlternatives(new URL('https://test.gitbook.io/sharelink/v/variant/space')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://test.gitbook.io/sharelink/v/variant', - extraPath: 'space', - primary: true, - }, - ], + changeRequest: 'id', }); }); it('should match a revision in a variant in a share-link', () => { expect( - getURLLookupAlternatives( + getURLLookup( new URL('https://test.gitbook.io/sharelink/v/variant/~/revisions/id/a/b/c') ) ).toEqual({ - revision: 'id', - changeRequest: undefined, + url: 'https://test.gitbook.io/sharelink/v/variant', + extraPath: 'a/b/c', basePath: '~/revisions/id', - urls: [ - { - url: 'https://test.gitbook.io/sharelink/v/variant', - extraPath: 'a/b/c', - primary: true, - }, - ], - }); - }); - - it('should match a change request in a variant in a share-link', () => { - expect( - getURLLookupAlternatives( - new URL('https://test.gitbook.io/sharelink/v/variant/~/changes/id/a/b/c') - ) - ).toEqual({ - revision: undefined, - changeRequest: 'id', - basePath: '~/changes/id', - urls: [ - { - url: 'https://test.gitbook.io/sharelink/v/variant', - extraPath: 'a/b/c', - primary: true, - }, - ], - }); - }); - - it('should limit depth', () => { - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/b/c/d/e'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - extraPath: 'a/b/c/d/e', - url: 'https://docs.mycompany.com/', - primary: false, - }, - { - extraPath: 'b/c/d/e', - url: 'https://docs.mycompany.com/a', - primary: false, - }, - { - extraPath: 'c/d/e', - url: 'https://docs.mycompany.com/a/b', - primary: false, - }, - { - extraPath: 'd/e', - url: 'https://docs.mycompany.com/a/b/c', - primary: false, - }, - { - extraPath: 'e', - url: 'https://docs.mycompany.com/a/b/c/d', - primary: true, - }, - ], - }); - }); - - it('should ignore dummy ~', () => { - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/~/b/c/d'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - extraPath: 'a/~/b/c/d', - url: 'https://docs.mycompany.com/', - primary: false, - }, - { - extraPath: '~/b/c/d', - url: 'https://docs.mycompany.com/a', - primary: false, - }, - { - extraPath: 'b/c/d', - url: 'https://docs.mycompany.com/a/~', - primary: false, - }, - { - extraPath: 'c/d', - url: 'https://docs.mycompany.com/a/~/b', - primary: false, - }, - { - extraPath: 'd', - primary: true, - url: 'https://docs.mycompany.com/a/~/b/c', - }, - ], - }); - }); - - it('should match a root gitbook.io domain', () => { - expect(getURLLookupAlternatives(new URL('https://test.gitbook.io/'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://test.gitbook.io/', - extraPath: '', - primary: true, - }, - ], - }); - }); - - it('should skip root gitbook.io domain if a path is present', () => { - expect(getURLLookupAlternatives(new URL('https://test.gitbook.io/space'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://test.gitbook.io/space', - extraPath: '', - primary: true, - }, - ], - }); - }); - - it('should match a root custom domain', () => { - expect(getURLLookupAlternatives(new URL('https://docs.mycompany.com/'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: '', - primary: true, - }, - ], - }); - }); - - it('should skip proxy root URLs', () => { - expect( - getURLLookupAlternatives( - new URL('https://proxy.gitbook.site/sites/site_foo/hello/world') - ) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://proxy.gitbook.site/sites/site_foo', - extraPath: 'hello/world', - primary: false, - }, - { - url: 'https://proxy.gitbook.site/sites/site_foo/hello', - extraPath: 'world', - primary: false, - }, - { - url: 'https://proxy.gitbook.site/sites/site_foo/hello/world', - extraPath: '', - primary: true, - }, - ], - }); - }); - - it('should not match internal paths as part of the site URL', () => { - expect( - getURLLookupAlternatives( - new URL('https://docs.mycompany.com/section/variant/~gitbook/ogimage/pageId') - ) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: 'section/variant/~gitbook/ogimage/pageId', - primary: false, - }, - { - url: 'https://docs.mycompany.com/section', - extraPath: 'variant/~gitbook/ogimage/pageId', - primary: false, - }, - { - url: 'https://docs.mycompany.com/section/variant', - extraPath: '~gitbook/ogimage/pageId', - primary: true, - }, - ], - }); - }); - - it('should match the root of a gitbook.io domain for an internal path', () => { - expect(getURLLookupAlternatives(new URL('https://org.gitbook.io/~gitbook/icon'))).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://org.gitbook.io/', - extraPath: '~gitbook/icon', - primary: true, - }, - ], - }); - - expect( - getURLLookupAlternatives(new URL('https://org.gitbook.io/docs/~gitbook/search')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://org.gitbook.io/docs', - extraPath: '~gitbook/search', - primary: true, - }, - ], - }); - }); - - it('should not match site root files', () => { - for (const file of ['llms.txt', 'llms-full.txt/100', 'robots.txt', 'sitemap.xml']) { - expect(getURLLookupAlternatives(new URL(`https://docs.mycompany.com/${file}`))).toEqual( - { - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: file, - primary: true, - }, - ], - } - ); - } - }); - - it('should not match a RSS feed at the end of a page path', () => { - expect( - getURLLookupAlternatives(new URL('https://docs.mycompany.com/section/blog/rss.xml')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: 'section/blog/rss.xml', - primary: false, - }, - { - url: 'https://docs.mycompany.com/section', - extraPath: 'blog/rss.xml', - primary: false, - }, - { - url: 'https://docs.mycompany.com/section/blog', - extraPath: 'rss.xml', - primary: true, - }, - ], - }); - }); - - it('should keep matching up to the variant for an internal path', () => { - expect( - getURLLookupAlternatives( - new URL('https://test.gitbook.io/sharelink/v/variant/~gitbook/search') - ) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://test.gitbook.io/sharelink/v/variant', - extraPath: '~gitbook/search', - primary: true, - }, - ], + revision: 'id', }); }); it('should keep the revision base path for an internal path', () => { expect( - getURLLookupAlternatives( + getURLLookup( new URL('https://docs.mycompany.com/~/revisions/id/a/~gitbook/ogimage/pageId') ) ).toEqual({ - revision: 'id', - changeRequest: undefined, + url: 'https://docs.mycompany.com/', + extraPath: 'a/~gitbook/ogimage/pageId', basePath: '~/revisions/id', - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: 'a/~gitbook/ogimage/pageId', - primary: true, - }, - ], + revision: 'id', }); }); - it('should only match internal paths at the end of the URL', () => { - expect( - getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/llms.txt/b')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: 'a/llms.txt/b', - primary: false, - }, - { - url: 'https://docs.mycompany.com/a', - extraPath: 'llms.txt/b', - primary: false, - }, - { - url: 'https://docs.mycompany.com/a/llms.txt', - extraPath: 'b', - primary: false, - }, - { - url: 'https://docs.mycompany.com/a/llms.txt/b', - extraPath: '', - primary: true, - }, - ], - }); - }); - - it('should not treat a markdown page path as internal', () => { - expect( - getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/b/intro.md')) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [ - { - url: 'https://docs.mycompany.com/', - extraPath: 'a/b/intro.md', - primary: false, - }, - { - url: 'https://docs.mycompany.com/a', - extraPath: 'b/intro.md', - primary: false, - }, - { - url: 'https://docs.mycompany.com/a/b', - extraPath: 'intro.md', - primary: false, - }, - { - url: 'https://docs.mycompany.com/a/b/intro.md', - extraPath: '', - primary: true, - }, - ], - }); + it.each([ + 'https://proxy.gitbook.site/', + 'https://proxy.gitbook.site/sites', + 'https://proxy.gitbook.site/sites/~/changes/id/hello', + ])('should skip the proxy root URL %s', (url) => { + expect(getURLLookup(new URL(url))).toBeNull(); }); }); @@ -837,54 +297,3 @@ describe('normalizeURL with encoded paths', () => { expect(result.searchParams.get('filter')).toBe(risonValue); }); }); - -describe('getURLLookupAlternatives with bypass', () => { - it('only looks up the full URL', () => { - expect( - getURLLookupAlternatives(new URL('https://docs.mycompany.com/a/b/c'), { bypass: true }) - ).toEqual({ - revision: undefined, - changeRequest: undefined, - basePath: undefined, - urls: [{ url: 'https://docs.mycompany.com/a/b/c', extraPath: '', primary: true }], - }); - }); - - it('only looks up the full URL for a variant', () => { - expect( - getURLLookupAlternatives(new URL('https://test.gitbook.io/v/variant/space'), { - bypass: true, - }).urls - ).toEqual([ - { url: 'https://test.gitbook.io/v/variant/space', extraPath: '', primary: true }, - ]); - }); - - it.each(['revisions', 'changes'])('keeps the alternatives for %s', (kind) => { - const url = new URL(`https://docs.mycompany.com/a/~/${kind}/id/page`); - expect(getURLLookupAlternatives(url, { bypass: true })).toEqual( - getURLLookupAlternatives(url, { bypass: false }) - ); - }); -}); - -describe('shouldBypassLookupAlternatives', () => { - const bypassURLs = ['https://docs.mycompany.com/section', 'https://other.mycompany.com/']; - - it.each([ - 'https://docs.mycompany.com/section', - 'https://docs.mycompany.com/section/page', - 'https://other.mycompany.com/', - 'https://other.mycompany.com/page', - ])('matches %s', (url) => { - expect(shouldBypassLookupAlternatives(normalizeURL(new URL(url)), bypassURLs)).toBe(true); - }); - - it.each([ - 'https://docs.mycompany.com/sectionpage', - 'https://docs.mycompany.com/', - 'https://unknown.mycompany.com/section', - ])('does not match %s', (url) => { - expect(shouldBypassLookupAlternatives(normalizeURL(new URL(url)), bypassURLs)).toBe(false); - }); -}); diff --git a/packages/gitbook/src/lib/data/urls.ts b/packages/gitbook/src/lib/data/urls.ts index bad5f2b2f..3e2faae4c 100644 --- a/packages/gitbook/src/lib/data/urls.ts +++ b/packages/gitbook/src/lib/data/urls.ts @@ -1,11 +1,10 @@ -import { GITBOOK_DISABLE_LOOKUP_ALTERNATIVES } from '../env'; import { joinPath, removeTrailingSlash } from '../paths'; import { isProxyRootRequest } from '../proxy'; import { DataFetcherError, getExposableError } from './errors'; /** * Paths served by our own routes rather than by site content. They can never be part of a - * site URL, so we stop matching there instead of spending lookups on URLs that cannot resolve. + * site URL, so a revision or change marker is never searched for inside them. * Kept in sync with the site route handlers and `encodePathInSiteContent` in the middleware. */ const INTERNAL_PATHS = [ @@ -44,156 +43,48 @@ function getContentPathSegments(pathSegments: string[]): string[] { } /** - * Site URL prefixes resolved with the full URL only, for sites where a shorter alternative - * would resolve to the wrong content. - */ -const LOOKUP_ALTERNATIVES_BYPASS_URLS: string[] = ['https://proxy.gitbook.site/sites/site_p4Xo4']; - -/** - * Whether the lookup of this (normalized) URL should skip the shorter alternatives. - */ -export function shouldBypassLookupAlternatives( - url: URL, - bypassURLs: string[] = LOOKUP_ALTERNATIVES_BYPASS_URLS -): boolean { - if (GITBOOK_DISABLE_LOOKUP_ALTERNATIVES) { - return true; - } - - return bypassURLs.some((bypassURL) => { - const prefix = normalizeURL(new URL(bypassURL)); - const prefixPath = removeTrailingSlash(prefix.pathname); - return ( - prefix.origin === url.origin && - (removeTrailingSlash(url.pathname) === prefixPath || - url.pathname.startsWith(`${prefixPath}/`)) - ); - }); -} - -/** - * For a given GitBook URL, return a list of alternative URLs that could be matched against to lookup the content. - * The approach is optimized to aim at reusing cached lookup results as much as possible. + * For a given GitBook URL, return the URL to resolve with the API. * - * The cases can be: + * The full URL is resolved as-is, except for revisions and changes in the path + * (`~/revisions//` or `~/changes//`): these are resolved up to the tilde, + * and the rest of the path goes in the extraPath. * - * Custom hostname - * - Public content has a custom hostname: docs.company.com/ - * - Public content has a custom hostname with a share-link: docs.company.com// - * - Public content has a custom hostname with a variant: docs.company.com/v// - * - Public content has a custom hostname with a variant and a share-link: docs.company.com//v// - * - Revisions in a custom hostname: docs.company.com/~/revisions// - * - Changes in a custom hostname: docs.company.com/~/changes// - * - * Custom hostname in the organization or GitBook domain (company.gitbook.io) - * - Public content has a custom hostname in the organization: docs.company.com// - * - Public content has a custom hostname in the organization with a share-link: docs.company.com/// - * - Public content has a custom hostname in the organization with a variant: docs.company.com//v// - * - Public content has a custom hostname in the organization with a variant and a share-link: docs.company.com///v// + * Returns `null` for the root proxy URLs, which can never resolve to a site. */ -export function getURLLookupAlternatives(input: URL, options: { bypass?: boolean } = {}) { +export function getURLLookup(input: URL): { + url: string; + extraPath: string; + basePath?: string; + changeRequest?: string; + revision?: string; +} | null { const url = normalizeURL(input); - const bypass = options.bypass ?? shouldBypassLookupAlternatives(url); - - let basePath: string | undefined = undefined; - let changeRequest: string | undefined = undefined; - let revision: string | undefined = undefined; - const alternatives: { url: string; extraPath: string; primary: boolean }[] = []; - - const pushAlternative = (adding: URL, extraPath: string) => { - const existing = alternatives.find((alt) => alt.url === adding.toString()); - if (existing) { - if (existing.extraPath !== extraPath) { - throw new Error( - `Invalid extraPath ${extraPath} for url ${adding.toString()}, already set to ${ - existing.extraPath - }` - ); - } - return; - } - - // We don't want to push the url if it is the root proxy URL (either https://proxy.gitbook.site or https://proxy.gitbook.site/sites) - if (isProxyRootRequest(adding)) { - return; - } - - alternatives.push({ - url: adding.toString(), - extraPath, - primary: adding.toString() === url.toString(), - }); - }; const pathSegments = url.pathname.slice(1).split('/'); - // Only the content part of the path can be matched against a site URL, the rest goes in the extraPath. + // A `~` in an internal path (like `~gitbook/...`) is not a revision or change marker. const contentSegments = getContentPathSegments(pathSegments); const tildeIndex = contentSegments.indexOf('~'); - const vIndex = contentSegments.indexOf('v'); + const kind = tildeIndex >= 0 ? contentSegments[tildeIndex + 1] : undefined; - // URL looks like a specific content url (with ~/revisions/ or ~/changes/ in the path) - // We only start matching after the ~/revisions/ or ~/changes/ segment and we ignore everything before it - if ( - tildeIndex >= 0 && - (contentSegments[tildeIndex + 1] === 'revisions' || - contentSegments[tildeIndex + 1] === 'changes') - ) { - const revisionOrChangeIdIndex = tildeIndex + 2; - - basePath = contentSegments.slice(tildeIndex, revisionOrChangeIdIndex + 1).join('/'); - if (contentSegments[tildeIndex + 1] === 'revisions') { - revision = contentSegments[revisionOrChangeIdIndex]; - } else { - changeRequest = contentSegments[revisionOrChangeIdIndex]; - } - - // Match up to the tilde - const contentURL = new URL(url); - contentURL.pathname = contentSegments.slice(0, tildeIndex).join('/'); - pushAlternative(contentURL, pathSegments.slice(revisionOrChangeIdIndex + 1).join('/')); + if (kind !== 'revisions' && kind !== 'changes') { + return isProxyRootRequest(url) ? null : { url: url.toString(), extraPath: '' }; } - // Revisions and changes above still need their alternatives to extract the base path. - else if (bypass) { - pushAlternative(url, ''); + const idIndex = tildeIndex + 2; + const id = contentSegments[idIndex]; + const contentURL = new URL(url); + contentURL.pathname = contentSegments.slice(0, tildeIndex).join('/'); + + if (isProxyRootRequest(contentURL)) { + return null; } - // URL looks like a collection url (with /v/ in the path) - // We only start matching after the /v/ segment and we ignore everything before it - // to avoid potentially matching as a page not found under the default space in the collection - else if (vIndex >= 0 && contentSegments.length >= vIndex + 2) { - const collectionURL = new URL(url); - collectionURL.pathname = contentSegments.slice(0, vIndex + 2).join('/'); - - pushAlternative(collectionURL, pathSegments.slice(vIndex + 2).join('/')); - } else { - // Match only with the host, if it can be a custom hostname - // It should cover most cases of custom domains, and with caching, it should be fast. - if (!url.hostname.includes('.gitbook.io') || contentSegments.length === 0) { - const noPathURL = new URL(url); - noPathURL.pathname = '/'; - - pushAlternative(noPathURL, url.pathname.slice(1)); - } - - // Otherwise match with the first four segments of the path - for (let i = 1; i <= 4; i++) { - if (contentSegments.length >= i) { - const shortURL = new URL(url); - shortURL.pathname = contentSegments.slice(0, i).join('/'); - - pushAlternative(shortURL, pathSegments.slice(i).join('/')); - } - } - } - - // Mark the longuest entry to lookup as primary - alternatives.sort((a, b) => b.extraPath.length - a.extraPath.length); - if (alternatives.length > 0) { - alternatives[alternatives.length - 1]!.primary = true; - } - - return { urls: alternatives, basePath, changeRequest, revision }; + return { + url: contentURL.toString(), + extraPath: pathSegments.slice(idIndex + 1).join('/'), + basePath: contentSegments.slice(tildeIndex, idIndex + 1).join('/'), + ...(kind === 'revisions' ? { revision: id } : { changeRequest: id }), + }; } /** Combine a resolved lookup with the remaining requested page path. */ diff --git a/packages/gitbook/src/lib/env/globals.ts b/packages/gitbook/src/lib/env/globals.ts index 0f5d0271e..983d2391f 100644 --- a/packages/gitbook/src/lib/env/globals.ts +++ b/packages/gitbook/src/lib/env/globals.ts @@ -93,12 +93,6 @@ export const GITBOOK_DISABLE_TRACKING = Boolean( */ export const GITBOOK_DISABLE_INSIGHTS = process.env.GITBOOK_DISABLE_INSIGHTS === 'true'; -/** - * Whether to resolve site content with the full URL only, skipping the shorter lookup alternatives. - */ -export const GITBOOK_DISABLE_LOOKUP_ALTERNATIVES = - process.env.GITBOOK_DISABLE_LOOKUP_ALTERNATIVES === 'true'; - /** * Hostname serving the integrations. */ diff --git a/packages/gitbook/src/lib/visitors.test.ts b/packages/gitbook/src/lib/visitors.test.ts index f5e2f3c0b..4e0ee02df 100644 --- a/packages/gitbook/src/lib/visitors.test.ts +++ b/packages/gitbook/src/lib/visitors.test.ts @@ -13,8 +13,6 @@ import { getVisitorUnsignedClaims, isRevalidationRequest, normalizeVisitorURL, - parseHostnameList, - shouldSendVisitorCountry, } from './visitors'; describe('getVisitorAuthToken', () => { @@ -662,32 +660,3 @@ describe('getVisitorCountry', () => { ).toBe('FR'); }); }); - -describe('shouldSendVisitorCountry', () => { - it.each(['dev', 'preview', 'staging'])('should be enabled on the %p stage', (stage) => { - expect(shouldSendVisitorCountry('docs.example.com', stage)).toBe(true); - }); - - it.each(['production', undefined])( - 'should be disabled for other hostnames on the %p stage', - (stage) => { - expect(shouldSendVisitorCountry('docs.example.com', stage)).toBe(false); - } - ); -}); - -describe('parseHostnameList', () => { - it.each([undefined, '', ' , '])('should return an empty set for %p', (value) => { - expect(parseHostnameList(value).size).toBe(0); - }); - - it('should parse a single entry', () => { - expect(parseHostnameList('docs.foo.com')).toEqual(new Set(['docs.foo.com'])); - }); - - it('should trim, lowercase and drop empty entries', () => { - expect(parseHostnameList(' Docs.Foo.com , ,bar.io')).toEqual( - new Set(['docs.foo.com', 'bar.io']) - ); - }); -}); diff --git a/packages/gitbook/src/lib/visitors.ts b/packages/gitbook/src/lib/visitors.ts index 33a7ed937..9a70c53b4 100644 --- a/packages/gitbook/src/lib/visitors.ts +++ b/packages/gitbook/src/lib/visitors.ts @@ -106,33 +106,6 @@ export function getVisitorType(request: { return detection.detected && detection.method !== 'heuristic' ? 'agent' : 'human'; } -// Production hostnames for which the visitor country is sent while the feature is rolled out. -// Set at build time from a GitHub Actions variable to keep customer hostnames out of the code. -const VISITOR_COUNTRY_HOSTNAMES = parseHostnameList(process.env.GITBOOK_VISITOR_COUNTRY_HOSTNAMES); -const VISITOR_COUNTRY_STAGES = new Set(['dev', 'preview', 'staging']); - -/** - * Parse a comma-separated list of hostnames. - */ -export function parseHostnameList(value: string | undefined): Set { - return new Set( - (value ?? '') - .split(',') - .map((hostname) => hostname.trim().toLowerCase()) - .filter(Boolean) - ); -} - -/** - * Whether the visitor country should be sent when resolving the site URL. - */ -export function shouldSendVisitorCountry(hostname: string, stage: string | undefined): boolean { - return ( - (!!stage && VISITOR_COUNTRY_STAGES.has(stage)) || - VISITOR_COUNTRY_HOSTNAMES.has(hostname.toLowerCase()) - ); -} - /** * Get the ISO 3166-1 alpha-2 country code of the visitor from the geolocation headers, * or from `x-gitbook-country` when the request comes from the revalidation worker. diff --git a/packages/gitbook/src/middleware.ts b/packages/gitbook/src/middleware.ts index f1b19715b..42ea83585 100644 --- a/packages/gitbook/src/middleware.ts +++ b/packages/gitbook/src/middleware.ts @@ -35,12 +35,7 @@ import { normalizeRequestURL, throwIfDataError, } from '@/lib/data'; -import { - GITBOOK_DISABLE_INSIGHTS, - GITBOOK_STAGE, - isGitBookAssetsHostURL, - isGitBookHostURL, -} from '@/lib/env'; +import { GITBOOK_DISABLE_INSIGHTS, isGitBookAssetsHostURL, isGitBookHostURL } from '@/lib/env'; import { getImageResizingContextId } from '@/lib/images'; import { isAITrainingOrIndexingRequest } from '@/lib/indexing-crawlers'; import { MCP_SERVER_CARD_PATH, MCP_SERVER_CARD_WELL_KNOWN_PATH } from '@/lib/mcp/paths'; @@ -67,7 +62,6 @@ import { isRevalidationRequest, normalizeVisitorURL, serveVisitorClaimsDataRequest, - shouldSendVisitorCountry, } from '@/lib/visitors'; import { waitUntil } from '@/lib/waitUntil'; import { serveResizedImage } from '@/routes/image'; @@ -227,9 +221,7 @@ async function serveSiteRoutes(requestURL: URL, request: NextRequest) { // request.headers.delete('x-gitbook-disable-tracking'); - const visitorCountry = shouldSendVisitorCountry(siteRequestURL.hostname, GITBOOK_STAGE) - ? getVisitorCountry(request) - : undefined; + const visitorCountry = getVisitorCountry(request); const withAPIToken = async (apiToken: string | null) => { const siteURLData = await throwIfDataError(