diff --git a/.changeset/pin-path-matcher-contracts.md b/.changeset/pin-path-matcher-contracts.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/pin-path-matcher-contracts.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/shared/src/__tests__/pathMatcher.spec.ts b/packages/shared/src/__tests__/pathMatcher.spec.ts index 839c285048b..1b9aae27634 100644 --- a/packages/shared/src/__tests__/pathMatcher.spec.ts +++ b/packages/shared/src/__tests__/pathMatcher.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test, vi } from 'vitest'; -import { createPathMatcher, MalformedURLError, normalizePath } from '../pathMatcher'; +import { createPathMatcher, isMalformedURLError, MalformedURLError, normalizePath } from '../pathMatcher'; vi.mock('../pathToRegexp', () => ({ pathToRegexp: (pattern: string) => new RegExp(`^${pattern.replace('(.*)', '.*')}$`), @@ -100,6 +100,47 @@ describe('createPathMatcher', () => { expect(() => matcher('/api/%zz/users')).toThrow(MalformedURLError); expect(() => matcher('/%')).toThrow(MalformedURLError); }); + + test('does not resolve dot-segments — `..` is treated as literal text', () => { + // Pinning current behavior: createPathMatcher does not perform RFC 3986 + // §5.2.4 dot-segment removal. Callers are responsible for passing a + // pathname that has already had `..` resolved (frameworks built on the + // WHATWG URL parser do this automatically). If anyone later teaches + // normalizePath to resolve `..`, that's a behavior change that should + // be deliberate and update this test. + const matcher = createPathMatcher('/api/admin(.*)'); + expect(matcher('/public/%2E%2E/api/admin')).toBe(false); + expect(matcher('/public/../api/admin')).toBe(false); + }); + + test('decodes exactly once — does not collapse double-percent encoding', () => { + // Pinning current behavior: normalizePath calls decodeURI a single + // time. `%2561dmin` decodes to `%61dmin` (literal `%` + `61dmin`), + // not `admin`. A two-pass decode would change matching semantics for + // any pattern containing literal `%` and is intentionally not done. + const matcher = createPathMatcher('/api/admin(.*)'); + expect(matcher('/api/%2561dmin/users')).toBe(false); + expect(normalizePath('/api/%2561dmin')).toBe('/api/%61dmin'); + }); + + test('decodes UTF-8 multi-byte sequences', () => { + // Decoded codepoint must round-trip cleanly through the matcher. + const matcher = createPathMatcher('/api/admin(.*)'); + expect(matcher('/api/admin/%E6%97%A5%E6%9C%AC')).toBe(true); // 日本 + expect(matcher('/api/admin/%F0%9F%92%A9')).toBe(true); // 💩 (surrogate pair) + expect(normalizePath('/api/%E6%97%A5')).toBe('/api/日'); + }); + + test('decodes backslash to a literal backslash, not a slash', () => { + // %5C is not in decodeURI's reservedURISet and not a path delimiter, + // so it decodes to `\` and stays as one character. Some servers + // (notably IIS) historically aliased `\` to `/`; that aliasing is the + // upstream router's job, not the matcher's, and the WHATWG URL parser + // handles it before pathname is ever seen here. + expect(normalizePath('/api/admin%5Cfoo')).toBe('/api/admin\\foo'); + const matcher = createPathMatcher('/api/admin(.*)'); + expect(matcher('/api/admin%5Cfoo')).toBe(true); + }); }); describe('double-slash normalization', () => { @@ -182,3 +223,42 @@ describe('normalizePath', () => { }); }); }); + +describe('MalformedURLError', () => { + // Public contract: callers like clerkMiddleware fail closed on this exception + // class. The shape (name, statusCode, instanceof Error) and the cross-bundle + // detection helper are part of that contract — pin them so they can't drift + // silently across releases. + + test('has the documented public shape', () => { + const err = new MalformedURLError('/foo'); + expect(err).toBeInstanceOf(Error); + expect(err.name).toBe('MalformedURLError'); + expect(err.statusCode).toBe(400); + expect(err.message).toContain('/foo'); + }); + + test('preserves the cause when one is provided', () => { + const cause = new URIError('boom'); + const err = new MalformedURLError('/foo', cause); + expect(err.cause).toBe(cause); + }); + + test('isMalformedURLError detects instances by name (not by class identity)', () => { + // The string-based check exists so callers in other bundles can detect + // MalformedURLError thrown by a different copy of @clerk/shared. Pin + // both halves: the positive case and the negative cases. + expect(isMalformedURLError(new MalformedURLError('/x'))).toBe(true); + + const lookalike = new Error('not us'); + lookalike.name = 'MalformedURLError'; + expect(isMalformedURLError(lookalike)).toBe(true); + + expect(isMalformedURLError(new Error('plain'))).toBe(false); + expect(isMalformedURLError(new URIError('uri'))).toBe(false); + expect(isMalformedURLError(undefined)).toBe(false); + expect(isMalformedURLError(null)).toBe(false); + expect(isMalformedURLError('MalformedURLError')).toBe(false); + expect(isMalformedURLError({ name: 'MalformedURLError' })).toBe(false); + }); +});