Skip to content

Commit f9f3864

Browse files
committed
fix(@angular/build): match proxy glob patterns against URL pathname only
When normalizing proxy configurations for Vite, glob patterns are converted into regular expression strings using picomatch. Because picomatch builds regular expressions assuming the input is a file path and anchors them with `$`, whereas Vite tests regex proxy keys against `req.url` (which includes any query string), requests with query parameters could fail to match glob entries such as `/search/**` or `/api/*.json`, or inadvertently match wildcards against query string parameters. Adapt the generated regular expression so that wildcard tokens do not match across the `?` query delimiter and end-of-path anchors allow an optional query string after the matched pathname. Closes #34257
1 parent b42cfee commit f9f3864

3 files changed

Lines changed: 211 additions & 2 deletions

File tree

‎packages/angular/build/src/builders/dev-server/tests/options/proxy-config_spec.ts‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,56 @@ describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupT
330330
await otherProxyServer.close();
331331
}
332332
});
333+
334+
it('matches glob entries against the URL pathname when a query string is present', async () => {
335+
harness.useTarget('serve', {
336+
...BASE_OPTIONS,
337+
proxyConfig: 'proxy.config.json',
338+
});
339+
340+
const proxyServer = await createProxyServer();
341+
try {
342+
await harness.writeFiles({
343+
'proxy.config.json': `
344+
{
345+
"/api/test/**": { "target": "http://127.0.0.1:${proxyServer.address.port}" }
346+
}
347+
`,
348+
});
349+
350+
const { result, response } = await executeOnceAndFetch(harness, '/api/test?q=angular');
351+
352+
expect(result?.success).toBeTrue();
353+
expect(await response?.text()).toContain('TEST_API_RETURN');
354+
} finally {
355+
await proxyServer.close();
356+
}
357+
});
358+
359+
it('does not match glob wildcards against query string parameters', async () => {
360+
harness.useTarget('serve', {
361+
...BASE_OPTIONS,
362+
proxyConfig: 'proxy.config.json',
363+
});
364+
365+
const proxyServer = await createProxyServer();
366+
try {
367+
await harness.writeFiles({
368+
'proxy.config.json': `
369+
{
370+
"/api/*.json": { "target": "http://127.0.0.1:${proxyServer.address.port}" }
371+
}
372+
`,
373+
});
374+
375+
const { result, response } = await executeOnceAndFetch(harness, '/api/test?file=data.json');
376+
377+
expect(result?.success).toBeTrue();
378+
expect(await response?.text()).not.toContain('TEST_API_RETURN');
379+
} finally {
380+
await proxyServer.close();
381+
}
382+
});
333383
});
334384
});
335385

