Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.17k stars 76 forks source link

Fixed rawurlencode for userInfo in Uri #202

Closed ezimuel closed 1 year ago

ezimuel commented 2 years ago

When you specify a username and password using Uri::withUserInfo() these are not encoded for URL (see here). For instance, if the username is foo@ and the password is bar the userInfo results in foo@:bar that is not a valid URL encoded string.

Other PSR-7 implementations use rawurlencode() for encoding the characters in the URL (e.g. Guzzle, Laminas-diactoros).

ezimuel commented 2 years ago

@Nyholm can you review this PR? Thanks.

Nyholm commented 2 years ago

Merging this PR will break all existing usage of this feature. I just quickly checked PSR7 specification and it does not mention anything about URL encoding.

I think it would be a better solution to throw an exception if you try to add UserInfo with characters not allowed.

ezimuel commented 2 years ago

@Nyholm I see your point. I did the change throwing an InvalidArgumentException if the user or the password is not URL encoded.

Nyholm commented 2 years ago

Thank you.

Let's refer to rfc3986 to determine if what is valid or not. I think it is this. Please verify:

user:
  REGEX: [a-zA-Z0-9-\._~%!\$&'\(\}\*\+,;=]+

password:
  REGEX: [a-zA-Z0-9-\._~%!\$&'\(\}\*\+,;=]+

We also need tests for this.

ezimuel commented 2 years ago

@Nyholm I checked rfc3986 and the correct regex is as follows:

^(?:[a-zA-Z0-9_\-\.~!\$&\'\(\)\*\+,;=:]|%[A-Fa-f0-9]{2})+$

We need to check for valid % HEXDIG HEXDIG that's why I used %[A-Fa-f0-9]{2}. I provided also some cases for unit testing.

ezimuel commented 2 years ago

@Nyholm I was thinking that we should throw two different exceptions for the user and for the password. Can we add specific Exception classes extending \InvalidArgumentException?

nicolas-grekas commented 1 year ago

Closing in favor of #213