Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

Use CGI::escape rather than URI.escape for query params #16

Merged
merged 1 commit into from Jun 22, 2015

Conversation

sayhiben
Copy link
Contributor

Explanation:

While SSO worked fine for "normal" email addresses, my usual workflow
involves using the + sign to generate reusable email addresses for
manual testing purposes (e.g., "myemail+0001@gmail.com",
"myemail+0002@gmail.com", etc.).

Because the + symbol represents a space character (" ") within query
parameters, Ambassador's SSO is unable to associate these email
addresses with their respective accounts.

This means users whose email addresses contain the + sign cannot log
into their Ambassador portals via SSO. Ambassador otherwise appears to
support email addresses containing the symbol.

CGI::escape properly converts email addresses to their url encoded form,
but URI.escape leaves them as-is:

CGI::escape('abc+123@gmail.com') => "abc%2B123%40gmail.com"
URI.escape('abc+123@gmail.com') => "abc+123@gmail.com"

See these "fun" links:

Explanation:

While SSO worked fine for "normal" email addresses, my usual workflow
involves using the `+` sign to generate reusable email addresses for
manual testing purposes (e.g., "myemail+0001@gmail.com",
"myemail+0002@gmail.com", etc.).

Because the `+` symbol represents a space character (" ") within query
parameters, Ambassador's SSO is unable to associate these email
addresses with their accounts.

This means users whose email addresses contain the `+` sign cannot log
into their Ambassador portals via SSO. Ambassador otherwise appears to
support email addresses containing the symbol.

CGI::escape properly converts email addresses to their url encoded form,
but URI.escape leaves them as-is:

`CGI::escape('abc+123@gmail.com') => "abc%2B123%40gmail.com"`
`URI.escape('abc+123@gmail.com') => "abc+123@gmail.com"`

See these "fun" links:
- http://stackoverflow.com/a/13059657/983049
- http://stackoverflow.com/questions/1005676/urls-and-plus-signs
@chaselee
Copy link
Contributor

Thanks! Looks good. I'll have a new version up shortly.

chaselee pushed a commit that referenced this pull request Jun 22, 2015
Use CGI::escape rather than URI.escape for query params
@chaselee chaselee merged commit 15959b6 into GetAmbassador:master Jun 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants