diff --git a/.changeset/encode-path-parameters.md b/.changeset/encode-path-parameters.md new file mode 100644 index 00000000..2a89b612 --- /dev/null +++ b/.changeset/encode-path-parameters.md @@ -0,0 +1,5 @@ +--- +'@fingerprint/node-sdk': patch +--- + +URL-encode path parameters; reject "." and ".." path parameters with a TypeError. diff --git a/src/urlUtils.ts b/src/urlUtils.ts index e38ec6eb..bad0810a 100644 --- a/src/urlUtils.ts +++ b/src/urlUtils.ts @@ -43,6 +43,39 @@ function serializeQueryStringParams(params: QueryStringParameters): string { return urlSearchParams.toString() } +/** + * Confines a value to a single URL path segment. `.` is deliberately left unencoded because + * the Server API does not decode path parameters, and valid parameter values can contain dots. + * + * A value of `.` or `..` is rejected: `new URL()` drops such a segment even when the dots are + * encoded, so it cannot be expressed. See https://url.spec.whatwg.org/#double-dot-path-segment + */ +function encodePathParam(placeholder: string, value: unknown): string { + // Coerce before comparing, because an untyped caller can pass something that is not a string + // but stringifies to one. Both conversions throw on values only such a caller could pass: + // `String` when the value has no primitive representation, `encodeURIComponent` on a lone + // surrogate. Neither should escape as its own error type. + let param: string + let encoded: string + try { + // eslint-disable-next-line @typescript-eslint/no-base-to-string -- runtime validation + param = String(value ?? '') + encoded = encodeURIComponent(param) + } catch (cause) { + throw new TypeError(`Invalid path parameter for ${placeholder}`, { cause }) + } + + if (param === '') { + throw new TypeError(`Missing path parameter for ${placeholder}`) + } + + if (param === '.' || param === '..') { + throw new TypeError(`Invalid path parameter for ${placeholder}: ${param}`) + } + + return encoded +} + function getServerApiUrl(region: Region): string { switch (region) { case Region.EU: @@ -67,7 +100,7 @@ export interface GetRequestPathOptions { } /** - * Formats a URL for the FingerprintJS server API by replacing placeholders and + * Formats a URL for the Fingerprint Server API by replacing placeholders and * appending query string parameters. * * @internal @@ -92,19 +125,12 @@ export function getRequestPath({ // eslint-disable-next-line @typescript-eslint/no-unused-vars method: _, }: GetRequestPathOptions): string { - // Step 1: Extract the path parameters (placeholders) from the path - const placeholders = Array.from(path.matchAll(/{(.*?)}/g)).map((match) => match[1]) - - // Step 2: Replace the placeholders with provided pathParams - let formattedPath: string = `${apiVersion}${path}` - placeholders.forEach((placeholder, index) => { - const param = pathParams?.[index] - if (param !== undefined && param !== '') { - formattedPath = formattedPath.replace(`{${placeholder}}`, param) - } else { - throw new Error(`Missing path parameter for ${placeholder}`) - } - }) + // Replace each `{placeholder}` with its path parameter. The replacement is a function so + // that `$&` and friends in a parameter are not read as replacement patterns. + let index = 0 + const formattedPath = `${apiVersion}${path}`.replace(/{(.*?)}/g, (_, placeholder: string) => + encodePathParam(placeholder, pathParams?.[index++]) + ) const queryStringParameters: QueryStringParameters = { ...(queryParams ?? {}), @@ -115,5 +141,9 @@ export function getRequestPath({ url.pathname = formattedPath url.search = serializeQueryStringParams(queryStringParameters) + if (url.pathname !== `/${formattedPath}`) { + throw new TypeError('Invalid path: path changed during normalization') + } + return url.toString() } diff --git a/tests/mocked-responses-tests/pathParamEncodingTests.spec.ts b/tests/mocked-responses-tests/pathParamEncodingTests.spec.ts new file mode 100644 index 00000000..25508ad4 --- /dev/null +++ b/tests/mocked-responses-tests/pathParamEncodingTests.spec.ts @@ -0,0 +1,72 @@ +import { FingerprintServerApiClient, Region } from '../../src' +import { getIntegrationInfo } from '../../src/urlUtils' +import { describe, expect, it } from 'vitest' +import { mockFetch } from './mockFetch' + +/** + * A path parameter must never be able to change which endpoint or which host the SDK talks + * to, no matter what the caller passes in. + */ +describe('[Mocked response] Path parameter encoding', () => { + const apiKey = 'dummy_api_key' + const ii = `ii=${encodeURIComponent(getIntegrationInfo())}` + + const client = new FingerprintServerApiClient({ region: Region.EU, apiKey }) + + const emptyResponse = () => new Response(undefined, { headers: { 'content-length': '0' } }) + + const operations = [ + { + name: 'getEvent', + prefix: 'v4/events', + placeholder: 'event_id', + paramName: 'eventId', + call: (param: string) => client.getEvent(param), + }, + { + name: 'updateEvent', + prefix: 'v4/events', + placeholder: 'event_id', + paramName: 'eventId', + call: (param: string) => client.updateEvent(param, { suspect: true }), + }, + { + name: 'deleteVisitorData', + prefix: 'v4/visitors', + placeholder: 'visitor_id', + paramName: 'visitorId', + call: (param: string) => client.deleteVisitorData(param), + }, + ] as const + + describe.each(operations)('$name', ({ prefix, placeholder, paramName, call }) => { + // The full encoding table lives in the unit tests; these are the cases INTER-2499 asks to + // be pinned at the wire level for every method. + it.each([ + ['../events', '..%2Fevents'], + ['evil.com', 'evil.com'], + ['//evil.com', '%2F%2Fevil.com'], + ])('requests a single path segment for %j', async (param, encoded) => { + mockFetch.mockReturnValue(Promise.resolve(emptyResponse())) + + // `getEvent` rejects on the empty body; only the requested URL matters here. + await call(param).catch(() => undefined) + + const requestedUrl = mockFetch.mock.calls[0]?.[0] as string + expect(requestedUrl).toEqual(`https://eu.api.fpjs.io/${prefix}/${encoded}?${ii}`) + expect(new URL(requestedUrl).host).toEqual('eu.api.fpjs.io') + }) + + it.each(['.', '..'])('does not send a request for %j', async (param) => { + await expect(call(param)).rejects.toThrow(new TypeError(`Invalid path parameter for ${placeholder}: ${param}`)) + + expect(mockFetch).not.toHaveBeenCalled() + }) + + it('does not send a request for an empty parameter', async () => { + await expect(call('')).rejects.toThrow(new TypeError(`${paramName} is not set`)) + + expect(mockFetch).not.toHaveBeenCalled() + }) + }) +}) diff --git a/tests/unit-tests/urlUtilsTests.spec.ts b/tests/unit-tests/urlUtilsTests.spec.ts index f281a13a..a26d8939 100644 --- a/tests/unit-tests/urlUtilsTests.spec.ts +++ b/tests/unit-tests/urlUtilsTests.spec.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { Region, SearchEventsFilter } from '../../src' import { version } from '../../package.json' +import type { paths } from '../../src/generatedApiTypes' import { getRequestPath } from '../../src/urlUtils' const visitorId = 'TaDnMBz9XCpZNuSzFUqP' @@ -184,6 +185,16 @@ describe('getRequestPath', () => { }).toThrow('Missing path parameter for event_id') }) + it('disallows normalized path segments', () => { + expect(() => { + getRequestPath({ + path: '/visitors/../events' as keyof paths, + method: 'get', + pathParams: [], + }) + }).toThrow('Invalid path: path changed during normalization') + }) + it('encodes special characters', () => { const actual = getRequestPath({ path: '/events', @@ -197,3 +208,86 @@ describe('getRequestPath', () => { expect(actual).toEqual(expected) }) }) + +// Encoding does not depend on which parameter is being replaced, so these run against one +// path. That every operation routes through it is covered by the mocked-response tests. +describe('path parameter encoding', () => { + const eventPath = (param: unknown) => + getRequestPath({ + path: '/events/{event_id}', + method: 'get', + pathParams: [param] as string[], + region: Region.Global, + }) + + it.each([ + ['../events', '..%2Fevents'], + ['../', '..%2F'], + ['/../../events', '%2F..%2F..%2Fevents'], + ['evil.com', 'evil.com'], + ['//evil.com', '%2F%2Fevil.com'], + ['https://evil.com', 'https%3A%2F%2Fevil.com'], + ['a b#c?d', 'a%20b%23c%3Fd'], + ['%2e%2e', '%252e%252e'], + ['..%2fevents', '..%252fevents'], + ['..\\..', '..%5C..'], + ['$&', '%24%26'], + ['...', '...'], + // A placeholder in a parameter must not reach the next replacement, which would matter + // for a path with two placeholders. + ['{event_id}', '%7Bevent_id%7D'], + ['1626550679751.cVc5Pm', '1626550679751.cVc5Pm'], + // An untyped caller can pass something that is not a string but stringifies to one. + [new String('../events'), '..%2Fevents'], + ])('keeps %j inside a single path segment', (param, encoded) => { + expect(eventPath(param)).toEqual(`https://api.fpjs.io/v4/events/${encoded}?${ii}`) + }) + + it.each([ + ['.', '.', 'Invalid path parameter for event_id: .'], + ['..', '..', 'Invalid path parameter for event_id: ..'], + ['a String object', new String('..'), 'Invalid path parameter for event_id: ..'], + ['an object with a toString', { toString: () => '..' }, 'Invalid path parameter for event_id: ..'], + ['an array', ['..'], 'Invalid path parameter for event_id: ..'], + // A lone surrogate makes `encodeURIComponent` throw a `URIError` + ['a lone surrogate', '\ud800', 'Invalid path parameter for event_id'], + // These have no primitive representation, so `String` itself throws + ['an object without a prototype', Object.create(null), 'Invalid path parameter for event_id'], + [ + 'an object whose toString throws', + { + toString: () => { + throw new Error('boom') + }, + }, + 'Invalid path parameter for event_id', + ], + ])('rejects %s', (_, param, message) => { + expect(() => eventPath(param)).toThrow(new TypeError(message)) + }) + + it('preserves the cause when string coercion fails', () => { + const cause = new Error('boom') + const param = { + toString: () => { + throw cause + }, + } + + expect(() => eventPath(param)).toThrow( + expect.objectContaining({ + message: 'Invalid path parameter for event_id', + cause, + }) + ) + }) + + it.each([ + ['an empty string', ''], + ['an empty String object', new String('')], + ['null', null], + ['undefined', undefined], + ])('rejects %s as missing', (_, param) => { + expect(() => eventPath(param)).toThrow(new TypeError('Missing path parameter for event_id')) + }) +})