Refactor not to use URLSearchParams, because it led to double decoding

Author: apata

assets/js/dashboard/query.test.ts

@@ -12,7 +12,7 @@ describe(`${maybeGetRedirectTargetFromLegacySearchParams.name}`, () => {
   ])('for modern search %p returns null', (search) => {
     expect(
       maybeGetRedirectTargetFromLegacySearchParams({
-        pathname: '/example.com%2Fdeeper',
+        pathname: '/example.com%2Fdeep%2Fpath',
         search
       } as Location)
     ).toBeNull()

assets/js/dashboard/util/url-search-params.test.ts

@@ -45,7 +45,7 @@ describe(`${isSearchEntryDefined.name}`, () => {
   })
 })
 
-describe(`${serializeLabelsEntry.name} and decodeURIComponent(${parseLabelsEntry.name}(...)) are opposite of each other`, () => {
+describe(`${serializeLabelsEntry.name} and ${parseLabelsEntry.name}(...) are opposite of each other`, () => {
   test.each<[[string, string], string]>([
     [['US', 'United States'], 'US,United%20States'],
     [['FR-IDF', 'Île-de-France'], 'FR-IDF,%C3%8Ele-de-France'],
@@ -55,23 +55,27 @@ describe(`${serializeLabelsEntry.name} and decodeURIComponent(${parseLabelsEntry
     (entry, expected) => {
       const serialized = serializeLabelsEntry(entry)
       expect(serialized).toEqual(expected)
-      expect(parseLabelsEntry(decodeURIComponent(serialized))).toEqual(entry)
+      expect(parseLabelsEntry(serialized)).toEqual(entry)
     }
   )
 })
 
-describe(`${serializeFilter.name} and decodeURIComponent(${parseFilter.name}(...)) are opposite of each other`, () => {
+describe(`${serializeFilter.name} and ${parseFilter.name}(...) are opposite of each other`, () => {
   test.each<[Filter, string]>([
     [
-      ['contains', 'entry_page', ['/forecast/:city', 'ü']],
-      'contains,entry_page,/forecast/:city,%C3%BC'
+      ['contains', 'entry_page', ['/forecast/:city', ',"\'']],
+      "contains,entry_page,/forecast/:city,%2C%22'"
+    ],
+    [
+      ['is', 'props:complex/prop-with-comma-etc,$#%', ['(none)']],
+      'is,props:complex/prop-with-comma-etc%2C%24%23%25,(none)'
     ]
   ])(
     'filter %p serializes to %p, parses back to original',
     (filter, expected) => {
       const serialized = serializeFilter(filter)
       expect(serialized).toEqual(expected)
-      expect(parseFilter(decodeURIComponent(serialized))).toEqual(filter)
+      expect(parseFilter(serialized)).toEqual(filter)
     }
   )
 })

assets/js/dashboard/util/url-search-params.ts

@@ -32,34 +32,38 @@ export function stringifySearch(
 }
 
 export function parseSearch(searchString: string): Record<string, unknown> {
-  const urlSearchParams = new URLSearchParams(searchString)
   const searchRecord: Record<string, string | boolean> = {}
   const filters: Filter[] = []
   const labels: FilterClauseLabels = {}
-  urlSearchParams.forEach((value, key) => {
+
+  for (const param of searchString.startsWith('?')
+    ? searchString.slice(1).split('&')
+    : searchString.split('&')) {
+    const [key, rawValue] = param.split('=')
     switch (key) {
       case FILTER_URL_PARAM_NAME: {
-        const filter = parseFilter(value)
+        const filter = parseFilter(rawValue)
         if (filter.length === 3 && filter[2].length) {
           filters.push(filter)
         }
         break
       }
       case LABEL_URL_PARAM_NAME: {
-        const [labelKey, labelValue] = parseLabelsEntry(value)
+        const [labelKey, labelValue] = parseLabelsEntry(rawValue)
         if (labelKey.length && labelValue.length) {
           labels[labelKey] = labelValue
         }
         break
       }
       default: {
-        const parsedValue = parseSimpleSearchEntry(value)
+        const parsedValue = parseSimpleSearchEntry(rawValue)
         if (parsedValue !== null) {
-          searchRecord[key] = parsedValue
+          searchRecord[decodeURIComponent(key)] = parsedValue
         }
       }
     }
-  })
+  }
+
   return {
     ...searchRecord,
     ...(filters.length && { filters }),
@@ -79,14 +83,13 @@ export function serializeLabelsEntry([labelKey, labelValue]: [string, string]) {
 }
 
 /**
- * Parses the output of @see serializeLabelsEntry back to labels object entry,
- * once it has gone through URL decoding via new URLSearchParams(location.search).
+ * Parses the output of @see serializeLabelsEntry back to labels object entry.
  */
 export function parseLabelsEntry(
   labelKeyValueString: string
 ): [string, string] {
   const [key, value] = labelKeyValueString.split(',')
-  return [key, value]
+  return [decodeURIComponent(key), decodeURIComponent(value)]
 }
 
 /**
@@ -96,8 +99,8 @@ export function parseLabelsEntry(
  */
 export function serializeFilter([operator, dimension, clauses]: Filter) {
   const serializedFilter = [
-    operator,
-    dimension,
+    encodeURIComponentPermissive(operator, ':/'),
+    encodeURIComponentPermissive(dimension, ':/'),
     ...clauses.map((clause) =>
       encodeURIComponentPermissive(clause.toString(), ':/')
     )
@@ -106,17 +109,23 @@ export function serializeFilter([operator, dimension, clauses]: Filter) {
 }
 
 /**
- * Parses the output of @see serializeFilter back to filters array item,
- * once it has gone through URL decoding via new URLSearchParams(location.search).
+ * Parses the output of @see serializeFilter back to filters array item.
  */
 export function parseFilter(filterString: string): Filter {
   const [operator, dimension, ...unparsedClauses] = filterString.split(',')
-  return [operator, dimension, unparsedClauses]
+  return [
+    decodeURIComponent(operator),
+    decodeURIComponent(dimension),
+    unparsedClauses.map(decodeURIComponent)
+  ]
 }
 
 /**
  * Encodes for URL simple search param values.
+ * Encodes numbers and number-like strings as indistinguishable strings. Parse treats them as strings.
+ * Encodes booleans and strings "true" and "false" as indistinguishable strings. Parse treats these as booleans.
  * Unifies unhandleable complex search entries like undefined, null, objects and arrays as undefined.
+ * Complex URL params must be handled separately.
  */
 export function serializeSimpleSearchEntry([key, value]: [string, unknown]): [
   string,
@@ -132,8 +141,7 @@ export function serializeSimpleSearchEntry([key, value]: [string, unknown]): [
 }
 
 /**
- * Parses output of @see serializeSimpleSearchEntry,
- * once it has gone through URL decoding via new URLSearchParams(location.search)
+ * Parses output of @see serializeSimpleSearchEntry.
  */
 export function parseSimpleSearchEntry(
   searchParamValue: string
@@ -144,7 +152,7 @@ export function parseSimpleSearchEntry(
   if (searchParamValue === 'false') {
     return false
   }
-  return searchParamValue
+  return decodeURIComponent(searchParamValue)
 }
 
 export function encodeURIComponentPermissive(