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

Encode user info #2327

Merged
merged 1 commit into from Nov 3, 2017
Merged

Encode user info #2327

merged 1 commit into from Nov 3, 2017

Conversation

akrabat
Copy link
Member

@akrabat akrabat commented Nov 3, 2017

If the username or password includes an @, : or other reserved
characters, they need to be encoded.

Fixes #2201

@akrabat akrabat added this to the 3.9.0 milestone Nov 3, 2017
If the username or password includes an `@`, `:` or other reserved
characters, they need to be encoded.

Fixes #2201
akrabat added a commit to akrabat/Slim-Http that referenced this pull request Nov 3, 2017
If the username or password includes an `@` or `:` or other reserved
characters, they need to be encoded.

Forward ports slimphp/Slim#2327.
akrabat added a commit to akrabat/Slim-Http that referenced this pull request Nov 3, 2017
If the username or password includes an `@` or `:` or other reserved
characters, they need to be encoded.

Fixes slimphp#34
Forward ports slimphp/Slim#2327.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 93.283% when pulling 72309b2 on akrabat:encode-user-info into 67a6cb3 on slimphp:3.x.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 93.283% when pulling 72309b2 on akrabat:encode-user-info into 67a6cb3 on slimphp:3.x.

@silentworks silentworks merged commit e42b86a into slimphp:3.x Nov 3, 2017
@mahagr
Copy link

mahagr commented Nov 6, 2017

Just a note: filterUserInfo() has /u modifier, but filterPath() and filterQuery() do not. Should they all have the modifier?

@akrabat
Copy link
Member Author

akrabat commented Nov 6, 2017

@mahagr Now that is a very good question :)

I think that they should all have /u as I think unicode is allowed in URIs. (e.g http://☃.net)

@mahagr
Copy link

mahagr commented Nov 6, 2017

Also another thing... Should __construct() also escape user and password?

@mahagr
Copy link

mahagr commented Nov 6, 2017

And from your example: hostname isn't filtered either...

@akrabat
Copy link
Member Author

akrabat commented Nov 6, 2017

Yes - I would appreciate a PR :)

@akrabat
Copy link
Member Author

akrabat commented Nov 6, 2017

Re hostname: is there any reason to filter it?

@mahagr
Copy link

mahagr commented Nov 6, 2017

You can use Uri class outside of request to build a new URL to be shown your page.

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

Successfully merging this pull request may close these issues.

None yet

4 participants