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

Uri::getBaseUrl() should escape @ in password #2201

Closed
Rudloff opened this issue Apr 15, 2017 · 7 comments
Closed

Uri::getBaseUrl() should escape @ in password #2201

Rudloff opened this issue Apr 15, 2017 · 7 comments
Assignees

Comments

@Rudloff
Copy link

Rudloff commented Apr 15, 2017

Hello,

Some user decided to use my app behind an Apache basic auth and some of the libraries I use started throwing errors about URLs. Turns out it was because they had a @ in their password.

And inded this code:

$url = new \Slim\Http\Uri('https', 'example.com', 443, '/', '', '', 'foo', 'b@r');
$url->getBaseUrl();

returns https://foo:b@[email protected].

But according the URL standard, the user and password should be percent-encoded: https://foo:b%[email protected].

@geggleto geggleto self-assigned this Apr 16, 2017
@geggleto geggleto added the bug label Apr 16, 2017
@geggleto geggleto added this to the 3.8.2 milestone Apr 16, 2017
@akrabat
Copy link
Member

akrabat commented Apr 17, 2017

I'm not sure that the URL Standard is an actual RFC.

However, I can see the argument that we should percent-encode at least the @ sign if it's in the user or password fields.

This URL Standard suggests percent-encoding all these characters:

U+0020 SPACE, U+0022 ("), U+0023 (#), U+003C (<), U+003E (>), U+003F (?), U+0060 (`),
U+007B ({), and U+007D (}), U+002F (/), U+003A (:), U+003B (;), U+003D (=), U+0040 (@),
U+005B ([), U+005C (\), U+005D (]), U+005E (^), and U+007C (|) 

If we did this, will we break BC?

@Rudloff
Copy link
Author

Rudloff commented Apr 17, 2017

Note that the W3C HTML validator also complains about this if such an URL is used in a href attribute.
Bad value http://foo:b@[email protected] for attribute href on element a: User or password contains an at symbol ("@") not percent-encoded.

@geggleto
Copy link
Member

@akrabat in theory a password could contain any of those values. I would still think this would be fine as it is a bug-fix...

@RyanNerd
Copy link
Contributor

RyanNerd commented Aug 4, 2017

@akrabat @geggleto Is this still an open issue? If so I can work on it (have some time on my hands right now)...

@akrabat
Copy link
Member

akrabat commented Aug 13, 2017

This should be fixed, yes @RyanNerd

@akrabat akrabat removed this from the 3.9.0 milestone Aug 13, 2017
@akrabat
Copy link
Member

akrabat commented Aug 13, 2017

Actually, see #2207 - this is needed for Slim 3 too.

@X-Tender
Copy link

I don't know if the output of the user:password@ values are still necessary or "up to date" because in Chrome you get an Deperacation error and the whole website don't work because Chrome blocks all traffic which goes through the base url.

[Deprecation] Subresource requests whose URLs contain embedded credentials (e.g. `https://user:pass@host/`) are blocked. See https://www.chromestatus.com/feature/5669008342777856 for more details.

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

No branches or pull requests

5 participants