paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 35 Packages Projects Releases Wiki Activity

Merge pull request #21719 from backstage/catalog-client-double-encoding

Browse Source 
fix(catalog-client): stop double encoding query params
This commit is contained in:
Fredrik Adelöw
2023-12-04 12:32:05 +01:00
committed by GitHub
parent 17d2c90a95 28361b6c4d
commit a8b6e2ed5a
3 changed files with 109 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/strange-brooms-poke.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/catalog-client': patch
---
Fixes a bug where some query parameters were double URL encoded.
packages/catalog-client/src/CatalogClient.test.ts
+97 -17
View File
@@ -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 () => {
packages/catalog-client/src/CatalogClient.ts
+7 -13
View File
@@ -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}`);
}
}
}