Coalesce toQueryString and query-string-encoding

Closes FOO-3485
flag=none

There were two separate shared utils for encoding a JavaScript
object into a URL query string; this combines them both into
a single place. There are now two ways to encode; they have
slightly different type signatures (array of objects vs a single
object) but one just calls the other after some massaging.

We will prefer toQueryString() in new code as it does a much
better job of matching the "PHP-style" query string encoding
that our Rails code seems to prefer; there's a wrapper method
for the legacy encodeQueryString one.

Tests were also coalesced and converted to Jest, and more test
cases were added for the kind of tricky legacy encodeQueryString
method.

Test plan:
* automated tests pass

Change-Id: Ib6f0246b3f438646967b233b72a2632ae2de9159
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/316612
Reviewed-by: Yona Appletree <yona.appletree@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Charley Kline <ckline@instructure.com>
Product-Review: Charley Kline <ckline@instructure.com>
This commit is contained in:
Charley Kline 2023-04-20 18:26:39 -05:00
parent ec926be9a8
commit e6c2fb3a75
6 changed files with 197 additions and 195 deletions

View File

@ -1,47 +0,0 @@
/*
* Copyright (C) 2017 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import {encodeQueryString, decodeQueryString} from '@canvas/query-string-encoding'
QUnit.module('Query String util')
QUnit.module('encodeQueryString')
test('encodes a query string from array of params', () => {
const params = [{foo: 'bar'}, {hello: 'world'}]
const query = encodeQueryString(params)
equal(query, 'foo=bar&hello=world')
})
test('encodes a query string from array of params with duplicate keys', () => {
const params = [{'foo[]': 'bar'}, {'foo[]': 'world'}]
const query = encodeQueryString(params)
equal(query, 'foo[]=bar&foo[]=world')
})
QUnit.module('decodeQueryString')
test('decodes a query string into an array of params', () => {
const params = decodeQueryString('foo=bar&hello=world')
deepEqual(params, [{foo: 'bar'}, {hello: 'world'}])
})
test('decodes a query string with duplicate keys into an array of params', () => {
const params = decodeQueryString('foo[]=bar&foo[]=world')
deepEqual(params, [{'foo[]': 'bar'}, {'foo[]': 'world'}])
})

View File

@ -19,10 +19,16 @@
import getCookie from '@instructure/get-cookie'
import parseLinkHeader from 'parse-link-header'
import {defaultFetchOptions} from '@instructure/js-utils'
import toQueryString from '@canvas/util/toQueryString'
import type {QueryParameterMap} from '@canvas/util/toQueryString'
import {toQueryString} from '@canvas/query-string-encoding'
import type {QueryParameterRecord} from '@canvas/query-string-encoding'
function constructRelativeUrl({path, params}: {path: string; params: QueryParameterMap}): string {
function constructRelativeUrl({
path,
params,
}: {
path: string
params: QueryParameterRecord
}): string {
const queryString = toQueryString(params)
if (queryString.length === 0) return path
return path + '?' + queryString
@ -37,7 +43,7 @@ export type DoFetchApiOpts = {
path: string
method?: string
headers?: {[k: string]: string}
params?: QueryParameterMap
params?: QueryParameterRecord
// eslint-disable-next-line no-undef
body?: BodyInit
fetchOpts?: RequestInit

View File

@ -0,0 +1,99 @@
/*
* Copyright (C) 2023 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import {toQueryString, encodeQueryString, decodeQueryString} from '../index'
import type {QueryParameterRecord} from '../index'
import $ from 'jquery'
type EncodeQueryStringParams = Array<Record<string, string | null>>
const func = () => 'result'
const func2 = () => 'yet another result'
const EX: {[k: string]: QueryParameterRecord} = {
empty: {},
scalars_with_numbers_strings_and_bools: {frd: 1, str: 'syzygy', butNo: false},
scalars_with_null: {frd: 1, butNo: null},
scalars_with_undefined: {frd: 1, butNo: undefined, last: 'zzz'},
scalars_with_functions: {frd: 1, func, func2},
simple_array: {blah: [1, 2, 3]},
mixed_types_array: {blah: [1, 'b', 'III']},
recursive_object: {blah: 1, sub: {one: 1, two: [2, 4]}, last: 'zzz'},
array_of_objects: {
blah: [
{one: 1, two: 2},
{one: '1', two: '2'},
],
},
object_with_arrays: {blah: {one: [1, 2, 3], two: [2, 4, 6]}},
}
describe('toQueryString::', () => {
describe('does everything jQuery does', () => {
for (const ex in EX) {
it(`handles example ${ex}`, () => {
const subject = EX[ex]
const testResult = toQueryString(subject)
const jQueryResult = $.param(subject)
expect(testResult).toBe(jQueryResult)
})
}
})
})
describe('encodeQueryString::', () => {
it('encodes a query string from array of params', () => {
const params: EncodeQueryStringParams = [{foo: 'bar'}, {hello: 'world'}]
const query = encodeQueryString(params)
expect(query).toBe('foo=bar&hello=world')
})
it('encodes a query string from array of params with duplicate keys', () => {
const params: EncodeQueryStringParams = [{'foo[]': 'bar'}, {'foo[]': 'world'}]
const query = encodeQueryString(params)
expect(query).toBe('foo%5B%5D=bar&foo%5B%5D=world')
})
it('wont pass along any null values for some reason', () => {
const params: EncodeQueryStringParams = [{foo: '256'}, {hello: null}, {baz: '512'}]
const query = encodeQueryString(params)
expect(query).toBe('foo=256&baz=512')
})
it('throws TypeError if we try to turn a non-array param into an array param', () => {
const params: EncodeQueryStringParams = [{foo: 'bar'}, {'foo[]': 'world'}]
expect(() => encodeQueryString(params)).toThrow(TypeError)
})
it('throws TypeError if we try to turn an array param into a non-array param', () => {
const params: EncodeQueryStringParams = [{'foo[]': 'bar'}, {foo: 'world'}]
expect(() => encodeQueryString(params)).toThrow(TypeError)
})
})
describe('decodeQueryString::', () => {
it('decodes a query string into an array of params', () => {
const params = decodeQueryString('foo=bar&hello=world')
expect(params).toMatchObject([{foo: 'bar'}, {hello: 'world'}])
})
it('decodes a query string with duplicate keys into an array of params', () => {
const params = decodeQueryString('foo[]=bar&foo[]=world')
expect(params).toMatchObject([{'foo[]': 'bar'}, {'foo[]': 'world'}])
})
})

View File

@ -16,17 +16,96 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
export function encodeQueryString(params: Array<{[key: string]: string | null}>) {
return params
.map(param => {
const key = Object.keys(param)[0]
const value = param[key]
return value !== null ? `${key}=${value}` : null
})
.filter(Boolean)
.join('&')
// toQueryString does what jQuery's .param() does, only without having to bring in
// all of jQuery.
// URLSearchParams turns arrays into queries like a=1,2,3
// But our jQuery makes the older "PHP" style like a[]=1&a[]=2&a[]=3
// JQuery also magically deals with parameters that are functions, and recursive
// objects within objects, {a: 1, b: {c: 2, d: 3}, e: 4} => a=1&b[c]=2&b[d]=3&e=4
// ... so we have to do a lot of massaging with the params object before using it
// So fun!
type QueryParameterElement =
| string
| number
| boolean
| null
| undefined
| (() => string) // n.b. for jQuery compatibility, this does not expect a generic return
| Array<QueryParameterElement>
| QueryParameterRecord
export type QueryParameterRecord = {[k: string]: QueryParameterElement}
export function toQueryString(params: QueryParameterRecord): string {
const paramsWithIndexes: Array<[k: string, v: string]> = []
// fix up the array/object indexes to match the PHP standard
const fixIndexes = (elt: [string, string]): [string, string] => [
elt[0]
.replace(/\[\d+\]$/, '[]')
.replace(/{/g, '[')
.replace(/}/g, ']'),
elt[1],
]
function serialize(k: string, elt: QueryParameterElement, suffix: string): void {
if (elt instanceof Function) {
paramsWithIndexes.push([k + suffix, elt()])
} else if (elt instanceof Array) {
elt.forEach((k2, i) => serialize(k, k2, `${suffix}[${i}]`))
} else if (elt instanceof Object) {
Object.keys(elt).forEach(k2 => serialize(k, elt[k2], `${suffix}{${k2}}`))
} else if (typeof elt === 'boolean' || typeof elt === 'number') {
paramsWithIndexes.push([k + suffix, elt.toString()])
} else if (typeof elt === 'undefined') {
paramsWithIndexes.push([k + suffix, 'undefined'])
} else if (elt === null) {
paramsWithIndexes.push([k + suffix, 'null'])
} else if (typeof elt === 'string') {
paramsWithIndexes.push([k + suffix, elt])
}
}
Object.keys(params).forEach(k => serialize(k, params[k], ''))
return new URLSearchParams(paramsWithIndexes.map(fixIndexes)).toString()
}
// This is just to implement backward-compatibility from the old package. Almost
// nothing uses it anyway and we recommend toQueryString() for new needs.
// Need to be careful about duplicated keys, which have to be mapped into arrays.
export function encodeQueryString(params: Array<Record<string, string | null>>): string {
const realParms: QueryParameterRecord = {}
params.forEach(p => {
const k = Object.keys(p)[0]
const m = k.match(/(.+)\[\]/)
if (p[k] === null) return
if (m === null) {
// Not building up an array, just jam this in there, unless it's already
// in there and IS already an array.
if (Array.isArray(realParms[k])) throw new TypeError('cannot mix scalar and array parameters')
Object.assign(realParms, p)
} else {
// We ARE building up an array... append this value to an existing array
// if it exists, otherwise create one. If an existing key already exists
// and is NOT an array, then this is an error
const idx = m[1]
let arrayParm = realParms[idx]
if (typeof arrayParm === 'undefined') arrayParm = []
if (!Array.isArray(arrayParm)) throw new TypeError('cannot mix scalar and array parameters')
arrayParm.push(p[k])
realParms[idx] = arrayParm
}
})
return toQueryString(realParms)
}
// TODO: should be an inverse of toQueryString that takes a string and returns a
// QueryParameterRecord. The result would be slightly different from decodeQueryString
// in terms of how things like foo[]=1&foo[]=2 would be decoded. But it doesn't
// seem like any current usage cares about that kind of thing.
export function decodeQueryString(string: string) {
return string
.split('&')

View File

@ -1,55 +0,0 @@
/*
* Copyright (C) 2022 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import toQueryString from '../toQueryString'
import type {QueryParameterMap} from '../toQueryString'
import $ from 'jquery'
const func = () => 'result'
const func2 = () => 'yet another result'
const EX: {[k: string]: QueryParameterMap} = {
empty: {},
scalars_with_numbers_strings_and_bools: {frd: 1, str: 'syzygy', butNo: false},
scalars_with_null: {frd: 1, butNo: null},
scalars_with_undefined: {frd: 1, butNo: undefined, last: 'zzz'},
scalars_with_functions: {frd: 1, func, func2},
simple_array: {blah: [1, 2, 3]},
mixed_types_array: {blah: [1, 'b', 'III']},
recursive_object: {blah: 1, sub: {one: 1, two: [2, 4]}, last: 'zzz'},
array_of_objects: {
blah: [
{one: 1, two: 2},
{one: '1', two: '2'},
],
},
object_with_arrays: {blah: {one: [1, 2, 3], two: [2, 4, 6]}},
}
describe('toQueryString::', () => {
describe('does everything jQuery does', () => {
for (const ex in EX) {
it(`handles example ${ex}`, () => {
const subject = EX[ex]
const testResult = toQueryString(subject)
const jQueryResult = $.param(subject)
expect(testResult).toBe(jQueryResult)
})
}
})
})

View File

@ -1,80 +0,0 @@
/*
* Copyright (C) 2022 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
// This does what jQuery's .param() does, only without having to bring in all
// of jQuery.
// URLSearchParams turns arrays into queries like a=1,2,3
// But our jQuery makes the older "PHP" style like a[]=1&a[]=2&a[]=3
// JQuery also magically deals with parameters that are functions, and recursive
// objects within objects, {a: 1, b: {c: 2, d: 3}, e: 4} => a=1&b[c]=2&b[d]=3&e=4
// ... so we have to do a lot of massaging with the params object before using it
// So fun!
type QueryParameterElement =
| string
| number
| boolean
| null
| undefined
| (() => string) // n.b. for jQuery compatibility, this does not expect a generic return
| Array<QueryParameterElement>
| QueryParameterMap
export type QueryParameterMap = {[k: string]: QueryParameterElement}
export default function toQueryString(params: QueryParameterMap): string {
const paramsWithIndexes: Array<[k: string, v: string]> = []
// fix up the array/object indexes to match the PHP standard
function fixIndexes(elt: [string, string]): [string, string] {
return [
elt[0]
.replace(/\[\d+\]$/, '[]')
.replace(/{/g, '[')
.replace(/}/g, ']'),
elt[1],
]
}
function serialize(k: string, elt: QueryParameterElement, suffix: string): void {
if (elt instanceof Function) {
paramsWithIndexes.push([k + suffix, elt()])
} else if (elt instanceof Array) {
for (const k2 in elt) {
serialize(k, elt[k2], `${suffix}[${k2}]`)
}
} else if (elt instanceof Object) {
for (const k2 in elt) {
serialize(k, elt[k2], `${suffix}{${k2}}`)
}
} else if (typeof elt === 'boolean' || typeof elt === 'number') {
paramsWithIndexes.push([k + suffix, elt.toString()])
} else if (typeof elt === 'undefined') {
paramsWithIndexes.push([k + suffix, 'undefined'])
} else if (elt === null) {
paramsWithIndexes.push([k + suffix, 'null'])
} else if (typeof elt === 'string') {
paramsWithIndexes.push([k + suffix, elt])
}
}
for (const k in params) serialize(k, params[k], '')
return new URLSearchParams(paramsWithIndexes.map(fixIndexes)).toString()
}