Merge branch '54218-fix-mergeUrlParams' into 'master'

Resolve "mergeUrlParams wrong with fragment url"

Closes #54218

See merge request gitlab-org/gitlab-ce!23193

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

@@ -17,27 +17,29 @@ export function getParameterValues(sParam) {
 // @param {Object} params - url keys and value to merge
 // @param {String} url
 export function mergeUrlParams(params, url) {
-  let newUrl = Object.keys(params).reduce((acc, paramName) => {
-    const paramValue = encodeURIComponent(params[paramName]);
-    const pattern = new RegExp(`\\b(${paramName}=).*?(&|$)`);
-
-    if (paramValue === null) {
-      return acc.replace(pattern, '');
-    } else if (url.search(pattern) !== -1) {
-      return acc.replace(pattern, `$1${paramValue}$2`);
-    }
-
-    return `${acc}${acc.indexOf('?') > 0 ? '&' : '?'}${paramName}=${paramValue}`;
-  }, decodeURIComponent(url));
+  const re = /^([^?#]*)(\?[^#]*)?(.*)/;
+  const merged = {};
+  const urlparts = url.match(re);
+
+  if (urlparts[2]) {
+    urlparts[2]
+      .substr(1)
+      .split('&')
+      .forEach(part => {
+        if (part.length) {
+          const kv = part.split('=');
+          merged[decodeURIComponent(kv[0])] = decodeURIComponent(kv.slice(1).join('='));
+        }
+      });
+  }
 
-  // Remove a trailing ampersand
-  const lastChar = newUrl[newUrl.length - 1];
+  Object.assign(merged, params);
 
-  if (lastChar === '&') {
-    newUrl = newUrl.slice(0, -1);
-  }
+  const query = Object.keys(merged)
+    .map(key => `${encodeURIComponent(key)}=${encodeURIComponent(merged[key])}`)
+    .join('&');
 
-  return newUrl;
+  return `${urlparts[1]}?${query}${urlparts[3]}`;
 }
 
 export function removeParamQueryString(url, param) {

changelogs/unreleased/54218-fix-mergeUrlParams.yml

+---
+title: "Fix mergeUrlParams with fragment URL"
+merge_request: 54218
+author: Thomas Holder
+type: fixed

spec/javascripts/lib/utils/url_utility_spec.js

-import { webIDEUrl } from '~/lib/utils/url_utility';
+import { webIDEUrl, mergeUrlParams } from '~/lib/utils/url_utility';
 
 describe('URL utility', () => {
   describe('webIDEUrl', () => {
@@ -26,4 +26,26 @@ describe('URL utility', () => {
       });
     });
   });
+
+  describe('mergeUrlParams', () => {
+    it('adds w', () => {
+      expect(mergeUrlParams({ w: 1 }, '#frag')).toBe('?w=1#frag');
+      expect(mergeUrlParams({ w: 1 }, '/path#frag')).toBe('/path?w=1#frag');
+      expect(mergeUrlParams({ w: 1 }, 'https://host/path')).toBe('https://host/path?w=1');
+      expect(mergeUrlParams({ w: 1 }, 'https://host/path#frag')).toBe('https://host/path?w=1#frag');
+      expect(mergeUrlParams({ w: 1 }, 'https://h/p?k1=v1#frag')).toBe('https://h/p?k1=v1&w=1#frag');
+    });
+
+    it('updates w', () => {
+      expect(mergeUrlParams({ w: 1 }, '?k1=v1&w=0#frag')).toBe('?k1=v1&w=1#frag');
+    });
+
+    it('adds multiple params', () => {
+      expect(mergeUrlParams({ a: 1, b: 2, c: 3 }, '#frag')).toBe('?a=1&b=2&c=3#frag');
+    });
+
+    it('adds and updates encoded params', () => {
+      expect(mergeUrlParams({ a: '&', q: '?' }, '?a=%23#frag')).toBe('?a=%26&q=%3F#frag');
+    });
+  });
 });