diff --git a/.changeset/strange-brooms-poke.md b/.changeset/strange-brooms-poke.md new file mode 100644 index 0000000000..35b7d48360 --- /dev/null +++ b/.changeset/strange-brooms-poke.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-client': patch +--- + +Fixes a bug where some query parameters were double URL encoded. diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 4892017032..b8212fde96 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -86,9 +86,12 @@ 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', - ); + 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([])); }), ); @@ -120,9 +123,8 @@ 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', - ); + const queryParams = new URLSearchParams(req.url.search); + expect(queryParams.getAll('filter')).toEqual(['a=1,b=2,b=3,ö==,c']); return res(ctx.json([])); }), ); @@ -142,12 +144,45 @@ 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); + + 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(2); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(decodeURIComponent(req.url.search)).toBe('?fields=a.b,%C3%B6'); + const queryParams = new URLSearchParams(req.url.search); + expect(queryParams.getAll('fields')).toEqual(['a.b,ö']); return res(ctx.json([])); }), ); @@ -205,9 +240,11 @@ describe('CatalogClient', () => { server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - 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([])); }), ); @@ -328,9 +365,46 @@ describe('CatalogClient', () => { expect(response).toEqual({ items: [], totalItems: 0 }); 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'); + + const queryParams = new URLSearchParams( + mockedEndpoint.mock.calls[0][0].url.search, + ); + 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 () => { + 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); + 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('should send query params correctly on initial request', async () => { @@ -353,11 +427,17 @@ describe('CatalogClient', () => { { field: 'metadata.uid', order: 'desc' }, ], }); - 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', + + 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 () => { diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 511bf45c51..f97360d1d7 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, 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; } } @@ -452,11 +448,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}`); } } }