-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(core): Add filterUrlQuery util
#23060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import type { CollectBehavior } from '../../types/datacollection'; | ||
| import { filterQueryParams } from './filterQueryParams'; | ||
|
|
||
| /** | ||
| * Applies a `CollectBehavior` to the query string of a full URL, leaving every other URL component | ||
| * (scheme, host, path, fragment) untouched. | ||
| * | ||
| * The query is located by string offsets rather than by parsing, so the URL is returned byte-for-byte | ||
| * apart from the query itself. This keeps relative URLs, non-HTTP schemes and unusual encodings intact, | ||
| * none of which survive a `URL` round-trip. | ||
| * | ||
| * Returns the URL with its query filtered, or with the query removed entirely when collection is off. | ||
| */ | ||
| export function filterUrlQuery(url: string, behavior: CollectBehavior): string { | ||
| // The fragment is delimited first: a `?` after a `#` belongs to the fragment, not the query. | ||
| const fragmentStart = url.indexOf('#'); | ||
| const queryEnd = fragmentStart === -1 ? url.length : fragmentStart; | ||
|
|
||
| const queryStart = url.indexOf('?'); | ||
| if (queryStart === -1 || queryStart > queryEnd) { | ||
| return url; | ||
| } | ||
|
|
||
| const prefix = url.slice(0, queryStart); | ||
| const query = url.slice(queryStart + 1, queryEnd); | ||
| const suffix = url.slice(queryEnd); | ||
|
|
||
| const filtered = filterQueryParams(query, behavior); | ||
|
|
||
| return filtered ? `${prefix}?${filtered}${suffix}` : `${prefix}${suffix}`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Suggested FixIn Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { filterUrlQuery } from '../../../../src/utils/data-collection/filterUrlQuery'; | ||
|
|
||
| describe('filterUrlQuery', () => { | ||
| describe('no query string', () => { | ||
| it('returns the URL unchanged', () => { | ||
| expect(filterUrlQuery('https://example.com/api/users', true)).toBe('https://example.com/api/users'); | ||
| }); | ||
|
|
||
| it('returns a URL with only a fragment unchanged', () => { | ||
| expect(filterUrlQuery('https://example.com/docs#section', true)).toBe('https://example.com/docs#section'); | ||
| }); | ||
|
|
||
| it('leaves a trailing `?` with no params alone', () => { | ||
| expect(filterUrlQuery('https://example.com/api?', true)).toBe('https://example.com/api'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('denyList mode (true)', () => { | ||
| it('filters sensitive params and preserves the rest', () => { | ||
| const result = filterUrlQuery('https://example.com/api/users?token=abc123&q=a%20b%26c&page=5', true); | ||
|
|
||
| expect(result).toBe('https://example.com/api/users?token=[Filtered]&q=a%20b%26c&page=5'); | ||
| }); | ||
|
|
||
| it('preserves the fragment', () => { | ||
| const result = filterUrlQuery('https://example.com/api?token=abc&page=5#results', true); | ||
|
|
||
| expect(result).toBe('https://example.com/api?token=[Filtered]&page=5#results'); | ||
| }); | ||
|
|
||
| it('preserves userinfo, port and path', () => { | ||
| const result = filterUrlQuery('https://user:pw@example.com:8443/a/b?secret=x&ok=1', true); | ||
|
|
||
| expect(result).toBe('https://user:pw@example.com:8443/a/b?secret=[Filtered]&ok=1'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('off mode (false)', () => { | ||
| it('removes the query entirely', () => { | ||
| expect(filterUrlQuery('https://example.com/api/users?token=abc&page=5', false)).toBe( | ||
| 'https://example.com/api/users', | ||
| ); | ||
| }); | ||
|
|
||
| it('removes the query but keeps the fragment', () => { | ||
| expect(filterUrlQuery('https://example.com/api?token=abc#results', false)).toBe( | ||
| 'https://example.com/api#results', | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('allow / deny behaviors', () => { | ||
| it('supports allowList mode', () => { | ||
| const result = filterUrlQuery('https://example.com/s?page=1&ref=x&sort=name', { allow: ['page', 'sort'] }); | ||
|
|
||
| expect(result).toBe('https://example.com/s?page=1&ref=[Filtered]&sort=name'); | ||
| }); | ||
|
|
||
| it('supports extra deny terms', () => { | ||
| const result = filterUrlQuery('https://example.com/s?page=1&utm_source=email', { deny: ['utm'] }); | ||
|
|
||
| expect(result).toBe('https://example.com/s?page=1&utm_source=[Filtered]'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('non-standard URLs', () => { | ||
| it('handles relative URLs', () => { | ||
| expect(filterUrlQuery('/api/users?token=abc&page=5', true)).toBe('/api/users?token=[Filtered]&page=5'); | ||
| }); | ||
|
|
||
| it('preserves duplicate params and their order', () => { | ||
| const result = filterUrlQuery('https://example.com/s?page=1&token=a&page=2', true); | ||
|
|
||
| expect(result).toBe('https://example.com/s?page=1&token=[Filtered]&page=2'); | ||
| }); | ||
|
|
||
| it('does not treat a `?` inside a fragment as a query', () => { | ||
| const result = filterUrlQuery('https://example.com/docs#/route?token=abc', true); | ||
|
|
||
| expect(result).toBe('https://example.com/docs#/route?token=abc'); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe just call this fragment?