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

Fix encoding of + in parameters for GET urls #1653

Merged
merged 4 commits into from
Mar 2, 2021

Conversation

AnthonyMDev
Copy link
Contributor

Fixes #1652

Copy link
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

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

Tests look reasonable but I think mayyybe there's a different solution here?

@@ -43,6 +43,8 @@ public struct GraphQLGETTransformer {
}

components.queryItems = queryItems
components.percentEncodedQuery =
components.percentEncodedQuery?.replacingOccurrences(of: "+", with: "%2B")
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this needs to actually be replaced in the query items rather than in the percent encoded query for it to essentially get re-percent-encoded so it's not read by the server as a raw string.

@designatednerd designatednerd force-pushed the fix-plus-sign-url-encoding branch from 0fe7c9d to e60e793 Compare February 22, 2021 19:59
@designatednerd
Copy link
Contributor

Ran this through a couple different URL decoders and discovered that @AnthonyMDev's fix is the correct one - my fix would get decoded as %2B, which makes sense since it's double-encoded.

@designatednerd
Copy link
Contributor

Since this is passing URL decoders, I'm going to go ahead and merge this. I'll re-open #1652 if merging this auto-closes since we're still waiting for a bit of feedback on that.

@designatednerd designatednerd merged commit 0d0064c into main Mar 2, 2021
@designatednerd designatednerd added this to the Next Release milestone Mar 2, 2021
@designatednerd designatednerd deleted the fix-plus-sign-url-encoding branch March 3, 2021 20:33
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.

iOS client not properly escaping "+" sign when using GET for queries
2 participants