Skip to content

Commit c122aac

Browse files
authored
fix(v9/nextjs): Align tunnel request matching in middleware with tunnel rewrite (#24760)
Backport of: #24499 ## Differences to the original PR - `packages/nextjs/src/common/utils/tunnelPathnameMatch.ts`: this file doesn't exist on v9, so it's added here with only `isSentryTunnelRequest`. `isPathnameUnderSentryTunnelRoute` is left out because nothing on v9 uses it. - On v9 the middleware checked tunnel requests with a plain `pathname.startsWith(tunnelRoute)`, so a path like `/api/things` counted as a tunnel request for `/api/t`. `develop` fixed that in an earlier PR. The new exact matching fixes it on v9 too, so the test for that case from `develop` is included here.
1 parent 0224444 commit c122aac

3 files changed

Lines changed: 85 additions & 17 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Returns true only for requests the tunnel rewrite (see `setUpTunnelRewriteRules`) would serve.
3+
*
4+
* This decides whether the user's middleware is skipped, so it must never be broader than the rewrite:
5+
* anything it matches that Next.js does not rewrite to Sentry reaches the app without middleware.
6+
*/
7+
export function isSentryTunnelRequest(request: Request, tunnelPath: string): boolean {
8+
// The SDK transport only ever sends POST requests
9+
if (request.method !== 'POST') {
10+
return false;
11+
}
12+
13+
const url = new URL(request.url);
14+
15+
if (url.pathname !== tunnelPath && url.pathname !== `${tunnelPath}/`) {
16+
return false;
17+
}
18+
19+
// Next.js evaluates `has` conditions against the last value of a repeated query param, so every value has to qualify
20+
return ['o', 'p'].every(key => {
21+
const values = url.searchParams.getAll(key);
22+
return values.length > 0 && values.every(value => /^\d+$/.test(value));
23+
});
24+
}

‎packages/nextjs/src/common/wrapMiddlewareWithSentry.ts‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
withIsolationScope,
1515
} from '@sentry/core';
1616
import { flushSafelyWithTimeout } from '../common/utils/responseEnd';
17+
import { isSentryTunnelRequest } from '../common/utils/tunnelPathnameMatch';
1718
import type { EdgeRouteHandler } from '../edge/types';
1819

1920
/**
@@ -34,22 +35,16 @@ export function wrapMiddlewareWithSentry<H extends EdgeRouteHandler>(
3435

3536
if (tunnelRoute && typeof tunnelRoute === 'string') {
3637
const req: unknown = args[0];
37-
// Check if the current request matches the tunnel route
38-
if (req instanceof Request) {
39-
const url = new URL(req.url);
40-
const isTunnelRequest = url.pathname.startsWith(tunnelRoute);
41-
42-
if (isTunnelRequest) {
43-
// Create a simple response that mimics NextResponse.next() so we don't need to import internals here
44-
// which breaks next 13 apps
45-
// https://github.com/vercel/next.js/blob/c12c9c1f78ad384270902f0890dc4cd341408105/packages/next/src/server/web/spec-extension/response.ts#L146
46-
return new Response(null, {
47-
status: 200,
48-
headers: {
49-
'x-middleware-next': '1',
50-
},
51-
}) as ReturnType<H>;
52-
}
38+
if (req instanceof Request && isSentryTunnelRequest(req, tunnelRoute)) {
39+
// Create a simple response that mimics NextResponse.next() so we don't need to import internals here
40+
// which breaks next 13 apps
41+
// https://github.com/vercel/next.js/blob/c12c9c1f78ad384270902f0890dc4cd341408105/packages/next/src/server/web/spec-extension/response.ts#L146
42+
return new Response(null, {
43+
status: 200,
44+
headers: {
45+
'x-middleware-next': '1',
46+
},
47+
}) as ReturnType<H>;
5348
}
5449
}
5550
// TODO: We still should add central isolation scope creation for when our build-time instrumentation does not work anymore with turbopack.

‎packages/nextjs/test/config/wrappers.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ describe('wrapMiddlewareWithSentry', () => {
106106
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);
107107

108108
// Create a mock Request that matches the tunnel route
109-
const mockRequest = new Request('https://example.com/monitoring/tunnel?o=123');
109+
const mockRequest = new Request('https://example.com/monitoring/tunnel?o=123&p=456', { method: 'POST' });
110110

111111
const result = await wrappedOriginal(mockRequest);
112112

@@ -187,4 +187,53 @@ describe('wrapMiddlewareWithSentry', () => {
187187
expect(origFunction).toHaveBeenCalledWith(mockRequest);
188188
expect(result).toBe(mockReturnValue);
189189
});
190+
191+
test('should not treat paths as tunnel when they only share a prefix with tunnelRoute', async () => {
192+
(globalThis as any)._sentryRewritesTunnelPath = '/api/t';
193+
194+
const mockReturnValue = { status: 200 };
195+
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => mockReturnValue);
196+
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);
197+
198+
const mockRequest = new Request('https://example.com/api/things', { method: 'GET' });
199+
200+
const result = await wrappedOriginal(mockRequest);
201+
202+
expect(origFunction).toHaveBeenCalledWith(mockRequest);
203+
expect(result).toBe(mockReturnValue);
204+
});
205+
206+
test('should skip processing for the tunnel route with a trailing slash', async () => {
207+
(globalThis as any)._sentryRewritesTunnelPath = '/monitoring';
208+
209+
const origFunction: EdgeRouteHandler = vi.fn(async () => ({ status: 200 }));
210+
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);
211+
212+
await wrappedOriginal(new Request('https://example.com/monitoring/?o=123&p=456&r=us', { method: 'POST' }));
213+
214+
expect(origFunction).not.toHaveBeenCalled();
215+
});
216+
217+
test.each([
218+
['a sub-path of the tunnel route', 'https://example.com/monitoring/anything/at/all?o=123&p=456', 'POST'],
219+
['a tunnel request without query params', 'https://example.com/monitoring', 'POST'],
220+
['a tunnel request without project id', 'https://example.com/monitoring?o=123', 'POST'],
221+
['a tunnel request with non-numeric ids', 'https://example.com/monitoring?o=abc&p=456', 'POST'],
222+
['a tunnel request with a repeated non-numeric org id', 'https://example.com/monitoring?o=123&o=abc&p=456', 'POST'],
223+
['a tunnel request with a repeated empty project id', 'https://example.com/monitoring?o=123&p=456&p=', 'POST'],
224+
['a non-POST tunnel request', 'https://example.com/monitoring?o=123&p=456', 'GET'],
225+
])('should run the middleware for %s', async (_, url, method) => {
226+
(globalThis as any)._sentryRewritesTunnelPath = '/monitoring';
227+
228+
const mockReturnValue = { status: 200 };
229+
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => mockReturnValue);
230+
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);
231+
232+
const mockRequest = new Request(url, { method });
233+
234+
const result = await wrappedOriginal(mockRequest);
235+
236+
expect(origFunction).toHaveBeenCalledWith(mockRequest);
237+
expect(result).toBe(mockReturnValue);
238+
});
190239
});

0 commit comments

Comments
 (0)