Skip to content

Commit

Permalink
Refactor not to use URLSearchParams, because it led to double decoding
Browse files Browse the repository at this point in the history
  • Loading branch information
apata committed Dec 11, 2024
1 parent caf00fe commit 6f83914
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 24 deletions.
16 changes: 10 additions & 6 deletions assets/js/dashboard/util/url-search-params.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -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)
}
)
})
Expand Down
44 changes: 26 additions & 18 deletions assets/js/dashboard/util/url-search-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
Expand All @@ -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)]
}

/**
Expand All @@ -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(), ':/')
)
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -144,7 +152,7 @@ export function parseSimpleSearchEntry(
if (searchParamValue === 'false') {
return false
}
return searchParamValue
return decodeURIComponent(searchParamValue)
}

export function encodeURIComponentPermissive(
Expand Down

0 comments on commit 6f83914

Please sign in to comment.