Skip to content

Commit 128ff68

Browse files
committed
fix(@angular/ssr): use router to normalize URLs for comparison
Updates `constructDecodedUrl` in the SSR engine to use the Angular `Router` for parsing and serializing URLs instead of manual string manipulation and decoding. This ensures that the URL comparison used to determine if a redirect is necessary is consistent with how the router interprets and serializes the URL. This prevents issues where differences in encoding or edge cases (like duplicate query parameters or empty values) could lead to incorrect comparison results and unexpected redirects. Also updates tests to include edge cases for query parameters and paths to verify this behavior. Fixes #33053
1 parent 5adc925 commit 128ff68

2 files changed

Lines changed: 34 additions & 15 deletions

File tree

packages/angular/ssr/src/utils/ng.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ export async function renderAngular(
126126
envInjector.get(REQUEST, null, { optional: true })?.headers.get('X-Forwarded-Prefix');
127127

128128
const { pathname, search, hash } = envInjector.get(PlatformLocation);
129-
const finalUrl = constructDecodedUrl({ pathname, search, hash }, requestPrefix);
130-
const urlToRenderString = constructDecodedUrl(urlToRender, requestPrefix);
129+
const finalUrl = constructSerializedUrl(router, { pathname, search, hash }, requestPrefix);
130+
const urlToRenderString = constructSerializedUrl(router, urlToRender, requestPrefix);
131131

132132
if (urlToRenderString !== finalUrl) {
133133
redirectTo = [pathname, search, hash].join('');
@@ -190,22 +190,27 @@ function asyncDestroyPlatform(platformRef: PlatformRef): Promise<void> {
190190
}
191191

192192
/**
193-
* Constructs a decoded URL string from its components, ensuring consistency for comparison.
193+
* Constructs a normalized and serialized URL string from its components.
194194
*
195-
* This function takes a URL-like object (containing `pathname`, `search`, and `hash`),
196-
* strips the trailing slash from the pathname, joins the components, and then decodes
197-
* the entire string. This normalization is crucial for accurately comparing URLs
198-
* that might differ only in encoding or trailing slashes.
195+
* This function uses the provided `Router` instance to parse and serialize the URL,
196+
* ensuring that the resulting string is consistent with the router's configuration.
197+
* It also handles the optional `prefix` parameter to ensure proper URL construction.
199198
*
199+
* @param router - The `Router` instance to use for parsing and serializing the URL.
200200
* @param url - An object containing the URL components:
201201
* - `pathname`: The path of the URL.
202202
* - `search`: The query string of the URL (including '?').
203203
* - `hash`: The hash fragment of the URL (including '#').
204204
* @param prefix - An optional prefix (e.g., `APP_BASE_HREF`) to prepend to the pathname
205205
* if it is not already present.
206-
* @returns The constructed and decoded URL string.
206+
* @returns The normalized and serialized URL string.
207+
*
208+
* @note
209+
* We use the Angular `Router` to construct the URL, so that the URL is consistent with the router's configuration.
210+
* This is important for the URL to be correctly parsed and serialized by the router as it might have different encodings.
207211
*/
208-
function constructDecodedUrl(
212+
function constructSerializedUrl(
213+
router: Router,
209214
url: { pathname: string; search: string; hash: string },
210215
prefix?: string | null,
211216
): string {
@@ -219,5 +224,7 @@ function constructDecodedUrl(
219224

220225
urlParts.push(search, hash);
221226

222-
return decodeURIComponent(urlParts.join(''));
227+
const urlTree = router.parseUrl(urlParts.join(''));
228+
229+
return router.serializeUrl(urlTree);
223230
}

packages/angular/ssr/test/app_spec.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,23 @@ describe('AngularServerApp', () => {
334334
expect(response?.status).toBe(302);
335335
});
336336

337-
it('should work with encoded characters', async () => {
338-
const request = new Request('http://localhost/home?email=xyz%40xyz.com');
339-
const response = await app.handle(request);
340-
expect(response?.status).toBe(200);
341-
expect(await response?.text()).toContain('Home works');
337+
it('should work with complex and encoded URLs', async () => {
338+
const urls = [
339+
'http://localhost/home?email=xyz%40xyz.com',
340+
'http://localhost/home?empty',
341+
'http://localhost/home?scope=email+profile',
342+
'http://localhost/home?bbb=1&aaa=2&bbb=3',
343+
'http://localhost//home',
344+
];
345+
346+
for (const url of urls) {
347+
const request = new Request(url);
348+
const response = await app.handle(request);
349+
expect(response?.status).withContext(`url: ${url}`).toBe(200);
350+
expect(await response?.text())
351+
.withContext(`url: ${url}`)
352+
.toContain('Home works');
353+
}
342354
});
343355

344356
it('should work with decoded characters', async () => {

0 commit comments

Comments
 (0)