Fix(mergeUrlparams): handle arrays gracefully

Ensuring that mergeUrlParams handles arrays to
mitigate problems when using this utility to
merge parameters that includes array values

Related issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/232465

Related merge request:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39588

app/assets/javascripts/lib/utils/url_utility.js

@@ -71,29 +71,56 @@ export function getParameterValues(sParam, url = window.location) {
  *
  * @param {Object} params - url keys and value to merge
  * @param {String} url
+ * @param {Object} options
+ * @param {Boolean} options.spreadArrays - split array values into separate key/value-pairs
  */
-export function mergeUrlParams(params, url) {
+export function mergeUrlParams(params, url, options = {}) {
+  const { spreadArrays = false } = options;
   const re = /^([^?#]*)(\?[^#]*)?(.*)/;
-  const merged = {};
+  let merged = {};
   const [, fullpath, query, fragment] = url.match(re);
 
   if (query) {
-    query
+    merged = query
       .substr(1)
       .split('&')
-      .forEach(part => {
+      .reduce((memo, part) => {
         if (part.length) {
           const kv = part.split('=');
-          merged[decodeUrlParameter(kv[0])] = decodeUrlParameter(kv.slice(1).join('='));
+          let key = decodeUrlParameter(kv[0]);
+          const value = decodeUrlParameter(kv.slice(1).join('='));
+          if (spreadArrays && key.endsWith('[]')) {
+            key = key.slice(0, -2);
+            if (!Array.isArray(memo[key])) {
+              return { ...memo, [key]: [value] };
             }
-      });
+            memo[key].push(value);
+
+            return memo;
+          }
+
+          return { ...memo, [key]: value };
+        }
+
+        return memo;
+      }, {});
   }
 
   Object.assign(merged, params);
 
   const newQuery = Object.keys(merged)
     .filter(key => merged[key] !== null)
-    .map(key => `${encodeURIComponent(key)}=${encodeURIComponent(merged[key])}`)
+    .map(key => {
+      let value = merged[key];
+      const encodedKey = encodeURIComponent(key);
+      if (spreadArrays && Array.isArray(value)) {
+        value = merged[key]
+          .map(arrayValue => encodeURIComponent(arrayValue))
+          .join(`&${encodedKey}[]=`);
+        return `${encodedKey}[]=${value}`;
+      }
+      return `${encodedKey}=${encodeURIComponent(value)}`;
+    })
     .join('&');
 
   if (newQuery) {

spec/frontend/lib/utils/url_utility_spec.js

@@ -160,6 +160,118 @@ describe('URL utility', () => {
         'https://host/path?op=%2B&foo=bar',
       );
     });
+
+    describe('with spread array option', () => {
+      const spreadArrayOptions = { spreadArrays: true };
+
+      it('maintains multiple values', () => {
+        expect(mergeUrlParams({}, '?array[]=foo&array[]=bar', spreadArrayOptions)).toBe(
+          '?array[]=foo&array[]=bar',
+        );
+      });
+
+      it('overrides multiple values with one', () => {
+        expect(
+          mergeUrlParams({ array: ['baz'] }, '?array[]=foo&array[]=bar', spreadArrayOptions),
+        ).toBe('?array[]=baz');
+      });
+      it('removes existing params', () => {
+        expect(
+          mergeUrlParams({ array: null }, '?array[]=foo&array[]=bar', spreadArrayOptions),
+        ).toBe('');
+      });
+      it('removes existing params and keeps others', () => {
+        expect(
+          mergeUrlParams(
+            { array: null },
+            '?array[]=foo&array[]=bar&other=quis',
+            spreadArrayOptions,
+          ),
+        ).toBe('?other=quis');
+      });
+      it('removes existing params along others', () => {
+        expect(
+          mergeUrlParams(
+            { array: null, other: 'quis' },
+            '?array[]=foo&array[]=bar',
+            spreadArrayOptions,
+          ),
+        ).toBe('?other=quis');
+      });
+      it('handles empty arrays along other parameters', () => {
+        expect(mergeUrlParams({ array: [], other: 'quis' }, '?array=baz', spreadArrayOptions)).toBe(
+          '?array[]=&other=quis',
+        );
+      });
+      it('handles multiple values along other parameters', () => {
+        expect(
+          mergeUrlParams(
+            { array: ['foo', 'bar'], other: 'quis' },
+            '?array=baz',
+            spreadArrayOptions,
+          ),
+        ).toBe('?array[]=foo&array[]=bar&other=quis');
+      });
+      it('handles array values with encoding', () => {
+        expect(
+          mergeUrlParams({ array: ['foo+', 'bar,baz'] }, '?array[]=%2Fbaz', spreadArrayOptions),
+        ).toBe('?array[]=foo%2B&array[]=bar%2Cbaz');
+      });
+      it('handles multiple arrays', () => {
+        expect(
+          mergeUrlParams(
+            { array1: ['foo+', 'bar,baz'], array2: ['quis', 'quux'] },
+            '?array1[]=%2Fbaz',
+            spreadArrayOptions,
+          ),
+        ).toBe('?array1[]=foo%2B&array1[]=bar%2Cbaz&array2[]=quis&array2[]=quux');
+      });
+    });
+
+    describe('without spread array option', () => {
+      it('maintains multiple values', () => {
+        expect(mergeUrlParams({}, '?array=foo%2Cbar')).toBe('?array=foo%2Cbar');
+      });
+      it('overrides multiple values with one', () => {
+        expect(mergeUrlParams({ array: ['baz'] }, '?array=foo%2Cbar')).toBe('?array=baz');
+      });
+      it('removes existing params', () => {
+        expect(mergeUrlParams({ array: null }, '?array=foo%2Cbar')).toBe('');
+      });
+      it('removes existing params and keeps others', () => {
+        expect(mergeUrlParams({ array: null }, '?array=foo&array=bar&other=quis')).toBe(
+          '?other=quis',
+        );
+      });
+      it('removes existing params along others', () => {
+        expect(mergeUrlParams({ array: null, other: 'quis' }, '?array=foo&array=bar')).toBe(
+          '?other=quis',
+        );
+      });
+      it('handles empty arrays along other parameters', () => {
+        expect(mergeUrlParams({ array: [], other: 'quis' }, '?array=baz')).toBe(
+          '?array=&other=quis',
+        );
+      });
+      it('handles multiple values along other parameters', () => {
+        expect(mergeUrlParams({ array: ['foo', 'bar'], other: 'quis' }, '?array=baz')).toBe(
+          '?array=foo%2Cbar&other=quis',
+        );
+      });
+      it('handles array values with encoding', () => {
+        expect(mergeUrlParams({ array: ['foo+', 'bar,baz'] }, '?array=%2Fbaz')).toBe(
+          '?array=foo%2B%2Cbar%2Cbaz',
+        );
+      });
+      it('handles multiple arrays', () => {
+        expect(
+          mergeUrlParams(
+            { array1: ['foo+', 'bar,baz'], array2: ['quis', 'quux'] },
+            '?array1=%2Fbaz',
+          ),
+        ).toBe('?array1=foo%2B%2Cbar%2Cbaz&array2=quis%2Cquux');
+      });
+    });
   });
 
   describe('removeParams', () => {