From 3a9fbdbcd6b819fb32e5bbe6c40227f2840d6e44 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 11:43:40 -0500 Subject: [PATCH 1/9] fix: update the catalog client to prevent double encoding of filter query params Signed-off-by: Aramis Sennyey --- packages/catalog-client/src/CatalogClient.test.ts | 6 +++--- packages/catalog-client/src/CatalogClient.ts | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 4892017032..745823471a 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -87,7 +87,7 @@ describe('CatalogClient', () => { server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { expect(decodeURIComponent(req.url.search)).toBe( - '?filter=a=1,b=2,b=3,%C3%B6=%3D&filter=a=2&filter=c', + '?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c', ); return res(ctx.json([])); }), @@ -121,7 +121,7 @@ describe('CatalogClient', () => { server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { expect(decodeURIComponent(req.url.search)).toBe( - '?filter=a=1,b=2,b=3,%C3%B6=%3D,c', + '?filter=a=1,b=2,b=3,ö==,c', ); return res(ctx.json([])); }), @@ -330,7 +330,7 @@ describe('CatalogClient', () => { expect(mockedEndpoint).toHaveBeenCalledTimes(1); expect( decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), - ).toBe('?filter=a=1,b=2,b=3,%C3%B6=%3D&filter=a=2&filter=c'); + ).toBe('?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c'); }); it('should send query params correctly on initial request', async () => { diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 511bf45c51..50219a0aa6 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -452,11 +452,9 @@ export class CatalogClient implements CatalogApi { for (const [key, value] of Object.entries(filterItem)) { for (const v of [value].flat()) { if (v === CATALOG_FILTER_EXISTS) { - filterParts.push(encodeURIComponent(key)); + filterParts.push(key); } else if (typeof v === 'string') { - filterParts.push( - `${encodeURIComponent(key)}=${encodeURIComponent(v)}`, - ); + filterParts.push(`${key}=${v}`); } } } From 82fa88be3b7edbc19e72eb1c8fad7ea98d113f9c Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 11:47:06 -0500 Subject: [PATCH 2/9] add changeset Signed-off-by: Aramis Sennyey --- .changeset/strange-brooms-poke.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/strange-brooms-poke.md diff --git a/.changeset/strange-brooms-poke.md b/.changeset/strange-brooms-poke.md new file mode 100644 index 0000000000..0c0e4d69a1 --- /dev/null +++ b/.changeset/strange-brooms-poke.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-client': patch +--- + +Fixes a bug where `filter` query parameters were double URL encoded. From 621afaab10e1429263815261e38f89ac3ee20309 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 13:55:06 -0500 Subject: [PATCH 3/9] add a new test for url unsafe values Signed-off-by: Aramis Sennyey --- .../catalog-client/src/CatalogClient.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 745823471a..1ade3f7170 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -333,6 +333,47 @@ describe('CatalogClient', () => { ).toBe('?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c'); }); + it('builds search filters property even those with URL unsafe values', async () => { + const mockedEndpoint = jest + .fn() + .mockImplementation((_req, res, ctx) => + res(ctx.json({ items: [], totalItems: 0 })), + ); + + server.use(rest.get(`${mockBaseUrl}/entities/by-query`, mockedEndpoint)); + + const response = await client.queryEntities( + { + filter: [ + { + 'metadata.annotation?test': 'backstage.io/test&test123?test=123', + a: '2', + b: ['1, 2', '3'], + }, + { + 'my-name test': 't%^url*encoded', + 'my-name test2': ['t%^url*encoded2', 'url'], + }, + { + c: CATALOG_FILTER_EXISTS, + }, + ], + }, + { token }, + ); + + expect(response).toEqual({ items: [], totalItems: 0 }); + expect(mockedEndpoint).toHaveBeenCalledTimes(1); + expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( + '?filter=metadata.annotation%3Ftest%3Dbackstage.io%2Ftest%26test123%3Ftest%3D123%2Ca%3D2%2Cb%3D1%2C%202%2Cb%3D3&filter=my-name%20test%3Dt%25%5Eurl%2Aencoded%2Cmy-name%20test2%3Dt%25%5Eurl%2Aencoded2%2Cmy-name%20test2%3Durl&filter=c', + ); + expect( + decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), + ).toBe( + '?filter=metadata.annotation?test=backstage.io/test&test123?test=123,a=2,b=1, 2,b=3&filter=my-name test=t%^url*encoded,my-name test2=t%^url*encoded2,my-name test2=url&filter=c', + ); + }); + it('should send query params correctly on initial request', async () => { const mockedEndpoint = jest .fn() From f3e44b73e0317d55cd924b09a60ddb65877f0e02 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 14:00:31 -0500 Subject: [PATCH 4/9] make the test case a little smaller Signed-off-by: Aramis Sennyey --- .../catalog-client/src/CatalogClient.test.ts | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 1ade3f7170..f8bb6660c7 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -346,16 +346,8 @@ describe('CatalogClient', () => { { filter: [ { - 'metadata.annotation?test': 'backstage.io/test&test123?test=123', - a: '2', - b: ['1, 2', '3'], - }, - { - 'my-name test': 't%^url*encoded', - 'my-name test2': ['t%^url*encoded2', 'url'], - }, - { - c: CATALOG_FILTER_EXISTS, + '!@#$%': 't?i=1&a:2', + '^&*(){}[]': ['t%^url*encoded2', 'url'], }, ], }, @@ -364,14 +356,15 @@ describe('CatalogClient', () => { expect(response).toEqual({ items: [], totalItems: 0 }); expect(mockedEndpoint).toHaveBeenCalledTimes(1); + // Validate that the URL is _actually_ encoded. expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( - '?filter=metadata.annotation%3Ftest%3Dbackstage.io%2Ftest%26test123%3Ftest%3D123%2Ca%3D2%2Cb%3D1%2C%202%2Cb%3D3&filter=my-name%20test%3Dt%25%5Eurl%2Aencoded%2Cmy-name%20test2%3Dt%25%5Eurl%2Aencoded2%2Cmy-name%20test2%3Durl&filter=c', + '?filter=%21%40%23%24%25%3Dt%3Fi%3D1%26a%3A2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Dt%25%5Eurl%2Aencoded2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Durl', ); + + // Validate that the URL matches the expected decoded value. expect( decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), - ).toBe( - '?filter=metadata.annotation?test=backstage.io/test&test123?test=123,a=2,b=1, 2,b=3&filter=my-name test=t%^url*encoded,my-name test2=t%^url*encoded2,my-name test2=url&filter=c', - ); + ).toBe('?filter=!@#$%=t?i=1&a:2,^&*(){}[]=t%^url*encoded2,^&*(){}[]=url'); }); it('should send query params correctly on initial request', async () => { From e7c59fcbae5031b7e4d241dcfb90ec37ed72088f Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 14:21:17 -0500 Subject: [PATCH 5/9] Fix URI encoding for other parameters as well and adjust tests. Signed-off-by: Aramis Sennyey --- .../catalog-client/src/CatalogClient.test.ts | 45 +++++++++++++++++-- packages/catalog-client/src/CatalogClient.ts | 14 +++--- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index f8bb6660c7..5cfb563d9f 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -142,12 +142,47 @@ describe('CatalogClient', () => { expect(response.items).toEqual([]); }); + it('builds search filters property even those with URL unsafe values', async () => { + const mockedEndpoint = jest + .fn() + .mockImplementation((_req, res, ctx) => + res(ctx.json({ items: [], totalItems: 0 })), + ); + + server.use(rest.get(`${mockBaseUrl}/entities/by-query`, mockedEndpoint)); + + const response = await client.queryEntities( + { + filter: [ + { + '!@#$%': 't?i=1&a:2', + '^&*(){}[]': ['t%^url*encoded2', 'url'], + }, + ], + }, + { token }, + ); + + expect(response).toEqual({ items: [], totalItems: 0 }); + expect(mockedEndpoint).toHaveBeenCalledTimes(1); + // Validate that the URL is _actually_ encoded. + expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( + '?filter=%21%40%23%24%25%3Dt%3Fi%3D1%26a%3A2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Dt%25%5Eurl%2Aencoded2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Durl', + ); + + // Validate that the URL matches the expected decoded value. + expect( + decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), + ).toBe('?filter=!@#$%=t?i=1&a:2,^&*(){}[]=t%^url*encoded2,^&*(){}[]=url'); + }); + it('builds entity field selectors properly', async () => { - expect.assertions(2); + expect.assertions(3); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(decodeURIComponent(req.url.search)).toBe('?fields=a.b,%C3%B6'); + expect(req.url.search).toBe('?fields=a.b,%C3%B6'); + expect(decodeURIComponent(req.url.search)).toBe('?fields=a.b,ö'); return res(ctx.json([])); }), ); @@ -387,10 +422,14 @@ describe('CatalogClient', () => { { field: 'metadata.uid', order: 'desc' }, ], }); + // Verify we actually encode the URI. + expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( + '?fields=a,b&limit=100&orderField=metadata.name%2Casc&orderField=metadata.uid%2Cdesc&fullTextFilterTerm=query', + ); expect( decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), ).toBe( - '?fields=a,b&limit=100&orderField=metadata.name%2Casc&orderField=metadata.uid%2Cdesc&fullTextFilterTerm=query', + '?fields=a,b&limit=100&orderField=metadata.name,asc&orderField=metadata.uid,desc&fullTextFilterTerm=query', ); }); diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 50219a0aa6..8409e655c0 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -107,11 +107,7 @@ export class CatalogClient implements CatalogApi { if (order) { for (const directive of [order].flat()) { if (directive) { - encodedOrder.push( - `${encodeURIComponent(directive.order)}:${encodeURIComponent( - directive.field, - )}`, - ); + encodedOrder.push(`${directive.order}:${directive.field}`); } } } @@ -120,7 +116,7 @@ export class CatalogClient implements CatalogApi { await this.apiClient.getEntities( { query: { - fields: fields.map(encodeURIComponent), + fields: fields, limit, filter: this.getFilterValue(filter), offset, @@ -209,10 +205,10 @@ export class CatalogClient implements CatalogApi { if (orderFields !== undefined) { params.orderField = ( Array.isArray(orderFields) ? orderFields : [orderFields] - ).map(({ field, order }) => encodeURIComponent(`${field},${order}`)); + ).map(({ field, order }) => `${field},${order}`); } if (fields.length) { - params.fields = fields.map(encodeURIComponent); + params.fields = fields; } const normalizedFullTextFilterTerm = fullTextFilter?.term?.trim(); @@ -230,7 +226,7 @@ export class CatalogClient implements CatalogApi { params.limit = limit; } if (fields.length) { - params.fields = fields.map(encodeURIComponent); + params.fields = fields; } } From b24742ad9f058412726119f8e18a2bb071bae5ab Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 14:27:52 -0500 Subject: [PATCH 6/9] add more checks in the tests for both URL encoding and decoding Signed-off-by: Aramis Sennyey --- .../catalog-client/src/CatalogClient.test.ts | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 5cfb563d9f..f15d695d54 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -82,10 +82,16 @@ describe('CatalogClient', () => { }); it('builds multiple entity search filters properly', async () => { - expect.assertions(2); + expect.assertions(3); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + // Validate that the URL is _actually_ encoded. + expect(req.url.search).toBe( + '?filter=a%3D1%2Cb%3D2%2Cb%3D3%2C%C3%B6%3D%3D&filter=a%3D2&filter=c', + ); + + // Validate that the decoded URL matches the expected value. expect(decodeURIComponent(req.url.search)).toBe( '?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c', ); @@ -116,10 +122,16 @@ describe('CatalogClient', () => { }); it('builds single entity search filter properly', async () => { - expect.assertions(2); + expect.assertions(3); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + // Validate that the URL is actually encoded. + expect(req.url.search).toBe( + '?filter=a%3D1%2Cb%3D2%2Cb%3D3%2C%C3%B6%3D%3D%2Cc', + ); + + // Validate that the decoded URL matches the expected value. expect(decodeURIComponent(req.url.search)).toBe( '?filter=a=1,b=2,b=3,ö==,c', ); @@ -181,7 +193,10 @@ describe('CatalogClient', () => { server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + // Validate that the URL is _actually_ encoded. expect(req.url.search).toBe('?fields=a.b,%C3%B6'); + + // Validate that the URL matches the expected decoded value. expect(decodeURIComponent(req.url.search)).toBe('?fields=a.b,ö'); return res(ctx.json([])); }), @@ -236,10 +251,16 @@ describe('CatalogClient', () => { }); it('handles ordering properly', async () => { - expect.assertions(2); + expect.assertions(3); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + // Validate that the URL is _actually_ encoded. + expect(req.url.search).toBe( + '?order=asc%3Akind&order=desc%3Ametadata.name', + ); + + // Validate that the URL matches the expected value. expect(decodeURIComponent(req.url.search)).toBe( '?order=asc:kind&order=desc:metadata.name', ); @@ -363,6 +384,13 @@ describe('CatalogClient', () => { expect(response).toEqual({ items: [], totalItems: 0 }); expect(mockedEndpoint).toHaveBeenCalledTimes(1); + + // Validate that the URL is _actually_ encoded. + expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( + '?filter=a%3D1%2Cb%3D2%2Cb%3D3%2C%C3%B6%3D%3D&filter=a%3D2&filter=c', + ); + + // Validate that the decoded URL matches the expected value. expect( decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), ).toBe('?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c'); From bd882e443e1811c71c1ca28314ef06e6c5ae68b3 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 14:29:21 -0500 Subject: [PATCH 7/9] quick syntax thing Signed-off-by: Aramis Sennyey --- packages/catalog-client/src/CatalogClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 8409e655c0..f97360d1d7 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -116,7 +116,7 @@ export class CatalogClient implements CatalogApi { await this.apiClient.getEntities( { query: { - fields: fields, + fields, limit, filter: this.getFilterValue(filter), offset, From 634f62065f290b663e310615657ac847f348b7e2 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Sat, 2 Dec 2023 14:48:59 -0500 Subject: [PATCH 8/9] use URLSearchParams instead of string comps Signed-off-by: Aramis Sennyey --- .../catalog-client/src/CatalogClient.test.ts | 115 +++++++----------- 1 file changed, 47 insertions(+), 68 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index f15d695d54..b8212fde96 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -82,19 +82,16 @@ describe('CatalogClient', () => { }); it('builds multiple entity search filters properly', async () => { - expect.assertions(3); + expect.assertions(2); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - // Validate that the URL is _actually_ encoded. - expect(req.url.search).toBe( - '?filter=a%3D1%2Cb%3D2%2Cb%3D3%2C%C3%B6%3D%3D&filter=a%3D2&filter=c', - ); - - // Validate that the decoded URL matches the expected value. - expect(decodeURIComponent(req.url.search)).toBe( - '?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c', - ); + const queryParams = new URLSearchParams(req.url.search); + expect(queryParams.getAll('filter')).toEqual([ + 'a=1,b=2,b=3,ö==', + 'a=2', + 'c', + ]); return res(ctx.json([])); }), ); @@ -122,19 +119,12 @@ describe('CatalogClient', () => { }); it('builds single entity search filter properly', async () => { - expect.assertions(3); + expect.assertions(2); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - // Validate that the URL is actually encoded. - expect(req.url.search).toBe( - '?filter=a%3D1%2Cb%3D2%2Cb%3D3%2C%C3%B6%3D%3D%2Cc', - ); - - // Validate that the decoded URL matches the expected value. - expect(decodeURIComponent(req.url.search)).toBe( - '?filter=a=1,b=2,b=3,ö==,c', - ); + const queryParams = new URLSearchParams(req.url.search); + expect(queryParams.getAll('filter')).toEqual(['a=1,b=2,b=3,ö==,c']); return res(ctx.json([])); }), ); @@ -177,27 +167,22 @@ describe('CatalogClient', () => { expect(response).toEqual({ items: [], totalItems: 0 }); expect(mockedEndpoint).toHaveBeenCalledTimes(1); - // Validate that the URL is _actually_ encoded. - expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( - '?filter=%21%40%23%24%25%3Dt%3Fi%3D1%26a%3A2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Dt%25%5Eurl%2Aencoded2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Durl', - ); - // Validate that the URL matches the expected decoded value. - expect( - decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), - ).toBe('?filter=!@#$%=t?i=1&a:2,^&*(){}[]=t%^url*encoded2,^&*(){}[]=url'); + const queryParams = new URLSearchParams( + mockedEndpoint.mock.calls[0][0].url.search, + ); + expect(queryParams.getAll('filter')).toEqual([ + '!@#$%=t?i=1&a:2,^&*(){}[]=t%^url*encoded2,^&*(){}[]=url', + ]); }); it('builds entity field selectors properly', async () => { - expect.assertions(3); + expect.assertions(2); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - // Validate that the URL is _actually_ encoded. - expect(req.url.search).toBe('?fields=a.b,%C3%B6'); - - // Validate that the URL matches the expected decoded value. - expect(decodeURIComponent(req.url.search)).toBe('?fields=a.b,ö'); + const queryParams = new URLSearchParams(req.url.search); + expect(queryParams.getAll('fields')).toEqual(['a.b,ö']); return res(ctx.json([])); }), ); @@ -251,19 +236,15 @@ describe('CatalogClient', () => { }); it('handles ordering properly', async () => { - expect.assertions(3); + expect.assertions(2); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - // Validate that the URL is _actually_ encoded. - expect(req.url.search).toBe( - '?order=asc%3Akind&order=desc%3Ametadata.name', - ); - - // Validate that the URL matches the expected value. - expect(decodeURIComponent(req.url.search)).toBe( - '?order=asc:kind&order=desc:metadata.name', - ); + const queryParams = new URLSearchParams(req.url.search); + expect(queryParams.getAll('order')).toEqual([ + 'asc:kind', + 'desc:metadata.name', + ]); return res(ctx.json([])); }), ); @@ -385,15 +366,14 @@ describe('CatalogClient', () => { expect(response).toEqual({ items: [], totalItems: 0 }); expect(mockedEndpoint).toHaveBeenCalledTimes(1); - // Validate that the URL is _actually_ encoded. - expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( - '?filter=a%3D1%2Cb%3D2%2Cb%3D3%2C%C3%B6%3D%3D&filter=a%3D2&filter=c', + const queryParams = new URLSearchParams( + mockedEndpoint.mock.calls[0][0].url.search, ); - - // Validate that the decoded URL matches the expected value. - expect( - decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), - ).toBe('?filter=a=1,b=2,b=3,ö==&filter=a=2&filter=c'); + expect(queryParams.getAll('filter')).toEqual([ + 'a=1,b=2,b=3,ö==', + 'a=2', + 'c', + ]); }); it('builds search filters property even those with URL unsafe values', async () => { @@ -419,15 +399,12 @@ describe('CatalogClient', () => { expect(response).toEqual({ items: [], totalItems: 0 }); expect(mockedEndpoint).toHaveBeenCalledTimes(1); - // Validate that the URL is _actually_ encoded. - expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( - '?filter=%21%40%23%24%25%3Dt%3Fi%3D1%26a%3A2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Dt%25%5Eurl%2Aencoded2%2C%5E%26%2A%28%29%7B%7D%5B%5D%3Durl', + const queryParams = new URLSearchParams( + mockedEndpoint.mock.calls[0][0].url.search, ); - - // Validate that the URL matches the expected decoded value. - expect( - decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), - ).toBe('?filter=!@#$%=t?i=1&a:2,^&*(){}[]=t%^url*encoded2,^&*(){}[]=url'); + expect(queryParams.getAll('filter')).toEqual([ + '!@#$%=t?i=1&a:2,^&*(){}[]=t%^url*encoded2,^&*(){}[]=url', + ]); }); it('should send query params correctly on initial request', async () => { @@ -450,15 +427,17 @@ describe('CatalogClient', () => { { field: 'metadata.uid', order: 'desc' }, ], }); - // Verify we actually encode the URI. - expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( - '?fields=a,b&limit=100&orderField=metadata.name%2Casc&orderField=metadata.uid%2Cdesc&fullTextFilterTerm=query', - ); - expect( - decodeURIComponent(mockedEndpoint.mock.calls[0][0].url.search), - ).toBe( - '?fields=a,b&limit=100&orderField=metadata.name,asc&orderField=metadata.uid,desc&fullTextFilterTerm=query', + + const queryParams = new URLSearchParams( + mockedEndpoint.mock.calls[0][0].url.search, ); + expect(queryParams.getAll('fields')).toEqual(['a,b']); + expect(queryParams.getAll('limit')).toEqual(['100']); + expect(queryParams.getAll('fullTextFilterTerm')).toEqual(['query']); + expect(queryParams.getAll('orderField')).toEqual([ + 'metadata.name,asc', + 'metadata.uid,desc', + ]); }); it('should ignore initial query params if cursor is passed', async () => { From 28361b6c4d9a1c36d7cf946c8244e0a6dbcd648d Mon Sep 17 00:00:00 2001 From: Aramis <34432188+sennyeya@users.noreply.github.com> Date: Sat, 2 Dec 2023 15:08:45 -0500 Subject: [PATCH 9/9] Update strange-brooms-poke.md Signed-off-by: Aramis <34432188+sennyeya@users.noreply.github.com> --- .changeset/strange-brooms-poke.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/strange-brooms-poke.md b/.changeset/strange-brooms-poke.md index 0c0e4d69a1..35b7d48360 100644 --- a/.changeset/strange-brooms-poke.md +++ b/.changeset/strange-brooms-poke.md @@ -2,4 +2,4 @@ '@backstage/catalog-client': patch --- -Fixes a bug where `filter` query parameters were double URL encoded. +Fixes a bug where some query parameters were double URL encoded.