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

Exception on url with redirects. #406

Open
Stajor opened this issue Jul 23, 2022 · 3 comments
Open

Exception on url with redirects. #406

Stajor opened this issue Jul 23, 2022 · 3 comments

Comments

@Stajor
Copy link

Stajor commented Jul 23, 2022

When I'm running Guzzle standalone requests, get status 200

$url = 'https://iphones.ru/';
$guzzle = new \GuzzleHttp\Client();
$result = $guzzle->get($url);
var_dump($result->getStatusCode()); // status 200

When running with FeedIo, it's throws an exception

$url = 'https://iphones.ru/';
$guzzle = new \GuzzleHttp\Client();
$client = new \FeedIo\Adapter\Http\Client($guzzle);
$feedIo = new \FeedIo\FeedIo($client);
$feeds = $feedIo->discover($url);
/*
FeedIo\Adapter\ServerErrorException 

internal server error
*/

composer.json

"guzzlehttp/guzzle": "7.4.5",
"debril/feed-io": "6.0"
@alexdebril
Copy link
Owner

Probably something you can solve by setting some configuration to Guzzle right before the call (I think it's already the case on feed reading). Can you submit the pull request to solve this please ?
thanks in advance

@ABouchard55
Copy link

ABouchard55 commented Mar 30, 2023

Actually, because of PSR-18, the use of sendRequest instead of request will forcefully disallow redirects, and you use $this->client->sendRequest($request), $this->client being an instance of PsrClientInterface.

@doenietzomoeilijk
Copy link

doenietzomoeilijk commented Apr 17, 2023

Can confirm, I'm running into the same issue, and while it theoretically could easily be solved, it opens up a few questions.

  • If I read this issue correctly, Guzzle (and presumably other PSR-18 compliant libraries) would all exhibit the same behaviour wrt redirects, so "just don't use Guzzle" wouldn't be a solution.
  • The \FeedIo\Reader::read() will have to do the follow-the-redirect song and dance itself, then, which means it'll need at least the most basic configuration ("should we allow this" and "how many redirects should we follow").
  • As far as I'm aware, there's not much in terms of configurability in the Reader, as in none at all, so that's something that needs to be added from scratch.
  • Alternatively, it's not configurable, but it'll require sane and safe defaults. What are those? The same that Guzzle uses?
  • It would need to be documented, of course, because (this is an assumption) most users will simply drop in a Guzzle client and expect things to work, unawares of the PSR-18 thing...

@alexdebril do you have any insights into usage wrt the third party client libraries? Any preference in which direction this could / should move?

Edit:

As a workaround, I've extended the \FeedIo\Adapter\Http\Client, and override the request() method thusly:

    protected function request(string $method, string $url, DateTime $modifiedSince = null): ResponseInterface
    {
        try {
            return parent::request($method, $url, $modifiedSince);
        } catch (ServerErrorException $e) {
            $psrResponse = $e->getResponse();
            if (in_array($psrResponse->getStatusCode(), [301, 302], true)) {
                return parent::request($method, $psrResponse->getHeader("location")[0], $modifiedSince);
            }

            throw $e;
        }
    }

This works for one level of redirect, which is most likely enough for most cases (redirecting to whatever URL you tried first, with a slash added or removed at the end).

Personally, I can live with this, even though I do consider this a bit of a hacky solution... :)

I could also PR it if you'd appreciate that.

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

No branches or pull requests

4 participants