Skip to content

Commit 117a5f9

Browse files
committed
fix(@angular/ssr): resolve query parameter placeholders in redirect targets
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.
1 parent 03996ec commit 117a5f9

4 files changed

Lines changed: 72 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

2934
/**
3035
* A set of well-known URLs that are not handled by Angular.
@@ -173,7 +178,7 @@ export class AngularServerApp {
173178

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

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,43 @@ export function buildPathWithParams(toPath: string, fromPath: string): string {
216216
return joinUrlParts(...resolvedParts);
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`, and is removed if `fromParams` has no such parameter.
224+
*
225+
* @param toUrl - A URL whose query may contain `:name` placeholders.
226+
* @param fromParams - The query parameters that provide the values for the placeholders.
227+
* @returns The URL with its query placeholders resolved.
228+
*
229+
* @example
230+
* ```typescript
231+
* buildQueryWithParams('/search?q=:term', new URLSearchParams('term=cats')); // returns '/search?q=cats'
232+
* ```
233+
*/
234+
export function buildQueryWithParams(toUrl: string, fromParams: URLSearchParams): string {
235+
const [, path, query, fragment] = /^([^?#]*)\?([^#]*)(.*)$/.exec(toUrl) ?? [];
236+
if (query === undefined || !query.includes('=:')) {
237+
return toUrl;
238+
}
239+
240+
const resolvedParams = new URLSearchParams();
241+
for (const [name, value] of new URLSearchParams(query)) {
242+
if (value[0] === ':') {
243+
for (const sourceValue of fromParams.getAll(value.slice(1))) {
244+
resolvedParams.append(name, sourceValue);
245+
}
246+
} else {
247+
resolvedParams.append(name, value);
248+
}
249+
}
250+
251+
const resolvedQuery = resolvedParams.toString();
252+
253+
return path + (resolvedQuery ? `?${resolvedQuery}` : '') + fragment;
254+
}
255+
219256
const MATRIX_PARAMS_REGEX = /;[^/]+/g;
220257

221258
/**

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

Lines changed: 12 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,14 @@ 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('/home?q=cats&q=dogs&static=value');
280+
expect(response?.status).toBe(302);
281+
});
282+
271283
it('should handle request abortion gracefully', async () => {
272284
const controller = new AbortController();
273285
const request = new Request('http://localhost/home', { signal: controller.signal });

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

Lines changed: 16 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,21 @@ 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=cats&term=dogs&page=2');
211+
expect(buildQueryWithParams('/search?q=:term&p=:page&static=1#top', fromParams)).toBe(
212+
'/search?q=cats&q=dogs&p=2&static=1#top',
213+
);
214+
});
215+
216+
it('should remove query parameters whose source parameter is missing', () => {
217+
const fromParams = new URLSearchParams();
218+
expect(buildQueryWithParams('/search?q=:term&static=1', fromParams)).toBe('/search?static=1');
219+
expect(buildQueryWithParams('/search?q=:term', fromParams)).toBe('/search');
220+
});
221+
});
222+
207223
describe('stripMatrixParams', () => {
208224
it('should remove a single matrix parameter', () => {
209225
expect(stripMatrixParams('/path;param=value')).toBe('/path');

0 commit comments

Comments
 (0)