Removed workarounds from UrlValidator that were required for guzzle 6.5.0

This commit is contained in:
Alejandro Celaya 2019-12-21 16:09:29 +01:00
parent 7c52d0ec19
commit d67321f187
2 changed files with 9 additions and 69 deletions

View file

@ -5,15 +5,12 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Util; namespace Shlinkio\Shlink\Core\Util;
use Fig\Http\Message\RequestMethodInterface; use Fig\Http\Message\RequestMethodInterface;
use Fig\Http\Message\StatusCodeInterface;
use GuzzleHttp\ClientInterface; use GuzzleHttp\ClientInterface;
use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Exception\GuzzleException;
use GuzzleHttp\RequestOptions; use GuzzleHttp\RequestOptions;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
use function Functional\contains; class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
class UrlValidator implements UrlValidatorInterface, RequestMethodInterface, StatusCodeInterface
{ {
private const MAX_REDIRECTS = 15; private const MAX_REDIRECTS = 15;
@ -30,32 +27,12 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface, Sta
*/ */
public function validateUrl(string $url): void public function validateUrl(string $url): void
{ {
$this->doValidateUrl($url);
}
/**
* @throws InvalidUrlException
*/
private function doValidateUrl(string $url, int $redirectNum = 1): void
{
// TODO Guzzle does not properly handle IDNs on redirects, just on first request.
// Because of that, we have to handle redirects manually.
try { try {
$resp = $this->httpClient->request(self::METHOD_GET, $url, [ $this->httpClient->request(self::METHOD_GET, $url, [
// RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS], RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS],
RequestOptions::ALLOW_REDIRECTS => false,
]); ]);
if ($redirectNum < self::MAX_REDIRECTS && $this->statusIsRedirect($resp->getStatusCode())) {
$this->doValidateUrl($resp->getHeaderLine('Location'), $redirectNum + 1);
}
} catch (GuzzleException $e) { } catch (GuzzleException $e) {
throw InvalidUrlException::fromUrl($url, $e); throw InvalidUrlException::fromUrl($url, $e);
} }
} }
private function statusIsRedirect(int $statusCode): bool
{
return contains([self::STATUS_MOVED_PERMANENTLY, self::STATUS_FOUND], $statusCode);
}
} }

View file

@ -7,17 +7,14 @@ namespace ShlinkioTest\Shlink\Core\Util;
use Fig\Http\Message\RequestMethodInterface; use Fig\Http\Message\RequestMethodInterface;
use GuzzleHttp\ClientInterface; use GuzzleHttp\ClientInterface;
use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\ClientException;
use GuzzleHttp\RequestOptions;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Prophecy\Argument; use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy; use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
use Shlinkio\Shlink\Core\Util\UrlValidator; use Shlinkio\Shlink\Core\Util\UrlValidator;
use Zend\Diactoros\Request;
use Zend\Diactoros\Response; use Zend\Diactoros\Response;
use function Functional\map;
use function range;
class UrlValidatorTest extends TestCase class UrlValidatorTest extends TestCase
{ {
/** @var UrlValidator */ /** @var UrlValidator */
@ -31,39 +28,17 @@ class UrlValidatorTest extends TestCase
$this->urlValidator = new UrlValidator($this->httpClient->reveal()); $this->urlValidator = new UrlValidator($this->httpClient->reveal());
} }
/** /** @test */
* @test public function exceptionIsThrownWhenUrlIsInvalid(): void
* @dataProvider provideAttemptThatThrows
*/
public function exceptionIsThrownWhenUrlIsInvalid(int $attemptThatThrows): void
{ {
$callNum = 1; $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class);
$e = new ClientException('', $this->prophesize(Request::class)->reveal());
$request = $this->httpClient->request(Argument::cetera())->will( $request->shouldBeCalledOnce();
function () use ($e, $attemptThatThrows, &$callNum) {
if ($callNum === $attemptThatThrows) {
throw $e;
}
$callNum++;
return new Response('php://memory', 302, ['Location' => 'http://foo.com']);
}
);
$request->shouldBeCalledTimes($attemptThatThrows);
$this->expectException(InvalidUrlException::class); $this->expectException(InvalidUrlException::class);
$this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar'); $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar');
} }
public function provideAttemptThatThrows(): iterable
{
return map(range(1, 15), function (int $attempt) {
return [$attempt];
});
}
/** @test */ /** @test */
public function expectedUrlIsCalledWhenTryingToVerify(): void public function expectedUrlIsCalledWhenTryingToVerify(): void
{ {
@ -72,23 +47,11 @@ class UrlValidatorTest extends TestCase
$request = $this->httpClient->request( $request = $this->httpClient->request(
RequestMethodInterface::METHOD_GET, RequestMethodInterface::METHOD_GET,
$expectedUrl, $expectedUrl,
Argument::cetera() [RequestOptions::ALLOW_REDIRECTS => ['max' => 15]]
)->willReturn(new Response()); )->willReturn(new Response());
$this->urlValidator->validateUrl($expectedUrl); $this->urlValidator->validateUrl($expectedUrl);
$request->shouldHaveBeenCalledOnce(); $request->shouldHaveBeenCalledOnce();
} }
/** @test */
public function urlIsConsideredValidWhenTooManyRedirectsAreReturned(): void
{
$request = $this->httpClient->request(Argument::cetera())->willReturn(
new Response('php://memory', 302, ['Location' => 'http://foo.com'])
);
$this->urlValidator->validateUrl('http://foobar.com');
$request->shouldHaveBeenCalledTimes(15);
}
} }