Skip to content
This repository has been archived by the owner on Jan 6, 2024. It is now read-only.

No Redirects following (default guzzle behaviour changed) #75

Closed
ricardofiorani opened this issue Jul 14, 2020 · 6 comments
Closed

No Redirects following (default guzzle behaviour changed) #75

ricardofiorani opened this issue Jul 14, 2020 · 6 comments

Comments

@ricardofiorani
Copy link

PHP version: PHP 7.4.8 (But should happen in any other I guess)

Description
When I use guzzle6, it follows redirects, when I use this adapter, it doesn't follow redirects by default.

How to reproduce

<?php
require "vendor/autoload.php";

$uri = new \GuzzleHttp\Psr7\Uri('https://embed.spotify.com/oembed?url=https%3A%2F%2Fopen.spotify.com%2Fplaylist%2F37i9dQZF1DWYtg7TV07mgz%3Fsi%3DOTx1DyDhSPK5q5V1CTR5NQ&format=json');
$request = new \GuzzleHttp\Psr7\Request('GET',$uri);

$goodOldGuzzle = new \GuzzleHttp\Client();
$guzzleAdapter = \Http\Adapter\Guzzle6\Client::createWithConfig([]);

var_dump(
	$goodOldGuzzle->send($request)->getStatusCode(),
	$guzzleAdapter->sendRequest($request)->getStatusCode(),
);

Responses will be 200 and 304.

e.g.: https://phpsandbox.io/n/bitter-frog-l2q1

Possible Solution
Sorry, I didn't dig much into this library so no idea. My guess is to not change the default options of guzzle maybe ? 🤷

Additional context
I made this one sandbox thingy to prove my problem.
https://phpsandbox.io/n/bitter-frog-l2q1

@dbu
Copy link
Contributor

dbu commented Jul 15, 2020

this is the expected behaviour according to the specification: https://www.php-fig.org/psr/psr-18/meta/#middleware-and-wrapping-a-client

all psr http clients are expected to behave the same way by default.

depending on what you do, you want to explicitly know about redirections. when building an api client, you probably want exceptions in all but the production environment to realize you are being inefficient, etc.

if it makes sense to have redirections in your project, you have 2 options:

  1. you can decorate any psr client with a plugin client and use the php-http redirect plugin
  2. you can instantiate the guzzle client yourself and pass it to the guzzle6-adapter: $adapter = new GuzzleAdapter($guzzle); (afaik the default guzzle also throws exceptions on 4xx/5xx responses, which PSR is supposed to also not do)

@ricardofiorani
Copy link
Author

Oh, I didn't know the problem was deep down on PSR-18 specification itself.

I mean, I'm assuming people want to have the functionality of following redirects as it makes everything easier instead of tumbling another problem on the PSR level. So I just question myself why was it done this way.

Anyway thank you for your help @dbu, I really appreciate it!

@dbu
Copy link
Contributor

dbu commented Jul 16, 2020

Anyway thank you for your help @dbu, I really appreciate it!

glad i could help 👍

Oh, I didn't know the problem was deep down on PSR-18 specification itself.

i don't think its a problem or a mistake of the PSR. it depends on the use case, redirection does not make sense for everyone. the PSR had to chose one way to guarantee a consistent experience over all clients. adding redirection handling is doable in such a scanner - removing it by wrapping the client somehow is impossible.

if you build an api client and don't notice that every single request you do is redirected, the efficiency penalty is massive, so you really want to know about that.

if you build a library to scan some site and want to explicitly know about redirects, you really need a client that does not follow redirects itself.

@ricardofiorani
Copy link
Author

Sorry, I didn't mean to say in a negative way against the PSR-18 itself, I completely respect the work that was done.
I just meant that the prevention for my problem's solution is the way that the PSR-18 was designed.
It would be nice if straight from a PSR18 client we would have the ability to tell if it should follow or not redirects, as other configurations like to set a proxy or set the timeout.

@xabbuh
Copy link
Member

xabbuh commented Jul 16, 2020

@ricardofiorani There is a redirect plugin. I think that should be of interest for you.

@ricardofiorani
Copy link
Author

That is exactly what I needed!
Thank you @xabbuh! 🙇

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

No branches or pull requests

3 participants