Setting a password with the '#' character results in invalid URI

[sporkmonger]

Setting a password with the '#' character results in invalid URI

[sporkmonger]

Actually, no. Passwords may not contain # characters. They have to be percent encoded.

[sporkmonger]

Actually, this is heuristic_parse, not parse, and in this case, intent is clear. But just so you're aware, that's not a valid URI.

[sporkmonger]

Let me think about this one. I'm pretty sure there's security considerations here that need to be taken into account. Doing the wrong thing could potentially open people up to phishing attacks.

[bblimke]

Thanks for looking at that. I understand it's not a vaild url but maybe it's possible to parse it, same as heuristic_parse is able to parse uri's with not encoded paths or params. I understand it may be very tricky but maybe there is a way to do it, without breaking existing parsing behaviour.

[sporkmonger]

It's not the trickiness of the parsing that concerns me. The issue here is that user/password exposure in URIs has been deprecated to begin with, and historically it's been used for phishing. The heuristic_parse is intended for usage in scenarios where users have manually entered a URI. If a user is typing their password into a URI themselves, I consider that to be some pretty massive fail, i.e., the only use-case I care about is undesirable. And then add in the issue that the user and password components have historically been one of the main ways that phishing happens, and that makes this a change I'm inclined to avoid making.

As an example:

https://www.google.com:[e-mail]/

[sporkmonger]

Could you explain why you have to parse an invalid URI that contains a user/password combo? Might help if I understood the use-case.

[bblimke]

Thanks for the explanation regarding usage of credentials in the uri (I didn't know it's deprecated :)
I use addressable in WebMock. WebMock allows developers to pass escaped or unescaped versions of uris when stubbing requests or setting request expectations.
@ebeigarts experienced a problem with a password with '#'. bblimke/webmock#116
It's not a very common case. People can workaround it by encoding '#' so it's not a big problem if it's not convenient to change Addressable behaviour.

[sporkmonger]

Well, if the password is set via uri.password = 'passw#rd', it should work fine. It just won't parse it if it's unencoded because... well, it's not valid.

[bblimke]

Ok, thank you for investigating the issue.

[sporkmonger]

Yup, no problem.

[ebeigarts]

Should'nt #password= and #user= automatically encode symbols like #@: or encode them when calling #to_s to avoid generating invalid URI?

[ebeigarts]

What is the best way/method to encode password 'pass#:@word' ?

Addressable::URI.encode_component("pass:#\@word", Addressable::URI::CharacterClasses::UNRESERVED + Addressable::URI::CharacterClasses::SUB_DELIMS) ?

[sporkmonger]

It does, but it's not obvious:

>> require 'addressable/uri'
=> true
>> uri = Addressable::URI.parse('http://example.com/')
=> #<Addressable::URI:0x80769fa0 URI:http://example.com/>
>> uri.user = 'test'
=> "test"
>> uri.password = '123#321'
=> "123#321"
>> uri
=> #<Addressable::URI:0x80769fa0 URI:http://test:123#[email protected]/>
>> uri.normalize
=> #<Addressable::URI:0x8073f3f4 URI:http://test:123%[email protected]/>

However, that actually is a bug.

[sporkmonger]

I changed my mind. This isn't a bug. It's just behavior that's not obvious. When you assign a property, you should reasonably expect that property to return the same value when accessed immediately after. This would be weird:

>> uri.password = '123#321'
=> "123#321"
>> uri.password
=> "123%23321"

The correct thing to do is to normalize after URI construction.

However, none of this resolves the issue that was reported on the WebMock project, but that seems to be a user error more than anything else. I looked at your code in WebMock and it's normalizing correctly. The issue that was reported on the WebMock project was for an error during parsing, which would only be resolved by actually passing something approaching a valid URI as the input. i.e., Both issues should be closed, and the original reporter should just stop using invalid URIs. :-)