-
Notifications
You must be signed in to change notification settings - Fork 231
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
Ensure URL encoding of + as %2B Authorize URL [SDK-691] #259
Conversation
@@ -181,6 +181,7 @@ class SafariWebAuth: WebAuth { | |||
|
|||
entries.forEach { items.append(URLQueryItem(name: $0, value: $1)) } | |||
components.queryItems = self.telemetry.queryItemsWithTelemetry(queryItems: items) | |||
components.percentEncodedQuery = components.percentEncodedQuery?.replacingOccurrences(of: "+", with: "%2B") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look at encoding at the key value. However, then the URL will end up double encoded. So this also appears to be the okay by apple http://www.openradar.me/24076063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh. Thanks for the link
let url = newWebAuth() | ||
.parameters(["login_hint": "first+last@host.com"]) | ||
.buildAuthorizeURL(withRedirectURL: RedirectURL, defaults: defaults, state: "state") | ||
expect(url.absoluteString.contains("first%2Blast@host.com")).to(beTrue()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ is valid in query
Changes
Please describe both what is changing and why this is important. Include:
URL Encoding has changed to force encoding of
+
to%2B
so+
can be usedas expected for the
login_hint
to pre-populate the ULP form.References
Internally raised issue.
Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.
Checklist