From e97c80b381cee926755b80353d7f790a815460b9 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Thu, 18 Jul 2024 13:18:10 +0200 Subject: [PATCH 1/8] feat: use URLPattern for request matching --- package.json | 2 +- pnpm-lock.yaml | 14 +- .../utils/matching/matchRequestUrl.test.ts | 362 ++++++++++++------ src/core/utils/matching/matchRequestUrl.ts | 208 +++++++--- 4 files changed, 420 insertions(+), 166 deletions(-) diff --git a/package.json b/package.json index 68dab022d..231ef62bd 100644 --- a/package.json +++ b/package.json @@ -146,9 +146,9 @@ "headers-polyfill": "^4.0.2", "is-node-process": "^1.2.0", "outvariant": "^1.4.2", - "path-to-regexp": "^6.2.0", "strict-event-emitter": "^0.5.1", "type-fest": "^4.9.0", + "urlpattern-polyfill": "^9.0.0", "yargs": "^17.7.2" }, "devDependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2aa6f5fa4..26ffdb9fc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -44,15 +44,15 @@ dependencies: outvariant: specifier: ^1.4.2 version: 1.4.2 - path-to-regexp: - specifier: ^6.2.0 - version: 6.2.1 strict-event-emitter: specifier: ^0.5.1 version: 0.5.1 type-fest: specifier: ^4.9.0 version: 4.14.0 + urlpattern-polyfill: + specifier: ^9.0.0 + version: 9.0.0 yargs: specifier: ^17.7.2 version: 17.7.2 @@ -6420,10 +6420,6 @@ packages: resolution: {integrity: sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ==} dev: true - /path-to-regexp@6.2.1: - resolution: {integrity: sha512-JLyh7xT1kizaEvcaXOQwOc2/Yhw6KZOvPf1S8401UyLk86CU79LN3vl7ztXGm/pZ+YjoyAJ4rxmHwbkBXJX+yw==} - dev: false - /path-type@4.0.0: resolution: {integrity: sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==} engines: {node: '>=8'} @@ -7950,6 +7946,10 @@ packages: resolution: {integrity: sha512-DOE84vZT2fEcl9gqCUTcnAw5ZY5Id55ikUcziSUntuEFL3pRvavg5kwDmTEUJkeCHInTlV/HexFomgYnzO5kdQ==} dev: true + /urlpattern-polyfill@9.0.0: + resolution: {integrity: sha512-WHN8KDQblxd32odxeIgo83rdVDE2bvdkb86it7bMhYZwWKJz0+O0RK/eZiHYnM+zgt/U7hAHOlCQGfjjvSkw2g==} + dev: false + /util-deprecate@1.0.2: resolution: {integrity: sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==} dev: true diff --git a/src/core/utils/matching/matchRequestUrl.test.ts b/src/core/utils/matching/matchRequestUrl.test.ts index 54575f709..8d31e325a 100644 --- a/src/core/utils/matching/matchRequestUrl.test.ts +++ b/src/core/utils/matching/matchRequestUrl.test.ts @@ -1,125 +1,269 @@ -/** - * @vitest-environment jsdom - */ -import { coercePath, matchRequestUrl } from './matchRequestUrl' - -describe('matchRequestUrl', () => { - test('returns true when matches against an exact URL', () => { - const match = matchRequestUrl( - new URL('https://test.mswjs.io'), - 'https://test.mswjs.io', - ) - expect(match).toHaveProperty('matches', true) - expect(match).toHaveProperty('params', {}) - }) - - test('returns true when matched against a wildcard', () => { - const match = matchRequestUrl(new URL('https://test.mswjs.io'), '*') - expect(match).toHaveProperty('matches', true) - expect(match).toHaveProperty('params', { - '0': 'https://test.mswjs.io/', - }) - }) - - test('returns true when matched against a RegExp', () => { - const match = matchRequestUrl( - new URL('https://test.mswjs.io'), - /test\.mswjs\.io/, - ) - expect(match).toHaveProperty('matches', true) - expect(match).toHaveProperty('params', {}) - }) - - test('returns path parameters when matched', () => { - const match = matchRequestUrl( - new URL('https://test.mswjs.io/user/abc-123'), - 'https://test.mswjs.io/user/:userId', - ) - expect(match).toHaveProperty('matches', true) - expect(match).toHaveProperty('params', { - userId: 'abc-123', - }) +// @vitest-environment jsdom +import { Match, matchRequestUrl } from './matchRequestUrl' + +test('supports RegExp', () => { + expect( + matchRequestUrl(new URL('https://test.mswjs.io'), /test\.mswjs\.io/), + ).toEqual({ + matches: true, + params: {}, + }) + + expect( + matchRequestUrl(new URL('https://example.com'), /test\.mswjs\.io/), + ).toEqual({ + matches: false, + params: {}, + }) +}) + +test('supports exact URL', () => { + expect( + matchRequestUrl(new URL('https://test.mswjs.io'), 'https://test.mswjs.io'), + ).toEqual({ + matches: true, + params: {}, + }) + + // Trailing slash doesn't matter. + expect( + matchRequestUrl(new URL('https://test.mswjs.io'), 'https://test.mswjs.io/'), + ).toEqual({ + matches: true, + params: {}, + }) + expect( + matchRequestUrl(new URL('https://test.mswjs.io/'), 'https://test.mswjs.io'), + ).toEqual({ + matches: true, + params: {}, + }) + + // Must not match completely different URLs. + expect( + matchRequestUrl(new URL('https://example.com'), 'https://test.mswjs.io'), + ).toEqual({ + matches: false, + params: {}, + }) +}) + +test('supports leading a wildcard as the entire pattern', () => { + expect(matchRequestUrl(new URL('https://test.mswjs.io'), '*')).toEqual( + { + matches: true, + params: { '0': 'https://test.mswjs.io/' }, + }, + ) + + expect( + matchRequestUrl(new URL('https://test.mswjs.io/path/here'), '*'), + ).toEqual({ + matches: true, + params: { '0': 'https://test.mswjs.io/path/here' }, + }) +}) + +test('supports leading a wildcard and path with a single unnamed group', () => { + expect( + matchRequestUrl(new URL('https://test.mswjs.io/user/123'), '*/user/*'), + ).toEqual({ + matches: true, + params: { '0': 'https://test.mswjs.io', '1': '123' }, + }) + + expect( + matchRequestUrl( + new URL('https://test.mswjs.io/user/123/456/789'), + '*/user/*', + ), + ).toEqual({ + matches: true, + params: { '0': 'https://test.mswjs.io', '1': '123/456/789' }, + }) +}) + +test('supports a wildcard and path with multiple unnamed groups', () => { + expect( + matchRequestUrl( + new URL('https://test.mswjs.io/user/123/bar/456'), + '*/user/*/bar/*', + ), + ).toEqual({ + matches: true, + params: { '0': 'https://test.mswjs.io', '1': '123', '2': '456' }, + }) +}) + +test('supports a leading wildcard and a path with wildcards and a single named group', () => { + expect( + matchRequestUrl( + new URL('https://test.mswjs.io/user/john/bar/456'), + '*/user/:name/bar/*', + ), + ).toEqual({ + matches: true, + params: { + '0': 'https://test.mswjs.io', + name: 'john', + '1': '456', + }, + }) +}) + +test('supports a leading wildcard and a path with wildcards and a multiple named group', () => { + expect( + matchRequestUrl( + new URL('https://test.mswjs.io/user/foo/john/bar/456'), + '*/user/*/:name/bar/*', + ), + ).toEqual({ + matches: true, + params: { + '0': 'https://test.mswjs.io', + '1': 'foo', + name: 'john', + '2': '456', + }, + }) +}) + +test('merges multiple same-named groups into an array of values', () => { + // Must match same-named groups in different URL components. + expect( + matchRequestUrl( + new URL('https://example.com/user/abc/bar'), + 'https://:segment.com/user/:segment/bar', + ), + ).toEqual({ + matches: true, + params: { + segment: ['example', 'abc'], + }, + }) + + // Must match same-named groups in the same URL component. + expect( + matchRequestUrl( + new URL('https://example.com/user/abc/bar/def'), + 'https://example.com/user/:segment/bar/:segment', + ), + ).toEqual({ + matches: true, + params: { + /** + * @note URLPattern doesn't support multiple same-named groups + * within the same URL component, to begin with. + * @see https://github.com/whatwg/urlpattern/issues/226 + * + * However, "path-to-regexp" does! So does Express, and so does MSW. + * But it "supports" it in a weird way. It doesn't throw but takes + * the latest value of the group and preceding values. + */ + segment: 'def', + }, }) +}) - test('decodes path parameters', () => { - const url = 'http://example.com:5001/example' - const match = matchRequestUrl( +test('decodes group matches', () => { + const url = 'http://example.com:5001/example' + expect( + matchRequestUrl( new URL(`https://test.mswjs.io/reflect-url/${encodeURIComponent(url)}`), 'https://test.mswjs.io/reflect-url/:url', - ) - expect(match).toHaveProperty('matches', true) - expect(match).toHaveProperty('params', { - url, - }) + ), + ).toEqual({ + matches: true, + params: { url }, }) +}) - test('returns false when does not match against the request URL', () => { - const match = matchRequestUrl( - new URL('https://test.mswjs.io'), - 'https://google.com', - ) - expect(match).toHaveProperty('matches', false) - expect(match).toHaveProperty('params', {}) +test('supports optional groups', () => { + // Must match the URL if the optional parameter is present. + expect( + matchRequestUrl( + new URL('https://test.mswjs.io/user/abc-123'), + 'https://test.mswjs.io/user/:userId?', + ), + ).toEqual({ + matches: true, + params: { + userId: 'abc-123', + }, }) - test('returns true when matching optional path parameters', () => { - const match = matchRequestUrl( + // Must match the URL if the optional parameter is missing. + expect( + matchRequestUrl( new URL('https://test.mswjs.io/user'), 'https://test.mswjs.io/user/:userId?', - ) - expect(match).toHaveProperty('matches', true) - expect(match).toHaveProperty('params', { + ), + ).toEqual({ + matches: true, + params: { + // The optional parameter key must still be present, + // with its value being undefined. userId: undefined, - }) + }, + }) +}) + +test('supports repeated groups', () => { + // Must match a single repeated group. + expect( + matchRequestUrl( + new URL('https://example.com/product/one'), + 'https://example.com/product/:action+', + ), + ).toEqual({ + matches: true, + params: { + action: 'one', + }, + }) + + // Must match on multiple repeated groups. + expect( + matchRequestUrl( + new URL('https://example.com/product/one/two/three'), + 'https://example.com/product/:action+', + ), + ).toEqual({ + matches: true, + params: { + /** + * @note The whole match is a single string. + * @see https://github.com/whatwg/urlpattern/issues/146 + */ + action: 'one/two/three', + }, + }) + + // Must match a repeated group within the path. + expect( + matchRequestUrl( + new URL('https://example.com/product/one/two/end'), + 'https://example.com/product/:action+/end', + ), + ).toEqual({ + matches: true, + params: { + action: 'one/two', + }, }) }) -describe('coercePath', () => { - test('escapes the colon in protocol', () => { - expect(coercePath('https://example.com')).toEqual('https\\://example.com') - expect(coercePath('https://example.com/:userId')).toEqual( - 'https\\://example.com/:userId', - ) - expect(coercePath('http://localhost:3000')).toEqual( - 'http\\://localhost\\:3000', - ) - }) - - test('escapes the colon before the port number', () => { - expect(coercePath('localhost:8080')).toEqual('localhost\\:8080') - expect(coercePath('http://127.0.0.1:8080')).toEqual( - 'http\\://127.0.0.1\\:8080', - ) - expect(coercePath('https://example.com:1234')).toEqual( - 'https\\://example.com\\:1234', - ) - - expect(coercePath('localhost:8080/:5678')).toEqual('localhost\\:8080/:5678') - expect(coercePath('https://example.com:8080/:5678')).toEqual( - 'https\\://example.com\\:8080/:5678', - ) - }) - - test('replaces wildcard with an unnnamed capturing group', () => { - expect(coercePath('*')).toEqual('(.*)') - expect(coercePath('**')).toEqual('(.*)') - expect(coercePath('/us*')).toEqual('/us(.*)') - expect(coercePath('/user/*')).toEqual('/user/(.*)') - expect(coercePath('https://example.com/user/*')).toEqual( - 'https\\://example.com/user/(.*)', - ) - expect(coercePath('https://example.com/us*')).toEqual( - 'https\\://example.com/us(.*)', - ) - }) - - test('preserves path parameter modifiers', () => { - expect(coercePath(':name*')).toEqual(':name*') - expect(coercePath('/foo/:name*')).toEqual('/foo/:name*') - expect(coercePath('/foo/**:name*')).toEqual('/foo/(.*):name*') - expect(coercePath('**/foo/*/:name*')).toEqual('(.*)/foo/(.*)/:name*') - expect(coercePath('/foo/:first/bar/:second*/*')).toEqual( - '/foo/:first/bar/:second*/(.*)', - ) +test('supports in-component matches', () => { + expect( + matchRequestUrl( + new URL('https://example.com/product/one.two'), + 'https://example.com/product/:first.:second', + ), + ).toEqual({ + matches: true, + params: { + first: 'one', + second: 'two', + }, }) }) diff --git a/src/core/utils/matching/matchRequestUrl.ts b/src/core/utils/matching/matchRequestUrl.ts index 3b9ce6ebf..83cf7c37d 100644 --- a/src/core/utils/matching/matchRequestUrl.ts +++ b/src/core/utils/matching/matchRequestUrl.ts @@ -1,10 +1,15 @@ -import { match } from 'path-to-regexp' +import { URLPattern } from 'urlpattern-polyfill' +import type { + URLPatternResult, + URLPatternComponentResult, + URLPatternInput, +} from 'urlpattern-polyfill/dist/types' import { getCleanUrl } from '@mswjs/interceptors' import { normalizePath } from './normalizePath' export type Path = string | RegExp export type PathParams = { - [ParamName in KeyType]: string | ReadonlyArray + [ParamName in KeyType]: string | ReadonlyArray | undefined } export interface Match { @@ -12,44 +17,56 @@ export interface Match { params?: PathParams } -/** - * Coerce a path supported by MSW into a path - * supported by "path-to-regexp". - */ -export function coercePath(path: string): string { - return ( - path - /** - * Replace wildcards ("*") with unnamed capturing groups - * because "path-to-regexp" doesn't support wildcards. - * Ignore path parameter' modifiers (i.e. ":name*"). - */ - .replace( - /([:a-zA-Z_-]*)(\*{1,2})+/g, - (_, parameterName: string | undefined, wildcard: string) => { - const expression = '(.*)' - - if (!parameterName) { - return expression - } - - return parameterName.startsWith(':') - ? `${parameterName}${wildcard}` - : `${parameterName}${expression}` - }, - ) - /** - * Escape the port so that "path-to-regexp" can match - * absolute URLs including port numbers. - */ - .replace(/([^\/])(:)(?=\d+)/, '$1\\$2') - /** - * Escape the protocol so that "path-to-regexp" could match - * absolute URL. - * @see https://github.com/pillarjs/path-to-regexp/issues/259 - */ - .replace(/^([^\/]+)(:)(?=\/\/)/, '$1\\$2') - ) +function parsePath( + path: string, +): [input: URLPatternInput | string, (result: URLPatternResult) => PathParams] { + if (path === '*') { + return [ + {}, + (result) => { + return { '0': result.inputs[0] as string } + }, + ] + } + + if (path.startsWith('*')) { + const pathname = path.slice(1) + + return [ + { + pathname, + }, + (result) => { + const url = new URL(result.inputs[0] as string) + const groups = groupsToArrayObject( + [result.pathname.groups, result.search.groups], + // Set the starting index for groups to 1 + // because the URL origin is matched at 0. + 1, + ) + + return { + '0': url.origin, + ...groups, + } + }, + ] + } + + return [ + path, + (result) => { + return groupsToArrayObject([ + result.protocol.groups, + result.username.groups, + result.password.groups, + result.hostname.groups, + result.port.groups, + result.pathname.groups, + result.search.groups, + ]) + }, + ] } /** @@ -57,17 +74,110 @@ export function coercePath(path: string): string { */ export function matchRequestUrl(url: URL, path: Path, baseUrl?: string): Match { const normalizedPath = normalizePath(path, baseUrl) - const cleanPath = - typeof normalizedPath === 'string' - ? coercePath(normalizedPath) - : normalizedPath - const cleanUrl = getCleanUrl(url) - const result = match(cleanPath, { decode: decodeURIComponent })(cleanUrl) - const params = (result && (result.params as PathParams)) || {} + + // Handle path predicates as regular expressions. + if (normalizedPath instanceof RegExp) { + const result = normalizedPath.exec(cleanUrl) + + return { + matches: result != null, + params: result ? regExpExecArrayToParams(result) : {}, + } + } + + const [input, urlPatternResultToParams] = parsePath(normalizedPath) + + // Handle pathp redicates as path strings (URL patterns). + /** + * @fixme URLPattern doesn't support encoding. + * We need to encode the path by hand. + */ + const pattern = new URLPattern(input, baseUrl) + const result = pattern.exec(cleanUrl) return { - matches: result !== false, - params, + matches: result != null, + params: result ? urlPatternResultToParams(result) : {}, } } + +/** + * Convert the result of running `RegExp.prototype.exec` + * to the parameters object expected by MSW. + */ +function regExpExecArrayToParams(result: RegExpExecArray): PathParams { + const params: PathParams = result.groups ? result.groups : {} + const unnamedGroups = result.slice(1) + + for (let index = 0; index < unnamedGroups.length - 1; index++) { + params[index] = unnamedGroups[index] + } + + return params +} + +function groupsToArrayObject( + groups: Array, + startIndex = 0, +): Record { + const result: Record = {} + let accumulativeStartIndex = startIndex + + for (const group of groups) { + let unnamedGroupsCount = 0 + + for (const key in group) { + const value = group[key] + + /** + * @fixme Undefined values may be okay to keep. + * Optional path parameters. + */ + // Skip group matches withouth values. + if (value == null || value == '') { + continue + } + + // Decode the match values. + const decodedValue = decodeURIComponent(value) + + /** + * Handle named group matches. + * @example "/user/:name" + * groups: { name: 'john' } + */ + if (isNaN(Number(key))) { + if (result[key]) { + result[key] = Array.prototype.concat([], result[key], decodedValue) + } else { + result[key] = decodedValue + } + + continue + } + + // Handle unnamed (numeric) group matches. + const index = accumulativeStartIndex + Number(key) + result[index] = decodedValue + unnamedGroupsCount++ + } + + /** + * Increment the accumulative index with the total + * count of matches in this group because each group + * in the match result segment starts from 0, while + * the path parameter indexes are consequential. + * + * @example + * { + * protocol: { groups: { '0': value } }, + * pathname: { groups: { '0': value } }, + * search: { groups: { '0': value } }, + * } + */ + accumulativeStartIndex += unnamedGroupsCount + } + + return result +} From f4f0129591e373930da417a0eb2470722f70e33b Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Thu, 18 Jul 2024 15:58:37 +0200 Subject: [PATCH 2/8] test(getResponse): fix relative url in node tests --- src/core/getResponse.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/getResponse.test.ts b/src/core/getResponse.test.ts index 8add7da8a..3cd7d4a79 100644 --- a/src/core/getResponse.test.ts +++ b/src/core/getResponse.test.ts @@ -13,7 +13,7 @@ it('returns undefined given empty headers array', async () => { it('returns undefined given no matching handlers', async () => { expect( await getResponse( - [http.get('/product', () => void 0)], + [http.get('*/product', () => void 0)], new Request('http://localhost/user'), ), ).toBeUndefined() From 49a050f6bc0f10d838f7503a6921fb6c90fe3f15 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Wed, 28 Aug 2024 14:44:16 +0200 Subject: [PATCH 3/8] chore: remove same-name params merge test --- .../utils/matching/matchRequestUrl.test.ts | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/src/core/utils/matching/matchRequestUrl.test.ts b/src/core/utils/matching/matchRequestUrl.test.ts index 8d31e325a..6a7353afa 100644 --- a/src/core/utils/matching/matchRequestUrl.test.ts +++ b/src/core/utils/matching/matchRequestUrl.test.ts @@ -128,43 +128,6 @@ test('supports a leading wildcard and a path with wildcards and a multiple named }) }) -test('merges multiple same-named groups into an array of values', () => { - // Must match same-named groups in different URL components. - expect( - matchRequestUrl( - new URL('https://example.com/user/abc/bar'), - 'https://:segment.com/user/:segment/bar', - ), - ).toEqual({ - matches: true, - params: { - segment: ['example', 'abc'], - }, - }) - - // Must match same-named groups in the same URL component. - expect( - matchRequestUrl( - new URL('https://example.com/user/abc/bar/def'), - 'https://example.com/user/:segment/bar/:segment', - ), - ).toEqual({ - matches: true, - params: { - /** - * @note URLPattern doesn't support multiple same-named groups - * within the same URL component, to begin with. - * @see https://github.com/whatwg/urlpattern/issues/226 - * - * However, "path-to-regexp" does! So does Express, and so does MSW. - * But it "supports" it in a weird way. It doesn't throw but takes - * the latest value of the group and preceding values. - */ - segment: 'def', - }, - }) -}) - test('decodes group matches', () => { const url = 'http://example.com:5001/example' expect( From d1405e90995af4c50b65c615784e4bbd182cc6b9 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Wed, 28 Aug 2024 15:03:45 +0200 Subject: [PATCH 4/8] fix(matchRequestUrl): treat relative urls as pathname --- src/core/utils/matching/matchRequestUrl.ts | 2 +- .../scenarios/relative-url.node.test.ts | 25 +++++++++++-------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/core/utils/matching/matchRequestUrl.ts b/src/core/utils/matching/matchRequestUrl.ts index 83cf7c37d..01b93eddb 100644 --- a/src/core/utils/matching/matchRequestUrl.ts +++ b/src/core/utils/matching/matchRequestUrl.ts @@ -29,7 +29,7 @@ function parsePath( ] } - if (path.startsWith('*')) { + if (path.startsWith('*') || path.startsWith('/')) { const pathname = path.slice(1) return [ diff --git a/test/node/msw-api/setup-server/scenarios/relative-url.node.test.ts b/test/node/msw-api/setup-server/scenarios/relative-url.node.test.ts index e68b7784e..a8b1fbd3b 100644 --- a/test/node/msw-api/setup-server/scenarios/relative-url.node.test.ts +++ b/test/node/msw-api/setup-server/scenarios/relative-url.node.test.ts @@ -1,10 +1,12 @@ -/** - * @vitest-environment node - */ +// @vitest-environment node import { HttpResponse, http } from 'msw' import { setupServer } from 'msw/node' const server = setupServer( + /** + * @note Although relative URLs do not exist in Node.js, + * MSW mustn't throw if you define one. It simply won't ever match. + */ http.get('/books', () => { return HttpResponse.json([1, 2, 3]) }), @@ -13,14 +15,15 @@ const server = setupServer( }), ) -beforeAll(() => server.listen()) - -afterAll(() => server.close()) +beforeAll(() => { + server.listen() +}) -test('tolerates relative request handlers on the server', async () => { - const res = await fetch('https://api.backend.com/path') - const body = await res.json() +afterAll(() => { + server.close() +}) - expect(res.status).toBe(200) - expect(body).toEqual({ success: true }) +test('does not throw on relative paths in request handlers in Node.js', async () => { + const response = await fetch('https://api.backend.com/path') + await expect(response.json()).resolves.toEqual({ success: true }) }) From 4356cdd6aa328ea3905facdcd08a4e8917775969 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Wed, 28 Aug 2024 15:48:03 +0200 Subject: [PATCH 5/8] fix: handle leading wildcard properly --- .../utils/matching/matchRequestUrl.test.ts | 18 ++++++++- src/core/utils/matching/matchRequestUrl.ts | 38 ++++++++++++------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/core/utils/matching/matchRequestUrl.test.ts b/src/core/utils/matching/matchRequestUrl.test.ts index 6a7353afa..ae2731b63 100644 --- a/src/core/utils/matching/matchRequestUrl.test.ts +++ b/src/core/utils/matching/matchRequestUrl.test.ts @@ -64,7 +64,23 @@ test('supports leading a wildcard as the entire pattern', () => { }) }) -test('supports leading a wildcard and path with a single unnamed group', () => { +test('supports a leading wildcard', async () => { + expect( + matchRequestUrl(new URL('http://localhost/cookies'), '*/cookies'), + ).toEqual({ + matches: true, + params: { 0: 'http://localhost' }, + }) + + expect( + matchRequestUrl(new URL('http://localhost/dashboard/cookies'), '*/cookies'), + ).toEqual({ + matches: true, + params: { 0: 'http://localhost/dashboard' }, + }) +}) + +test('supports a leading wildcard and path with a single unnamed group', () => { expect( matchRequestUrl(new URL('https://test.mswjs.io/user/123'), '*/user/*'), ).toEqual({ diff --git a/src/core/utils/matching/matchRequestUrl.ts b/src/core/utils/matching/matchRequestUrl.ts index 01b93eddb..c736c631f 100644 --- a/src/core/utils/matching/matchRequestUrl.ts +++ b/src/core/utils/matching/matchRequestUrl.ts @@ -22,6 +22,9 @@ function parsePath( ): [input: URLPatternInput | string, (result: URLPatternResult) => PathParams] { if (path === '*') { return [ + /** + * @note Empty input makes URLPattern match everything. + */ {}, (result) => { return { '0': result.inputs[0] as string } @@ -29,30 +32,39 @@ function parsePath( ] } - if (path.startsWith('*') || path.startsWith('/')) { - const pathname = path.slice(1) - + if (path.startsWith('*')) { return [ { - pathname, + pathname: path, }, (result) => { const url = new URL(result.inputs[0] as string) - const groups = groupsToArrayObject( - [result.pathname.groups, result.search.groups], - // Set the starting index for groups to 1 - // because the URL origin is matched at 0. - 1, + + // Grab the first pathname match because it belongs to the leading wildcard. + // Then, decrease each subsequent pathname group index by 1 to prevent overlap. + const firstPathname = result.pathname.groups[0] || '' + delete result.pathname.groups[0] + result.pathname.groups = Object.fromEntries( + Object.entries(result.pathname.groups).map(([key, value]) => [ + isNaN(Number(key)) ? key : String(Number(key) - 1), + value, + ]), ) - return { - '0': url.origin, - ...groups, - } + const groups = groupsToArrayObject([ + { 0: url.origin + firstPathname }, + result.pathname.groups, + ]) + + return groups }, ] } + if (path.startsWith('/') || path.startsWith('./')) { + throw new Error('TODO: Handle relative paths') + } + return [ path, (result) => { From 9e24077e3477aa57142f85f7aa655948f400708d Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Wed, 28 Aug 2024 15:54:57 +0200 Subject: [PATCH 6/8] fix: make "params" non-nullable --- src/core/utils/matching/matchRequestUrl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/utils/matching/matchRequestUrl.ts b/src/core/utils/matching/matchRequestUrl.ts index c736c631f..f66bcf371 100644 --- a/src/core/utils/matching/matchRequestUrl.ts +++ b/src/core/utils/matching/matchRequestUrl.ts @@ -14,7 +14,7 @@ export type PathParams = { export interface Match { matches: boolean - params?: PathParams + params: PathParams } function parsePath( From bbb66b55a88fcf98efa95355eade4feb032a8695 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Wed, 28 Aug 2024 15:55:19 +0200 Subject: [PATCH 7/8] fix: handle relative urls in node.js properly --- .../matching/matchRequestUrl.node.test.ts | 23 +++++++++++++++++ .../utils/matching/matchRequestUrl.test.ts | 25 +++++++++++++++++++ src/core/utils/matching/matchRequestUrl.ts | 13 +++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 src/core/utils/matching/matchRequestUrl.node.test.ts diff --git a/src/core/utils/matching/matchRequestUrl.node.test.ts b/src/core/utils/matching/matchRequestUrl.node.test.ts new file mode 100644 index 000000000..b3595c4c8 --- /dev/null +++ b/src/core/utils/matching/matchRequestUrl.node.test.ts @@ -0,0 +1,23 @@ +// @vite-environment node +import { Match, matchRequestUrl } from './matchRequestUrl' + +test('does not throw on relative URLs in Node.js', () => { + expect(matchRequestUrl(new URL('http://localhost'), '/foo')).toEqual({ + matches: false, + params: {}, + }) + + expect( + matchRequestUrl(new URL('http://localhost/foo'), '/foo'), + ).toEqual({ + matches: false, + params: {}, + }) + + expect( + matchRequestUrl(new URL('http://localhost/foo'), './foo'), + ).toEqual({ + matches: false, + params: {}, + }) +}) diff --git a/src/core/utils/matching/matchRequestUrl.test.ts b/src/core/utils/matching/matchRequestUrl.test.ts index ae2731b63..b5b02bdbb 100644 --- a/src/core/utils/matching/matchRequestUrl.test.ts +++ b/src/core/utils/matching/matchRequestUrl.test.ts @@ -48,6 +48,31 @@ test('supports exact URL', () => { }) }) +test('supports relative URLs', () => { + /** + * @note This only works because this test suite is using JSDOM, + * and MSW rebases relative URLs in JSDOM against the document's base URI. + */ + expect(matchRequestUrl(new URL('http://localhost'), '/foo')).toEqual({ + matches: false, + params: {}, + }) + + expect( + matchRequestUrl(new URL('http://localhost/foo'), '/foo'), + ).toEqual({ + matches: true, + params: {}, + }) + + expect( + matchRequestUrl(new URL('http://localhost/foo'), './foo'), + ).toEqual({ + matches: true, + params: {}, + }) +}) + test('supports leading a wildcard as the entire pattern', () => { expect(matchRequestUrl(new URL('https://test.mswjs.io'), '*')).toEqual( { diff --git a/src/core/utils/matching/matchRequestUrl.ts b/src/core/utils/matching/matchRequestUrl.ts index f66bcf371..613a7a2b5 100644 --- a/src/core/utils/matching/matchRequestUrl.ts +++ b/src/core/utils/matching/matchRequestUrl.ts @@ -61,8 +61,19 @@ function parsePath( ] } + /** + * @note The path will remain relative in Node.js. + * In browser and browser-like environments, it's always rebased + * against the document's base URI. This is mostly for Node.js. + */ if (path.startsWith('/') || path.startsWith('./')) { - throw new Error('TODO: Handle relative paths') + return [ + { + // Construct an extremely unlikely hash string to match. + hash: '#relative-urls-are-not-supported-in-node', + }, + () => ({}), + ] } return [ From bb1ad82197f13177cd86d8da8017c6ecd87812b2 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Wed, 28 Aug 2024 16:49:29 +0200 Subject: [PATCH 8/8] fix: respect trailing slash --- .../utils/matching/matchRequestUrl.test.ts | 53 ++++++++++++++----- src/core/utils/matching/matchRequestUrl.ts | 2 + .../rest-api/request/matching/uri.test.ts | 2 +- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/core/utils/matching/matchRequestUrl.test.ts b/src/core/utils/matching/matchRequestUrl.test.ts index b5b02bdbb..87408d70e 100644 --- a/src/core/utils/matching/matchRequestUrl.test.ts +++ b/src/core/utils/matching/matchRequestUrl.test.ts @@ -25,20 +25,6 @@ test('supports exact URL', () => { params: {}, }) - // Trailing slash doesn't matter. - expect( - matchRequestUrl(new URL('https://test.mswjs.io'), 'https://test.mswjs.io/'), - ).toEqual({ - matches: true, - params: {}, - }) - expect( - matchRequestUrl(new URL('https://test.mswjs.io/'), 'https://test.mswjs.io'), - ).toEqual({ - matches: true, - params: {}, - }) - // Must not match completely different URLs. expect( matchRequestUrl(new URL('https://example.com'), 'https://test.mswjs.io'), @@ -73,6 +59,45 @@ test('supports relative URLs', () => { }) }) +test('ignores trailing slashes for root-level URLs', () => { + expect( + matchRequestUrl(new URL('https://test.mswjs.io'), 'https://test.mswjs.io/'), + ).toEqual({ + matches: true, + params: {}, + }) +}) + +test('respects trailing slashes for pathnames', () => { + expect( + matchRequestUrl( + new URL('https://test.mswjs.io/foo'), + 'https://test.mswjs.io/foo/', + ), + ).toEqual({ + matches: false, + params: {}, + }) + expect( + matchRequestUrl( + new URL('https://api.github.com/made-up'), + 'https://api.github.com/made-up/', + ), + ).toEqual({ + matches: false, + params: {}, + }) + expect( + matchRequestUrl( + new URL('https://api.github.com/made-up/'), + 'https://api.github.com/made-up', + ), + ).toEqual({ + matches: false, + params: {}, + }) +}) + test('supports leading a wildcard as the entire pattern', () => { expect(matchRequestUrl(new URL('https://test.mswjs.io'), '*')).toEqual( { diff --git a/src/core/utils/matching/matchRequestUrl.ts b/src/core/utils/matching/matchRequestUrl.ts index 613a7a2b5..eb9c82ffa 100644 --- a/src/core/utils/matching/matchRequestUrl.ts +++ b/src/core/utils/matching/matchRequestUrl.ts @@ -111,6 +111,8 @@ export function matchRequestUrl(url: URL, path: Path, baseUrl?: string): Match { const [input, urlPatternResultToParams] = parsePath(normalizedPath) + console.log({ path, url, cleanUrl, input }) + // Handle pathp redicates as path strings (URL patterns). /** * @fixme URLPattern doesn't support encoding. diff --git a/test/browser/rest-api/request/matching/uri.test.ts b/test/browser/rest-api/request/matching/uri.test.ts index 3029fe2fb..975b83e97 100644 --- a/test/browser/rest-api/request/matching/uri.test.ts +++ b/test/browser/rest-api/request/matching/uri.test.ts @@ -6,7 +6,7 @@ test('matches an exact string with the same request URL with a trailing slash', }) => { await loadExample(require.resolve('./uri.mocks.ts')) - const res = await fetch('https://api.github.com/made-up/') + const res = await fetch('https://api.github.com/made-up') expect(res.status()).toEqual(200) expect(res.fromServiceWorker()).toBe(true)