Skip to content

Commit 3beb255

Browse files
bschaeublinalan-agius4
authored andcommitted
fix(@angular/ssr): resolve query parameter placeholders in redirect targets (#34321)
Query parameters of an absolute `redirectTo` target whose value is a placeholder, such as `q=:term`, were sent unchanged in the `Location` header by the server, while the router replaces them with the values of the matching query parameters of the current URL. The server now does the same. Query parameters whose source parameter is missing in the current URL are left out, which matches the `undefined` value that the router assigns to them. PR Close #34321
1 parent ff3f372 commit 3beb255

4 files changed

Lines changed: 99 additions & 2 deletions

File tree

‎packages/angular/ssr/src/app.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ import { ServerRouter } from './routes/router';
2424
import { AngularBootstrap, renderAngular } from './utils/ng';
2525
import { promiseWithAbort } from './utils/promise';
2626
import { createRedirectResponse } from './utils/redirect';
27-
import { buildPathWithParams, joinUrlParts, stripLeadingSlash } from './utils/url';
27+
import {
28+
buildPathWithParams,
29+
buildQueryWithParams,
30+
joinUrlParts,
31+
stripLeadingSlash,
32+
} from './utils/url';
2833
import { getFirstHeaderValue } from './utils/validation';
2934

3035
/**
@@ -174,7 +179,7 @@ export class AngularServerApp {
174179

175180
// Match Angular router behavior: relative redirects keep the query string.
176181
return createRedirectResponse(
177-
relativeRedirect ? location + url.search : location,
182+
relativeRedirect ? location + url.search : buildQueryWithParams(location, url.searchParams),
178183
status,
179184
headers,
180185
);

‎packages/angular/ssr/src/utils/url.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,62 @@ export function buildPathWithParams(toPath: string, fromPath: string): string {
216216
return joinUrlParts(...resolvedParts) + toPath.slice(path.length);
217217
}
218218

219+
/**
220+
* Resolves `:name` placeholders in the query of a URL, like the Angular router does for redirects.
221+
*
222+
* Each query parameter whose value is a `:name` placeholder gets the values of the `name`
223+
* query parameter of `fromParams`, or `undefined` if `fromParams` has no such parameter.
224+
* Like in the Angular router, placeholders of repeated query parameters are not resolved.
225+
*
226+
* @param toUrl - A URL whose query may contain `:name` placeholders.
227+
* @param fromParams - The query parameters that provide the values for the placeholders.
228+
* @returns The URL with its query placeholders resolved.
229+
*
230+
* @example
231+
* ```typescript
232+
* buildQueryWithParams('/find?q=:x', new URLSearchParams('x=cats')); // returns '/find?q=cats'
233+
* ```
234+
*/
235+
export function buildQueryWithParams(toUrl: string, fromParams: URLSearchParams): string {
236+
const [, path, query, fragment] = /^([^?#]*)\?([^#]*)(.*)$/.exec(toUrl) ?? [];
237+
if (query === undefined) {
238+
return toUrl;
239+
}
240+
241+
const params = query.split('&');
242+
const names = params.map((param) => param.split('=', 1)[0]);
243+
const resolvedQuery = params
244+
.map((param, index) => {
245+
const name = names[index];
246+
const value = param.slice(name.length + 1);
247+
if (value[0] !== ':' || names.indexOf(name) !== names.lastIndexOf(name)) {
248+
return param;
249+
}
250+
251+
const sourceValues = fromParams.getAll(value.slice(1));
252+
253+
return (sourceValues.length ? sourceValues : ['undefined'])
254+
.map((sourceValue) => `${name}=${encodeQueryValue(sourceValue)}`)
255+
.join('&');
256+
})
257+
.join('&');
258+
259+
return `${path}?${resolvedQuery}${fragment}`;
260+
}
261+
262+
/**
263+
* Encodes a query parameter value like the Angular router (`DefaultUrlSerializer`), which keeps
264+
* `@`, `:`, `$`, `,` and `;` as is.
265+
*
266+
* @param value - The query parameter value to encode.
267+
* @returns The encoded query parameter value.
268+
*/
269+
function encodeQueryValue(value: string): string {
270+
return encodeURIComponent(value).replace(/%(40|3A|24|2C|3B)/g, (match) =>
271+
decodeURIComponent(match),
272+
);
273+
}
274+
219275
const MATRIX_PARAMS_REGEX = /;[^/]+/g;
220276

221277
/**

‎packages/angular/ssr/test/app_spec.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ describe('AngularServerApp', () => {
9292
{ path: 'redirect-nested/:param', children: [{ path: 'old', redirectTo: 'new' }] },
9393
{ path: 'redirect-param/:a/:b', redirectTo: '/home/:b/:a' },
9494
{ path: 'redirect-param-relative/:id', redirectTo: 'target/:id' },
95+
{
96+
path: 'redirect-query-placeholder',
97+
redirectTo: '/home?q=:term&id=:missing&static=value',
98+
},
9599
{
96100
path: 'redirect-to-function',
97101
redirectTo: () => 'home',
@@ -268,6 +272,16 @@ describe('AngularServerApp', () => {
268272
expect(response?.status).toBe(302);
269273
});
270274

275+
it('should resolve query parameters in absolute redirect targets', async () => {
276+
const response = await app.handle(
277+
new Request('http://localhost/redirect-query-placeholder?term=cats&term=dogs'),
278+
);
279+
expect(response?.headers.get('location')).toBe(
280+
'/home?q=cats&q=dogs&id=undefined&static=value',
281+
);
282+
expect(response?.status).toBe(302);
283+
});
284+
271285
it('should handle request abortion gracefully', async () => {
272286
const controller = new AbortController();
273287
const request = new Request('http://localhost/home', { signal: controller.signal });

‎packages/angular/ssr/test/utils/url_spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
addLeadingSlash,
1111
addTrailingSlash,
1212
buildPathWithParams,
13+
buildQueryWithParams,
1314
joinUrlParts,
1415
stripIndexHtmlFromURL,
1516
stripLeadingSlash,
@@ -204,6 +205,27 @@ describe('URL Utils', () => {
204205
});
205206
});
206207

208+
describe('buildQueryWithParams', () => {
209+
it('should replace query placeholders with the values of the source parameters', () => {
210+
const fromParams = new URLSearchParams('term=big cats&term=a:b@c&page=2');
211+
expect(buildQueryWithParams('/search?q=:term&p=:page&static=a=:b#top', fromParams)).toBe(
212+
'/search?q=big%20cats&q=a:b@c&p=2&static=a=:b#top',
213+
);
214+
});
215+
216+
it('should set query parameters whose source parameter is missing to undefined', () => {
217+
const fromParams = new URLSearchParams();
218+
expect(buildQueryWithParams('/search?q=:term&static=1', fromParams)).toBe(
219+
'/search?q=undefined&static=1',
220+
);
221+
});
222+
223+
it('should not replace placeholders of repeated query parameters', () => {
224+
const fromParams = new URLSearchParams('a=1&b=2');
225+
expect(buildQueryWithParams('/search?q=:a&q=:b', fromParams)).toBe('/search?q=:a&q=:b');
226+
});
227+
});
228+
207229
describe('stripMatrixParams', () => {
208230
it('should remove a single matrix parameter', () => {
209231
expect(stripMatrixParams('/path;param=value')).toBe('/path');

0 commit comments

Comments
 (0)