Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bugfix] Fix encoding #95

Merged
merged 10 commits into from
Nov 7, 2021
Merged

[Bugfix] Fix encoding #95

merged 10 commits into from
Nov 7, 2021

Conversation

pkgacek
Copy link
Contributor

@pkgacek pkgacek commented Nov 4, 2021

Version 1.3.0 enabled encoding in RequestController - it was used to fix encoding for both server side and client side SDKs. This introduced issue, as we did not notice that ClientSide get methods were explicitly encoding query params (toQueryParams function). That meant the query params were encoded twice and that lead to issues with characters such as %.

@pkgacek pkgacek requested a review from darekg11 November 4, 2021 15:44
@alex-all3dp
Copy link

I suppose this fixes #94 ? :)

@pkgacek
Copy link
Contributor Author

pkgacek commented Nov 5, 2021

I suppose this fixes #94 ? :)

Hey, I'll inform You when the issue is resolved 🚀🚀

@@ -50,7 +50,7 @@ export function assert(condition: any, message?: string): asserts condition {
throw new Error(message)
}

export function toQueryParams(obj: Record<string, unknown>): Record<string, string> {
export function toQueryParams(obj: Record<string, unknown> | any): Record<string, string> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change really necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore.

Copy link
Contributor

@darekg11 darekg11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, please remember to release both SDK and React-widget

@darekg11
Copy link
Contributor

darekg11 commented Nov 7, 2021

@alex-all3dp Yes, this is meant to fix encoding query params in Client Side SDK.
In version 1.3.0 we have enabled encoding in RequestController to fix encoding for both server side and client side SDKs.
Unfortunately we did not notice that Client Side SDK was explicitly encoding query params, because of that for Client Side SDK query params were encoded twice and that lead to issues with characters such as %

@pkgacek pkgacek merged commit 8ff9b8d into main Nov 7, 2021
@github-actions github-actions bot mentioned this pull request Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants