Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.15k stars 75 forks source link

Escaping of user info breaks NTLM authentication #219

Closed sunkan closed 1 year ago

sunkan commented 1 year ago

The change in #213 breaks ntml based authentication when specifying a domain.

When using ntml auth it's common to have a domain that is part of the username like this

curl_setopt($ch, CURLOPT_USERPWD, 'DOMAIN\username:password');

The slash is not suppose to be escaped and if it's the authentication fails

nicolas-grekas commented 1 year ago

Did you try other implementations of PSR-7? AFAIK, they all encode the \, and this matches https://www.rfc-editor.org/rfc/rfc3986#appendix-A

I feel like the correct course of action is to pass rawurldecode($request->getUserInfo()) to curl here.

sunkan commented 1 year ago

I've tried laminas and realized it didn't work but it has worked fine with the library.

So your solution is for me to go and modify buzz?

It's a bit annoying that with this change there is no popular psr-7 implementation that can be used to talk to Microsoft's stuff.

nicolas-grekas commented 1 year ago

Buzz is clearly relying on an unspecified behavior yes... My reco is to replace buzz by Symfony HttpClient at this stage. And to fix Buzz indeed!

nicolas-grekas commented 1 year ago

(you can pin nyholm/psr7 to ~1.15.0 if you want also, but then you know you'll have some technical debt to fix at some point)

nicolas-grekas commented 1 year ago

Please link back if you do anything on another repo!

sunkan commented 1 year ago

My reason for using this library was because it was following the psr and that says nothing about escaping the username and password.

And the psr say that the UriInterface Is a Value object so in my eyes it shouldn't deal with escaping since as this prove the escaping is domain specific.

What you say is that ofc it should escape it and if I want to use it in any other way I need to know how it's been escaped and revers it instead of getting what I passed in. I feel like that's the definition of insanity.

nicolas-grekas commented 1 year ago

I'm sorry for the confusion. You might want to check https://github.com/php-fig/fig-standards/pull/1298 as a follow up.

The previous behavior wasn't correct either, as shown when doing $uri->withUserInfo('a#b'), which produces a broken URI string.

nicolas-grekas commented 1 year ago

I figured out a more seamless way to produce non-broken URLs, see #221

sunkan commented 1 year ago

Thank you for improving and fixing this