php-http / client-common

Common HTTP Client implementations and tools for HTTPlug
http://httplug.io
MIT License
1.03k stars 54 forks source link

RedirectPlugin is wrongly overwriting path and falsely detects circular redirect #217

Closed ostrolucky closed 2 years ago

ostrolucky commented 2 years ago

PHP version: 8.1.7

Description Given HTTPlug requests http://foo/entry?deviceoutput=app And URL above responds with redirect to http://foo?deviceoutput=app Then RedirectPlugin falsely detects circular redirect

Possible Solution I believe reason for this is because original request is used as a base object for target location here https://github.com/php-http/client-common/blob/master/src/Plugin/RedirectPlugin.php#L233

And path from it is not overridden if it's not specified in location header.

ostrolucky commented 2 years ago

@dbu what do you think we should do? I would assume we can simply remove that condition and default to empty path instead, but I don't know what was the original reason in wrapping it in the if condition in the first place

dbu commented 2 years ago

would a redirect to http://foo/?deviceoutput=app also be considered circular? but yeah, it is incorrect.

so our redirect plugin would interpret that redirection as "go to host foo but keep the previous path"? if thats the case, it indeed would lead to circular redirection - but the logic would be false. afaik redirect with a host but no path should set the path to / and not keep the path from the original request.

dbu commented 2 years ago

i am away over the weekend, so please don't expect further answers from me before monday ;-)

ostrolucky commented 2 years ago

Alright. Meanwhile I've created https://github.com/php-http/client-common/pull/218. Btw perhaps it's time to stop supporting PHP 7.1 ;)