diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index c91f37ff..db7c6c2a 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -5,15 +5,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Util; use Fig\Http\Message\RequestMethodInterface; -use Fig\Http\Message\StatusCodeInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use function Functional\contains; - -class UrlValidator implements UrlValidatorInterface, RequestMethodInterface, StatusCodeInterface +class UrlValidator implements UrlValidatorInterface, RequestMethodInterface { private const MAX_REDIRECTS = 15; @@ -30,32 +27,12 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface, Sta */ 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 { - $resp = $this->httpClient->request(self::METHOD_GET, $url, [ -// RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS], - RequestOptions::ALLOW_REDIRECTS => false, + $this->httpClient->request(self::METHOD_GET, $url, [ + RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS], ]); - - if ($redirectNum < self::MAX_REDIRECTS && $this->statusIsRedirect($resp->getStatusCode())) { - $this->doValidateUrl($resp->getHeaderLine('Location'), $redirectNum + 1); - } } catch (GuzzleException $e) { throw InvalidUrlException::fromUrl($url, $e); } } - - private function statusIsRedirect(int $statusCode): bool - { - return contains([self::STATUS_MOVED_PERMANENTLY, self::STATUS_FOUND], $statusCode); - } } diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php index af558e44..3a6880ea 100644 --- a/module/Core/test/Util/UrlValidatorTest.php +++ b/module/Core/test/Util/UrlValidatorTest.php @@ -7,17 +7,14 @@ namespace ShlinkioTest\Shlink\Core\Util; use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\RequestOptions; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Util\UrlValidator; -use Zend\Diactoros\Request; use Zend\Diactoros\Response; -use function Functional\map; -use function range; - class UrlValidatorTest extends TestCase { /** @var UrlValidator */ @@ -31,39 +28,17 @@ class UrlValidatorTest extends TestCase $this->urlValidator = new UrlValidator($this->httpClient->reveal()); } - /** - * @test - * @dataProvider provideAttemptThatThrows - */ - public function exceptionIsThrownWhenUrlIsInvalid(int $attemptThatThrows): void + /** @test */ + public function exceptionIsThrownWhenUrlIsInvalid(): void { - $callNum = 1; - $e = new ClientException('', $this->prophesize(Request::class)->reveal()); + $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class); - $request = $this->httpClient->request(Argument::cetera())->will( - function () use ($e, $attemptThatThrows, &$callNum) { - if ($callNum === $attemptThatThrows) { - throw $e; - } - - $callNum++; - return new Response('php://memory', 302, ['Location' => 'http://foo.com']); - } - ); - - $request->shouldBeCalledTimes($attemptThatThrows); + $request->shouldBeCalledOnce(); $this->expectException(InvalidUrlException::class); $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 */ public function expectedUrlIsCalledWhenTryingToVerify(): void { @@ -72,23 +47,11 @@ class UrlValidatorTest extends TestCase $request = $this->httpClient->request( RequestMethodInterface::METHOD_GET, $expectedUrl, - Argument::cetera() + [RequestOptions::ALLOW_REDIRECTS => ['max' => 15]] )->willReturn(new Response()); $this->urlValidator->validateUrl($expectedUrl); $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); - } }