Skip to content

Commit 31cc183

Browse files
authored
fix(nextjs): Remove polynomial regular expression (#18725)
Check out the regex here: https://regex101.com/r/Tw2fuQ/1 Closes https://github.com/getsentry/sentry-javascript/security/code-scanning/419 Closes #18726 (added automatically)
1 parent ce62e84 commit 31cc183

File tree

5 files changed

+35
-3
lines changed

5 files changed

+35
-3
lines changed

packages/nextjs/src/config/manifest/createRouteManifest.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ function isRouteGroup(name: string): boolean {
2828
return name.startsWith('(') && name.endsWith(')');
2929
}
3030

31-
function normalizeRoutePath(routePath: string): string {
31+
function normalizeRouteGroupPath(routePath: string): string {
3232
// Remove route group segments from the path
33-
return routePath.replace(/\/\([^)]+\)/g, '');
33+
// Using positive lookahead with (?=[^)\/]*\)) to avoid polynomial matching
34+
return routePath.replace(/\/\((?=[^)/]*\))[^)/]+\)/g, '');
3435
}
3536

3637
function getDynamicRouteSegment(name: string): string {
@@ -140,7 +141,7 @@ function scanAppDirectory(dir: string, basePath: string = '', includeRouteGroups
140141

141142
if (pageFile) {
142143
// Conditionally normalize the path based on includeRouteGroups option
143-
const routePath = includeRouteGroups ? basePath || '/' : normalizeRoutePath(basePath || '/');
144+
const routePath = includeRouteGroups ? basePath || '/' : normalizeRouteGroupPath(basePath || '/');
144145
const isDynamic = routePath.includes(':');
145146

146147
// Check if this page has generateStaticParams (ISR/SSG indicator)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// API Internal Page
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// Login V2 Page
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// Features Beta Page

packages/nextjs/test/config/manifest/suites/route-groups/route-groups.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@ describe('route-groups', () => {
1212
expect(manifest).toEqual({
1313
staticRoutes: [
1414
{ path: '/' },
15+
{ path: '/api' },
1516
{ path: '/login' },
1617
{ path: '/signup' },
18+
{ path: '/login' }, // from (auth-v2)
1719
{ path: '/dashboard' },
1820
{ path: '/settings/profile' },
1921
{ path: '/public/about' },
22+
{ path: '/features' },
2023
],
2124
dynamicRoutes: [
2225
{
@@ -28,6 +31,8 @@ describe('route-groups', () => {
2831
],
2932
isrRoutes: [],
3033
});
34+
// Verify we have 9 static routes total (including duplicates from special chars)
35+
expect(manifest.staticRoutes).toHaveLength(9);
3136
});
3237

3338
test('should handle dynamic routes within route groups', () => {
@@ -37,6 +42,17 @@ describe('route-groups', () => {
3742
expect(regex.test('/dashboard/abc')).toBe(true);
3843
expect(regex.test('/dashboard/123/456')).toBe(false);
3944
});
45+
46+
test.each([
47+
{ routeGroup: '(auth-v2)', strippedPath: '/login', description: 'hyphens' },
48+
{ routeGroup: '(api_internal)', strippedPath: '/api', description: 'underscores' },
49+
{ routeGroup: '(v2.0.beta)', strippedPath: '/features', description: 'dots' },
50+
])('should strip route groups with $description', ({ routeGroup, strippedPath }) => {
51+
// Verify the stripped path exists
52+
expect(manifest.staticRoutes.find(route => route.path === strippedPath)).toBeDefined();
53+
// Verify the route group was stripped, not included
54+
expect(manifest.staticRoutes.find(route => route.path.includes(routeGroup))).toBeUndefined();
55+
});
4056
});
4157

4258
describe('includeRouteGroups: true', () => {
@@ -46,11 +62,14 @@ describe('route-groups', () => {
4662
expect(manifest).toEqual({
4763
staticRoutes: [
4864
{ path: '/' },
65+
{ path: '/(api_internal)/api' },
4966
{ path: '/(auth)/login' },
5067
{ path: '/(auth)/signup' },
68+
{ path: '/(auth-v2)/login' },
5169
{ path: '/(dashboard)/dashboard' },
5270
{ path: '/(dashboard)/settings/profile' },
5371
{ path: '/(marketing)/public/about' },
72+
{ path: '/(v2.0.beta)/features' },
5473
],
5574
dynamicRoutes: [
5675
{
@@ -62,6 +81,7 @@ describe('route-groups', () => {
6281
],
6382
isrRoutes: [],
6483
});
84+
expect(manifest.staticRoutes).toHaveLength(9);
6585
});
6686

6787
test('should handle dynamic routes within route groups with proper regex escaping', () => {
@@ -92,5 +112,13 @@ describe('route-groups', () => {
92112
expect(authSignup).toBeDefined();
93113
expect(marketingPublic).toBeDefined();
94114
});
115+
116+
test.each([
117+
{ fullPath: '/(auth-v2)/login', description: 'hyphens' },
118+
{ fullPath: '/(api_internal)/api', description: 'underscores' },
119+
{ fullPath: '/(v2.0.beta)/features', description: 'dots' },
120+
])('should preserve route groups with $description when includeRouteGroups is true', ({ fullPath }) => {
121+
expect(manifest.staticRoutes.find(route => route.path === fullPath)).toBeDefined();
122+
});
95123
});
96124
});

0 commit comments

Comments
 (0)