Skip to content

Commit

Permalink
fix: Regression with encoding parameters (#801)
Browse files Browse the repository at this point in the history
  • Loading branch information
shilgapira authored Sep 15, 2024
1 parent f57819e commit 92105aa
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
7 changes: 3 additions & 4 deletions packages/sdks/core-js-sdk/src/httpClient/urlBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ export const urlBuilder = ({

// add query params if given
if (queryParams) {
url = `${url}?`;
const keys = Object.keys(queryParams);
keys.forEach((key: string, index: number) => {
url = `${url}${key}=${queryParams[key]}${
index === keys.length - 1 ? '' : '&'
}`;
url = `${url}${index === 0 ? '?' : ''}${key}=${encodeURIComponent(
queryParams[key],
)}${index === keys.length - 1 ? '' : '&'}`;
});
}

Expand Down
26 changes: 24 additions & 2 deletions packages/sdks/core-js-sdk/test/httpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,36 @@ describe('httpClient', () => {
);
});

it('should call fetch without ? when calling "get" without params', () => {
httpClient.get('1/2/3', {
queryParams: {},
});

expect(mockFetch).toHaveBeenCalledWith(`http://descope.com/1/2/3`, {
body: undefined,
credentials: 'include',
headers: new Headers({
test: '123',
Authorization: 'Bearer 456',
...descopeHeaders,
}),
method: 'GET',
});
});

it('should call fetch with multiple params when calling "get"', () => {
httpClient.get('1/2/3', {
headers: { test2: '123' },
queryParams: { test2: '123', test3: '456', test4: '789' },
queryParams: {
test2: '123',
test3: '456',
test4: '789',
test5: `don't+forget+to@escape.urls`,
},
});

expect(mockFetch).toHaveBeenCalledWith(
'http://descope.com/1/2/3?test2=123&test3=456&test4=789',
`http://descope.com/1/2/3?test2=123&test3=456&test4=789&test5=don't%2Bforget%2Bto%40escape.urls`,
{
body: undefined,
credentials: 'include',
Expand Down

0 comments on commit 92105aa

Please sign in to comment.