@@ -340,7 +390,7 @@ describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupT
340390
*/
341391
async function createProxyServer(body = 'TEST_API_RETURN') {
342392
const proxyServer = createServer((request, response) => {
343-
if (request.url?.endsWith('/test')) {
393+
if (request.url?.split('?')[0].endsWith('/test')) {
344394
response.writeHead(200);
345395
response.end(body);
346396
} else {

‎packages/angular/build/src/utils/load-proxy-config.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ function normalizeProxyConfiguration(
118118
// since Vite proxies a request with the first entry that matches it.
119119
const orderedProxy: Record<string, object> = {};
120120
for (const [key, value] of Object.entries(normalizedProxy)) {
121-
const context = key[0] !== '^' && isDynamicPattern(key) ? makeRegExpFromGlob(key).source : key;
121+
const context = key[0] !== '^' && isDynamicPattern(key) ? makeUrlRegExpFromGlob(key) : key;
122122
orderedProxy[context] = value;
123123
}
124124
normalizedProxy = orderedProxy;
@@ -151,6 +151,36 @@ function normalizeProxyConfiguration(
151151
return normalizedProxy;
152152
}
153153

154+
/**
155+
* Converts a glob pattern to a regular expression source string that matches
156+
* the pathname portion of a request URL while allowing an optional query string.
157+
*
158+
* Vite tests regex proxy keys against `req.url`, which includes the query string,
159+
* whereas `picomatch` generates regexes assuming the input is only a pathname.
160+
*/
161+
function makeUrlRegExpFromGlob(glob: string): string {
162+
const { source } = makeRegExpFromGlob(glob);
163+
164+
return (
165+
source
166+
// Ensure picomatch's "at least one character" lookahead does not
167+
// treat the `?` query delimiter as a pathname character (e.g., `/api/*` on `/api/?a=1`).
168+
.replaceAll('(?=.)', '(?=[^?])')
169+
// Prevent single-segment wildcards (`*` and `?`) from matching
170+
// across `?` into the query string (e.g., `/api/*.json` on `/api/users?file=data.json`).
171+
.replaceAll('[^/]', '[^/?]')
172+
// Prevent leading-dot-aware `?` wildcards from matching the `?` delimiter.
173+
.replaceAll('[^.\\/]', '[^.\\/?]')
174+
// Prevent multi-segment globstars (`**`) from matching across `?` into the query string
175+
// (e.g., `/api/**/users` on `/api/other?redirect=/users`).
176+
.replaceAll('\\.).)*?', '\\.)[^?])*?')
177+
// Replace unescaped end-of-input anchors (`$`, both at the end of the pattern and inside
178+
// internal alternation groups for trailing `/**` or negated globs) so an optional query
179+
// string is permitted after the matched pathname.
180+
.replace(/(?<!\\)\$/g, '(?:\\?.*)?$')
181+
);
182+
}
183+
154184
function pathRewriter(pathRewriteEntries: [RegExp, string][], path: string): string {
155185
for (const [pattern, value] of pathRewriteEntries) {
156186
const updated = path.replace(pattern, value);
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import assert from 'node:assert';
10+
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
11+
import { join } from 'node:path';
12+
import { loadProxyConfiguration } from './load-proxy-config';
13+
14+
describe('loadProxyConfiguration', () => {
15+
let tempDir: string;
16+
17+
beforeEach(async () => {
18+
const baseTmpDir = process.env['TEST_TMPDIR'];
19+
assert(baseTmpDir, 'TEST_TMPDIR is not set');
20+
tempDir = await mkdtemp(join(baseTmpDir, 'angular-proxy-config-spec-'));
21+
});
22+
23+
afterEach(async () => {
24+
await rm(tempDir, { recursive: true, force: true });
25+
});
26+
27+
async function createGlobMatcher(glob: string): Promise<(url: string) => boolean> {
28+
const configPath = join(tempDir, 'proxy.conf.json');
29+
await writeFile(configPath, JSON.stringify({ [glob]: { target: 'http://localhost:3000' } }));
30+
const proxy = await loadProxyConfiguration(tempDir, 'proxy.conf.json');
31+
assert(proxy, 'Expected proxy configuration to be loaded');
32+
const keys = Object.keys(proxy);
33+
expect(keys.length).toBe(1);
34+
const regex = new RegExp(keys[0]);
35+
36+
return (url: string) => regex.test(url);
37+
}
38+
39+
describe('glob to RegExp conversion with query strings', () => {
40+
it('handles trailing globstar (/**) with and without query strings', async () => {
41+
const matches = await createGlobMatcher('/search/**');
42+
43+
expect(matches('/search')).toBeTrue();
44+
expect(matches('/search?q=angular')).toBeTrue();
45+
expect(matches('/search/')).toBeTrue();
46+
expect(matches('/search/?q=angular')).toBeTrue();
47+
expect(matches('/search/items')).toBeTrue();
48+
expect(matches('/search/items?q=angular')).toBeTrue();
49+
expect(matches('/search/items?callback=/.hidden')).toBeTrue();
50+
51+
expect(matches('/searching')).toBeFalse();
52+
expect(matches('/searching?q=angular')).toBeFalse();
53+
expect(matches('/other?redirect=/search')).toBeFalse();
54+
});
55+
56+
it('handles single-segment wildcard (*) without matching across slashes or query strings', async () => {
57+
const matches = await createGlobMatcher('/api/*');
58+
59+
expect(matches('/api/users')).toBeTrue();
60+
expect(matches('/api/users?id=1')).toBeTrue();
61+
expect(matches('/api/users?redirect=/home/page')).toBeTrue();
62+
63+
expect(matches('/api')).toBeFalse();
64+
expect(matches('/api/')).toBeFalse();
65+
expect(matches('/api/?a=1')).toBeFalse();
66+
expect(matches('/api/users/profile')).toBeFalse();
67+
expect(matches('/api/users/profile?a=1')).toBeFalse();
68+
});
69+
70+
it('handles suffix globs (*.json) without matching query parameters', async () => {
71+
const matches = await createGlobMatcher('/api/*.json');
72+
73+
expect(matches('/api/data.json')).toBeTrue();
74+
expect(matches('/api/data.json?v=1')).toBeTrue();
75+
76+
expect(matches('/api/users')).toBeFalse();
77+
expect(matches('/api/users?file=data.json')).toBeFalse();
78+
expect(matches('/api/.json?v=1')).toBeFalse();
79+
});
80+
81+
it('handles middle globstar (/**/) without matching paths in query parameters', async () => {
82+
const matches = await createGlobMatcher('/api/**/users');
83+
84+
expect(matches('/api/users')).toBeTrue();
85+
expect(matches('/api/users?page=1')).toBeTrue();
86+
expect(matches('/api/v1/users?page=1')).toBeTrue();
87+
expect(matches('/api/v1/v2/users?page=1')).toBeTrue();
88+
89+
expect(matches('/api/other?redirect=/users')).toBeFalse();
90+
expect(matches('/api/users/profile?page=1')).toBeFalse();
91+
});
92+
93+
it('handles single-character wildcard (?) without matching the query delimiter', async () => {
94+
const matches = await createGlobMatcher('/api/v?');
95+
96+
expect(matches('/api/v1')).toBeTrue();
97+
expect(matches('/api/v1?a=1')).toBeTrue();
98+
99+
expect(matches('/api/v')).toBeFalse();
100+
expect(matches('/api/v?')).toBeFalse();
101+
expect(matches('/api/v?a=1')).toBeFalse();
102+
expect(matches('/api/v12?a=1')).toBeFalse();
103+
});
104+
105+
it('handles negated globs (!) with and without query strings', async () => {
106+
const matches = await createGlobMatcher('!/api/public/**');
107+
108+
expect(matches('/api/public')).toBeFalse();
109+
expect(matches('/api/public?foo=1')).toBeFalse();
110+
expect(matches('/api/public/assets?foo=1')).toBeFalse();
111+
112+
expect(matches('/api/private')).toBeTrue();
113+
expect(matches('/api/private?foo=1')).toBeTrue();
114+
});
115+
116+
it('handles brace expansion and extglobs with query strings', async () => {
117+
const matchesBraces = await createGlobMatcher('/api/{auth,users}/**');
118+
expect(matchesBraces('/api/auth?q=1')).toBeTrue();
119+
expect(matchesBraces('/api/users/list?q=1')).toBeTrue();
120+
expect(matchesBraces('/api/other?q=1')).toBeFalse();
121+
122+
const matchesExtglob = await createGlobMatcher('/**/!(*.d).ts');
123+
expect(matchesExtglob('/app/main.ts')).toBeTrue();
124+
expect(matchesExtglob('/app/main.ts?extra=.d.ts')).toBeTrue();
125+
expect(matchesExtglob('/app/main.d.ts')).toBeFalse();
126+
expect(matchesExtglob('/app/main.d.ts?extra=.ts')).toBeFalse();
127+
});
128+
});
129+
});

0 commit comments

Comments
 (0)