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

Addressable::URI.encode not working as expected when passed an Addressable::URI #171

Open
justinmiller61 opened this issue Oct 17, 2014 · 1 comment
Labels

Comments

@justinmiller61
Copy link

If I'm reading RFC3986, correctly (and I may not be), it says that any data characters falling outside the allowed set for a component, must be percent encoded.

So, I understand why passing this string to encode would not encode as expected:

Addressable::URI.encode("http://example.com/to_be_or_not_to_be?.txt")

encode correctly sees the ? as a delimiter and not as data. I then figured that if I built the URI by specifying each part, that encode would call encode_component on each piece separately. But the following produces the same results as above:

uri = Addressable::URI.parse("http://example.com")
uri.path << "/to_be_or_not_to_be?.txt"

Addressable::URI.encode(uri)

Looking at the code, it's because encode converts the argument passed to it to a string, regardless of whether or not it's a URI, calls parse on the resulting string to create a new URI and then encodes each component separately. It's that intermediate to string and parse that, while technically correct, is not what we intended (it'll treat that ? as the beginning of the query string).

I think this is a bug because looking at line 575 (in v2.3.6), there is a check to see if it's a URI. But it'll never be a URI because if you look up at the preceding begin/rescue, it's being converted to a string if it's anything but a string.

@sporkmonger
Copy link
Owner

Worth noting that this code is wrong:
uri.path << "/to_be_or_not_to_be?.txt"

Should be:
uri.path = "/to_be_or_not_to_be?.txt"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants