Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion packages/next/src/lib/route-pattern-normalizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type { Token } from 'next/dist/compiled/path-to-regexp'
* This unique marker is inserted between adjacent parameters and stripped out
* during parameter extraction to avoid conflicts with real URL content.
*/
const PARAM_SEPARATOR = '_NEXTSEP_'
export const PARAM_SEPARATOR = '_NEXTSEP_'

/**
* Detects if a route pattern needs normalization for path-to-regexp compatibility.
Expand Down Expand Up @@ -97,6 +97,24 @@ export function normalizeTokensForRegexp(tokens: Token[]): Token[] {
})
}

/**
* Strips normalization separators from compiled pathname.
* This removes separators that were inserted by normalizeAdjacentParameters
* to satisfy path-to-regexp validation.
*
* Only removes separators in the specific contexts where they were inserted:
* - After interception route markers: (.)_NEXTSEP_ -> (.)
*
* This targeted approach ensures we don't accidentally remove the separator
* from legitimate user content.
*/
export function stripNormalizedSeparators(pathname: string): string {
// Remove separator after interception route markers
// Pattern: (.)_NEXTSEP_ -> (.), (..)_NEXTSEP_ -> (..), etc.
// The separator appears after the closing paren of interception markers
return pathname.replace(new RegExp(`\\)${PARAM_SEPARATOR}`, 'g'), ')')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stripNormalizedSeparators function will incorrectly modify user parameter values that contain the substring )_NEXTSEP_, not just the normalization markers it's intended to strip.

View Details
📝 Patch Details
diff --git a/packages/next/src/lib/route-pattern-normalizer.ts b/packages/next/src/lib/route-pattern-normalizer.ts
index 0460e3b0f7..52b6646ceb 100644
--- a/packages/next/src/lib/route-pattern-normalizer.ts
+++ b/packages/next/src/lib/route-pattern-normalizer.ts
@@ -109,10 +109,15 @@ export function normalizeTokensForRegexp(tokens: Token[]): Token[] {
  * from legitimate user content.
  */
 export function stripNormalizedSeparators(pathname: string): string {
-  // Remove separator after interception route markers
+  // Remove separator after interception route markers only
   // Pattern: (.)_NEXTSEP_ -> (.), (..)_NEXTSEP_ -> (..), etc.
-  // The separator appears after the closing paren of interception markers
-  return pathname.replace(new RegExp(`\\)${PARAM_SEPARATOR}`, 'g'), ')')
+  // Match the full interception marker pattern: opening paren, one or more dots, closing paren, then separator
+  // This ensures we only strip separators that were inserted by normalizeAdjacentParameters
+  // for interception routes, not user content that happens to contain )_NEXTSEP_
+  return pathname.replace(
+    new RegExp(`(\\(\\.+\\))${PARAM_SEPARATOR}`, 'g'),
+    '$1'
+  )
 }
 
 /**
diff --git a/packages/next/src/shared/lib/router/utils/route-match-utils.test.ts b/packages/next/src/shared/lib/router/utils/route-match-utils.test.ts
index daa1fc7a3e..0199bc3d89 100644
--- a/packages/next/src/shared/lib/router/utils/route-match-utils.test.ts
+++ b/packages/next/src/shared/lib/router/utils/route-match-utils.test.ts
@@ -132,6 +132,23 @@ describe('safeCompile', () => {
       // The _NEXTSEP_ in user content should be preserved
       expect(result).toBe('/my_NEXTSEP_folder/my_NEXTSEP_file.txt')
     })
+
+    it('should not strip )_NEXTSEP_ from user parameter values in interception routes', () => {
+      // Regression test: User content containing )_NEXTSEP_ should not be modified
+      // even when used with interception routes that have the separator
+      const pattern = '/photos/(.):username/:id'
+      const compile = safeCompile(pattern, { validate: false })
+
+      // User's username parameter contains )_NEXTSEP_ (edge case but possible)
+      const result = compile({
+        '0': '(.)',
+        username: 'user)_NEXTSEP_name',
+        id: '123',
+      })
+
+      // The )_NEXTSEP_ in user content should be preserved, not stripped
+      expect(result).toBe('/photos/(.)user)_NEXTSEP_name/123')
+    })
   })
 })
 
@@ -185,4 +202,13 @@ describe('stripNormalizedSeparators', () => {
     // Should only strip the one after ), not the one before
     expect(result).toBe(`/path${PARAM_SEPARATOR}(.)value`)
   })
+
+  it('should not strip )_NEXTSEP_ when not part of interception marker', () => {
+    // Regression test: Only strip separators after interception markers like (.)
+    // User content with )_NEXTSEP_ should be preserved
+    const input = `/photos/(.)${PARAM_SEPARATOR}user)${PARAM_SEPARATOR}name/123`
+    const result = stripNormalizedSeparators(input)
+    // Should strip (.)_NEXTSEP_ but preserve user)_NEXTSEP_name
+    expect(result).toBe(`/photos/(.)user)${PARAM_SEPARATOR}name/123`)
+  })
 })

Analysis

Silent data corruption in stripNormalizedSeparators strips user content containing )_NEXTSEP_

What fails: The stripNormalizedSeparators function in packages/next/src/lib/route-pattern-normalizer.ts line 115 strips ANY occurrence of )_NEXTSEP_ from compiled pathnames, not just normalization markers after interception routes

How to reproduce:

// Pattern with interception marker
const pattern = '/photos/(.):author/:id'
const compile = safeCompile(pattern, { validate: false })

// User's author parameter contains )_NEXTSEP_
const result = compile({ 
  '0': '(.)', 
  author: 'user)_NEXTSEP_name',  // User content
  id: '123' 
})

console.log(result)
// Expected: '/photos/(.)user)_NEXTSEP_name/123'
// Actual:   '/photos/(.)user)name/123'  
// The _NEXTSEP_ is incorrectly stripped from user content

Result: User parameter values containing the substring )_NEXTSEP_ are silently corrupted. The 10-character sequence is removed without error, breaking URLs, usernames, file paths, etc.

Expected: The function's documentation (lines 108-109) claims it uses a "targeted approach" that "ensures we don't accidentally remove the separator from legitimate user content," but the regex /\)_NEXTSEP_/g matches ANY )_NEXTSEP_ regardless of context.

Fix: Changed regex from /\)_NEXTSEP_/g to /(\(\.+\))_NEXTSEP_/g to match only interception marker patterns: (.), (..), (...) followed by the separator, not arbitrary user content.

}

/**
* Strips normalization separators from extracted route parameters.
* Used by both server and client code to clean up parameters after route matching.
Expand Down
188 changes: 188 additions & 0 deletions packages/next/src/shared/lib/router/utils/route-match-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
import { safeCompile } from './route-match-utils'
import {
PARAM_SEPARATOR,
stripNormalizedSeparators,
} from '../../../../lib/route-pattern-normalizer'

describe('safeCompile', () => {
describe('interception route patterns', () => {
it('should strip _NEXTSEP_ from compiled output for (.) interception marker', () => {
// Pattern with interception marker followed by parameter
const pattern = '/photos/(.):author/:id'
const compile = safeCompile(pattern, { validate: false })

// The interception marker (.) is treated as an unnamed parameter (index 0)
const result = compile({ '0': '(.)', author: 'next', id: '123' })

// Should NOT contain the internal separator
expect(result).toBe('/photos/(.)next/123')
})

it('should strip _NEXTSEP_ from compiled output for (..) interception marker', () => {
const pattern = '/photos/(..):category/:id'
const compile = safeCompile(pattern, { validate: false })

const result = compile({ '0': '(..)', category: 'blog', id: '456' })

expect(result).toBe('/photos/(..)blog/456')
})

it('should strip _NEXTSEP_ from compiled output for (...) interception marker', () => {
const pattern = '/photos/(...):path'
const compile = safeCompile(pattern, { validate: false })

const result = compile({ '0': '(...)', path: 'deep/nested/route' })

expect(result).toBe('/photos/(...)deep/nested/route')
})

it('should strip _NEXTSEP_ from compiled output for (..)(..) interception marker', () => {
const pattern = '/photos/(.)(..)/:id'
const compile = safeCompile(pattern, { validate: false })

// (..)(..) is treated as two unnamed parameters
const result = compile({ '0': '(..)', '1': '(..)', id: '789' })

expect(result).toBe('/photos/(..)(..)/789')
})

it('should handle multiple interception markers in one pattern', () => {
const pattern = '/(.):author/photos/(.):id'
const compile = safeCompile(pattern, { validate: false })

// Multiple markers are numbered sequentially
const result = compile({
'0': '(.)',
author: 'john',
'1': '(.)',
id: '999',
})

expect(result).toBe('/(.)john/photos/(.)999')
})

it('should work with the actual failing case from interception routes', () => {
// This is the exact pattern that was failing
const pattern =
'/intercepting-routes-dynamic/photos/(.):nxtPauthor/:nxtPid'
const compile = safeCompile(pattern, { validate: false })

const result = compile({
'0': '(.)',
nxtPauthor: 'next',
nxtPid: '123',
})

expect(result).toBe('/intercepting-routes-dynamic/photos/(.)next/123')
})
})

describe('patterns without normalization needs', () => {
it('should work normally for patterns without adjacent parameters', () => {
const pattern = '/photos/:author/:id'
const compile = safeCompile(pattern, { validate: false })

const result = compile({ author: 'jane', id: '456' })

expect(result).toBe('/photos/jane/456')
})

it('should work with optional parameters', () => {
const pattern = '/photos/:author?/:id'
const compile = safeCompile(pattern, { validate: false })

const result = compile({ id: '789' })

expect(result).toBe('/photos/789')
})

it('should work with catchall parameters', () => {
const pattern = '/files/:path*'
const compile = safeCompile(pattern, { validate: false })

const result = compile({ path: ['folder', 'subfolder', 'file.txt'] })

expect(result).toBe('/files/folder/subfolder/file.txt')
})
})

describe('edge cases', () => {
it('should handle patterns with path separators between parameters', () => {
// Normal case - parameters separated by path segments
const pattern = '/:param1/separator/:param2'
const compile = safeCompile(pattern, { validate: false })

const result = compile({ param1: 'value1', param2: 'value2' })

expect(result).toBe('/value1/separator/value2')
})

it('should not strip _NEXTSEP_ from user content outside interception markers', () => {
// If user content happens to contain _NEXTSEP_, it should be preserved
// Only separators after interception markers should be stripped
const pattern = '/:folder/:file'
const compile = safeCompile(pattern, { validate: false })

// User has a file or folder named something_NEXTSEP_something
const result = compile({
folder: 'my_NEXTSEP_folder',
file: 'my_NEXTSEP_file.txt',
})

// The _NEXTSEP_ in user content should be preserved
expect(result).toBe('/my_NEXTSEP_folder/my_NEXTSEP_file.txt')
})
})
})

describe('stripNormalizedSeparators', () => {
it('should strip _NEXTSEP_ after single dot interception marker', () => {
const input = `/photos/(.)${PARAM_SEPARATOR}next/123`
const result = stripNormalizedSeparators(input)
expect(result).toBe('/photos/(.)next/123')
})

it('should strip _NEXTSEP_ after double dot interception marker', () => {
const input = `/photos/(..)${PARAM_SEPARATOR}blog/456`
const result = stripNormalizedSeparators(input)
expect(result).toBe('/photos/(..)blog/456')
})

it('should strip _NEXTSEP_ after triple dot interception marker', () => {
const input = `/photos/(...)${PARAM_SEPARATOR}deep/nested/route`
const result = stripNormalizedSeparators(input)
expect(result).toBe('/photos/(...)deep/nested/route')
})

it('should strip _NEXTSEP_ for adjacent interception markers with parameters', () => {
// When there are two separate interception paths, each with parameters
// Pattern: /(.)_NEXTSEP_:param1/(..)_NEXTSEP_:param2
// After compilation: /(.)_NEXTSEP_value1/(..)_NEXTSEP_value2
const input = `/(.)${PARAM_SEPARATOR}first/(..)${PARAM_SEPARATOR}second`
const result = stripNormalizedSeparators(input)
expect(result).toBe('/(.)first/(..)second')
})

it('should handle multiple interception markers in one path', () => {
const input = `/(.)${PARAM_SEPARATOR}john/photos/(.)${PARAM_SEPARATOR}999`
const result = stripNormalizedSeparators(input)
expect(result).toBe('/(.)john/photos/(.)999')
})

it('should NOT strip _NEXTSEP_ from user content', () => {
// If the separator appears outside the interception marker context,
// it should be preserved as it's part of user content
const input = `/folder/my${PARAM_SEPARATOR}file/data${PARAM_SEPARATOR}value`
const result = stripNormalizedSeparators(input)
expect(result).toBe(
`/folder/my${PARAM_SEPARATOR}file/data${PARAM_SEPARATOR}value`
)
})

it('should only strip after closing paren, not before', () => {
const input = `/path${PARAM_SEPARATOR}(.)${PARAM_SEPARATOR}value`
const result = stripNormalizedSeparators(input)
// Should only strip the one after ), not the one before
expect(result).toBe(`/path${PARAM_SEPARATOR}(.)value`)
})
})
23 changes: 21 additions & 2 deletions packages/next/src/shared/lib/router/utils/route-match-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
hasAdjacentParameterIssues,
normalizeAdjacentParameters,
stripParameterSeparators,
stripNormalizedSeparators,
} from '../../../../lib/route-pattern-normalizer'

/**
Expand Down Expand Up @@ -59,6 +60,8 @@ export function safePathToRegexp(
/**
* Client-safe wrapper around compile that handles path-to-regexp 6.3.0+ validation errors.
* No server-side error reporting to avoid bundling issues.
* When normalization is applied, the returned compiler function automatically strips
* the internal separator from the output URL.
*/
export function safeCompile(
route: string,
Expand All @@ -71,13 +74,29 @@ export function safeCompile(
: route

try {
return compile(routeToUse, options)
const compiler = compile(routeToUse, options)

// If we normalized the route, wrap the compiler to strip separators from output
// The normalization inserts _NEXTSEP_ as a literal string in the pattern to satisfy
// path-to-regexp validation, but we don't want it in the final compiled URL
if (needsNormalization) {
return (params: any) => {
return stripNormalizedSeparators(compiler(params))
}
}

return compiler
} catch (error) {
// Only try normalization if we haven't already normalized
if (!needsNormalization) {
try {
const normalizedRoute = normalizeAdjacentParameters(route)
return compile(normalizedRoute, options)
const compiler = compile(normalizedRoute, options)

// Wrap the compiler to strip separators from output
return (params: any) => {
return stripNormalizedSeparators(compiler(params))
}
} catch (retryError) {
// If that doesn't work, fall back to original error
throw error
Expand Down
Loading