fixup! fix(@angular/ssr): resolve query parameter placeholders in redirect targets

Author: bschaeublin

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

@@ -220,37 +220,56 @@ export function buildPathWithParams(toPath: string, fromPath: string): string {
  * Resolves `:name` placeholders in the query of a URL, like the Angular router does for redirects.
  *
  * Each query parameter whose value is a `:name` placeholder gets the values of the `name`
- * query parameter of `fromParams`, and is removed if `fromParams` has no such parameter.
+ * query parameter of `fromParams`, or `undefined` if `fromParams` has no such parameter.
+ * Like in the Angular router, placeholders of repeated query parameters are not resolved.
  *
  * @param toUrl - A URL whose query may contain `:name` placeholders.
  * @param fromParams - The query parameters that provide the values for the placeholders.
  * @returns The URL with its query placeholders resolved.
  *
  * @example
  * ```typescript
- * buildQueryWithParams('/search?q=:term', new URLSearchParams('term=cats')); // returns '/search?q=cats'
+ * buildQueryWithParams('/find?q=:x', new URLSearchParams('x=cats')); // returns '/find?q=cats'
  * ```
  */
 export function buildQueryWithParams(toUrl: string, fromParams: URLSearchParams): string {
   const [, path, query, fragment] = /^([^?#]*)\?([^#]*)(.*)$/.exec(toUrl) ?? [];
-  if (query === undefined || !query.includes('=:')) {
+  if (query === undefined) {
     return toUrl;
   }
 
-  const resolvedParams = new URLSearchParams();
-  for (const [name, value] of new URLSearchParams(query)) {
-    if (value[0] === ':') {
-      for (const sourceValue of fromParams.getAll(value.slice(1))) {
-        resolvedParams.append(name, sourceValue);
+  const params = query.split('&');
+  const names = params.map((param) => param.split('=', 1)[0]);
+  const resolvedQuery = params
+    .map((param, index) => {
+      const name = names[index];
+      const value = param.slice(name.length + 1);
+      if (value[0] !== ':' || names.indexOf(name) !== names.lastIndexOf(name)) {
+        return param;
       }
-    } else {
-      resolvedParams.append(name, value);
-    }
-  }
 
-  const resolvedQuery = resolvedParams.toString();
+      const sourceValues = fromParams.getAll(value.slice(1));
+
+      return (sourceValues.length ? sourceValues : ['undefined'])
+        .map((sourceValue) => `${name}=${encodeQueryValue(sourceValue)}`)
+        .join('&');
+    })
+    .join('&');
+
+  return `${path}?${resolvedQuery}${fragment}`;
+}
 
-  return path + (resolvedQuery ? `?${resolvedQuery}` : '') + fragment;
+/**
+ * Encodes a query parameter value like the Angular router (`DefaultUrlSerializer`), which keeps
+ * `@`, `:`, `$`, `,` and `;` as is.
+ *
+ * @param value - The query parameter value to encode.
+ * @returns The encoded query parameter value.
+ */
+function encodeQueryValue(value: string): string {
+  return encodeURIComponent(value).replace(/%(40|3A|24|2C|3B)/g, (match) =>
+    decodeURIComponent(match),
+  );
 }
 
 const MATRIX_PARAMS_REGEX = /;[^/]+/g;

packages/angular/ssr/test/app_spec.ts

@@ -276,7 +276,9 @@ describe('AngularServerApp', () => {
         const response = await app.handle(
           new Request('http://localhost/redirect-query-placeholder?term=cats&term=dogs'),
         );
-        expect(response?.headers.get('location')).toBe('/home?q=cats&q=dogs&static=value');
+        expect(response?.headers.get('location')).toBe(
+          '/home?q=cats&q=dogs&id=undefined&static=value',
+        );
         expect(response?.status).toBe(302);
       });
 

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

@@ -207,16 +207,22 @@ describe('URL Utils', () => {
 
   describe('buildQueryWithParams', () => {
     it('should replace query placeholders with the values of the source parameters', () => {
-      const fromParams = new URLSearchParams('term=cats&term=dogs&page=2');
-      expect(buildQueryWithParams('/search?q=:term&p=:page&static=1#top', fromParams)).toBe(
-        '/search?q=cats&q=dogs&p=2&static=1#top',
+      const fromParams = new URLSearchParams('term=big cats&term=a:b@c&page=2');
+      expect(buildQueryWithParams('/search?q=:term&p=:page&static=a=:b#top', fromParams)).toBe(
+        '/search?q=big%20cats&q=a:b@c&p=2&static=a=:b#top',
       );
     });
 
-    it('should remove query parameters whose source parameter is missing', () => {
+    it('should set query parameters whose source parameter is missing to undefined', () => {
       const fromParams = new URLSearchParams();
-      expect(buildQueryWithParams('/search?q=:term&static=1', fromParams)).toBe('/search?static=1');
-      expect(buildQueryWithParams('/search?q=:term', fromParams)).toBe('/search');
+      expect(buildQueryWithParams('/search?q=:term&static=1', fromParams)).toBe(
+        '/search?q=undefined&static=1',
+      );
+    });
+
+    it('should not replace placeholders of repeated query parameters', () => {
+      const fromParams = new URLSearchParams('a=1&b=2');
+      expect(buildQueryWithParams('/search?q=:a&q=:b', fromParams)).toBe('/search?q=:a&q=:b');
     });
   });