From 63c2e27587249639cf3b570650ef5d890232daf8 Mon Sep 17 00:00:00 2001 From: HarrySeop Date: Sat, 17 Jan 2026 00:48:10 +0900 Subject: [PATCH 1/2] fix(router-core): prevent search params mutation in middlewares --- packages/router-core/src/searchMiddleware.ts | 11 +- .../tests/searchMiddleware.test.ts | 135 ++++++++++++++++++ 2 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 packages/router-core/tests/searchMiddleware.test.ts diff --git a/packages/router-core/src/searchMiddleware.ts b/packages/router-core/src/searchMiddleware.ts index e90d453350f..1d60cc9587c 100644 --- a/packages/router-core/src/searchMiddleware.ts +++ b/packages/router-core/src/searchMiddleware.ts @@ -21,13 +21,14 @@ export function retainSearchParams( if (keys === true) { return { ...search, ...result } } - // add missing keys from search to result + const copy = { ...result } + // add missing keys from search to copy keys.forEach((key) => { - if (!(key in result)) { - result[key] = search[key] + if (!(key in copy)) { + copy[key] = search[key] } }) - return result + return copy } } @@ -55,7 +56,7 @@ export function stripSearchParams< if (input === true) { return {} } - const result = next(search) as Record + const result = { ...next(search) } as Record if (Array.isArray(input)) { input.forEach((key) => { delete result[key] diff --git a/packages/router-core/tests/searchMiddleware.test.ts b/packages/router-core/tests/searchMiddleware.test.ts new file mode 100644 index 00000000000..1013443150c --- /dev/null +++ b/packages/router-core/tests/searchMiddleware.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, test } from 'vitest' +import { retainSearchParams, stripSearchParams } from '../src/searchMiddleware' + +describe('searchMiddleware - mutation prevention', () => { + describe('retainSearchParams', () => { + test('should not mutate original search object', () => { + const originalSearch = { id: '1', filter: 'active', page: '2' } + const originalCopy = { ...originalSearch } + + const middleware = retainSearchParams(['id', 'filter']) + + const result = middleware({ + search: originalSearch, + next: (search) => ({ id: search.id, filter: '', page: '' }), + }) + + expect(originalSearch).toEqual(originalCopy) + expect(originalSearch).toEqual({ id: '1', filter: 'active', page: '2' }) + expect(result).toEqual({ id: '1', filter: 'active', page: '' }) + expect(result).not.toBe(originalSearch) + }) + + test('should work correctly when same reference is reused', () => { + const sharedSearch = { id: '1', filter: 'active', page: '1' } + const middleware = retainSearchParams(['id']) + + const result1 = middleware({ + search: sharedSearch, + next: () => ({ id: '', filter: '', page: '' }), + }) + + expect(sharedSearch).toEqual({ id: '1', filter: 'active', page: '1' }) + expect(result1).toEqual({ id: '1', filter: '', page: '' }) + + const result2 = middleware({ + search: sharedSearch, + next: () => ({ id: '', filter: '', page: '' }), + }) + + expect(sharedSearch).toEqual({ id: '1', filter: 'active', page: '1' }) + expect(result2).toEqual({ id: '1', filter: '', page: '' }) + }) + + test('should handle retainSearchParams(true) correctly', () => { + const originalSearch = { id: '1', filter: 'active' } + const originalCopy = { ...originalSearch } + + const middleware = retainSearchParams(true) + + const result = middleware({ + search: originalSearch, + next: () => ({ id: '2', filter: '' }), + }) + + expect(originalSearch).toEqual(originalCopy) + expect(result).toEqual({ id: '2', filter: 'active' }) + }) + }) + + describe('stripSearchParams', () => { + test('should not mutate original search object (array input)', () => { + const originalSearch = { id: '1', filter: 'active', page: '1' } + const originalCopy = { ...originalSearch } + + const middleware = stripSearchParams(['filter', 'page']) + + const result = middleware({ + search: originalSearch, + next: (search) => search, + }) + + expect(originalSearch).toEqual(originalCopy) + expect(originalSearch).toEqual({ id: '1', filter: 'active', page: '1' }) + expect(result).toEqual({ id: '1' }) + expect(result).not.toBe(originalSearch) + }) + + test('should not mutate original search object (object input)', () => { + const originalSearch = { id: '1', filter: 'active', status: 'done' } + const originalCopy = { ...originalSearch } + + const middleware = stripSearchParams({ filter: 'active', status: 'done' }) + + const result = middleware({ + search: originalSearch, + next: (search) => search, + }) + + expect(originalSearch).toEqual(originalCopy) + expect(originalSearch).toEqual({ + id: '1', + filter: 'active', + status: 'done', + }) + expect(result).toEqual({ id: '1' }) + expect(result).not.toBe(originalSearch) + }) + + test('should work correctly when same reference is reused', () => { + const sharedSearch = { id: '1', filter: 'active', page: '1' } + const middleware = stripSearchParams(['filter', 'page']) + + const result1 = middleware({ + search: sharedSearch, + next: (search) => search, + }) + + expect(sharedSearch).toEqual({ id: '1', filter: 'active', page: '1' }) + expect(result1).toEqual({ id: '1' }) + + const result2 = middleware({ + search: sharedSearch, + next: (search) => search, + }) + + expect(sharedSearch).toEqual({ id: '1', filter: 'active', page: '1' }) + expect(result2).toEqual({ id: '1' }) + }) + + test('should handle stripSearchParams(true) correctly', () => { + const originalSearch = { id: '1', filter: 'active' } + const originalCopy = { ...originalSearch } + + const middleware = stripSearchParams(true) + + const result = middleware({ + search: originalSearch, + next: (search) => search, + }) + + expect(originalSearch).toEqual(originalCopy) + expect(result).toEqual({}) + }) + }) +}) From 708eaeb1cbc8f590f54953a60001215515234efb Mon Sep 17 00:00:00 2001 From: HarrySeop Date: Sat, 17 Jan 2026 13:59:45 +0900 Subject: [PATCH 2/2] test(router-core): fix retainSearchParams test cases to match actual behavior --- .../tests/searchMiddleware.test.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/router-core/tests/searchMiddleware.test.ts b/packages/router-core/tests/searchMiddleware.test.ts index 1013443150c..10c131bfd51 100644 --- a/packages/router-core/tests/searchMiddleware.test.ts +++ b/packages/router-core/tests/searchMiddleware.test.ts @@ -9,36 +9,39 @@ describe('searchMiddleware - mutation prevention', () => { const middleware = retainSearchParams(['id', 'filter']) + // next() returns object without 'id' and 'filter' keys + // so retainSearchParams should add them from search const result = middleware({ search: originalSearch, - next: (search) => ({ id: search.id, filter: '', page: '' }), + next: () => ({ page: 'new' }) as any, }) expect(originalSearch).toEqual(originalCopy) expect(originalSearch).toEqual({ id: '1', filter: 'active', page: '2' }) - expect(result).toEqual({ id: '1', filter: 'active', page: '' }) + expect(result).toEqual({ id: '1', filter: 'active', page: 'new' }) expect(result).not.toBe(originalSearch) }) test('should work correctly when same reference is reused', () => { const sharedSearch = { id: '1', filter: 'active', page: '1' } - const middleware = retainSearchParams(['id']) + const middleware = retainSearchParams(['id', 'filter']) + // next() returns object without 'id' and 'filter' keys const result1 = middleware({ search: sharedSearch, - next: () => ({ id: '', filter: '', page: '' }), + next: () => ({ page: '2' }) as any, }) expect(sharedSearch).toEqual({ id: '1', filter: 'active', page: '1' }) - expect(result1).toEqual({ id: '1', filter: '', page: '' }) + expect(result1).toEqual({ id: '1', filter: 'active', page: '2' }) const result2 = middleware({ search: sharedSearch, - next: () => ({ id: '', filter: '', page: '' }), + next: () => ({ page: '3' }) as any, }) expect(sharedSearch).toEqual({ id: '1', filter: 'active', page: '1' }) - expect(result2).toEqual({ id: '1', filter: '', page: '' }) + expect(result2).toEqual({ id: '1', filter: 'active', page: '3' }) }) test('should handle retainSearchParams(true) correctly', () => { @@ -49,7 +52,7 @@ describe('searchMiddleware - mutation prevention', () => { const result = middleware({ search: originalSearch, - next: () => ({ id: '2', filter: '' }), + next: () => ({ id: '2' }) as any, }) expect(originalSearch).toEqual(originalCopy)