diff --git a/CHANGELOG.md b/CHANGELOG.md index 323458c..6ea50fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- When following redirections (via `FollowRedirections`) if the target url is only a path it crashed because no host was provided, now the target url is resolved based on the previous request + ## 7.2.0 - 2023-12-14 ### Added diff --git a/src/Curl.php b/src/Curl.php index b1320b0..b7f9ff4 100644 --- a/src/Curl.php +++ b/src/Curl.php @@ -3,13 +3,9 @@ namespace Innmind\HttpTransport; -use Innmind\HttpTransport\{ - Curl\Scheduled, - Curl\Ready, - Curl\Concurrency, - Transport, - Failure, - Success, +use Innmind\HttpTransport\Curl\{ + Scheduled, + Concurrency }; use Innmind\Http\{ Request, @@ -26,11 +22,7 @@ Capabilities, Streams, }; -use Innmind\Immutable\{ - Either, - Str, - Sequence, -}; +use Innmind\Immutable\Either; /** * @psalm-import-type Errors from Transport diff --git a/src/Curl/Ready.php b/src/Curl/Ready.php index 8125f6a..920e0bb 100644 --- a/src/Curl/Ready.php +++ b/src/Curl/Ready.php @@ -27,7 +27,6 @@ use Innmind\Filesystem\File\Content; use Innmind\IO\IO; use Innmind\Stream\{ - Readable, Writable, Bidirectional, }; diff --git a/src/Curl/Scheduled.php b/src/Curl/Scheduled.php index c8736d4..2fa6374 100644 --- a/src/Curl/Scheduled.php +++ b/src/Curl/Scheduled.php @@ -27,7 +27,6 @@ use Innmind\Immutable\{ Either, Str, - Sequence, }; /** diff --git a/src/ExponentialBackoff.php b/src/ExponentialBackoff.php index 0c5fdfa..edb81d0 100644 --- a/src/ExponentialBackoff.php +++ b/src/ExponentialBackoff.php @@ -3,11 +3,7 @@ namespace Innmind\HttpTransport; -use Innmind\Http\{ - Request, - Response, - Response\StatusCode, -}; +use Innmind\Http\Request; use Innmind\TimeContinuum\{ Period, Earth\Period\Millisecond, diff --git a/src/FollowRedirections.php b/src/FollowRedirections.php index 32a4aef..ae63ccf 100644 --- a/src/FollowRedirections.php +++ b/src/FollowRedirections.php @@ -9,6 +9,10 @@ Response\StatusCode, Header\Location, }; +use Innmind\Url\{ + Url, + Authority, +}; use Innmind\Immutable\{ Either, Sequence, @@ -93,7 +97,7 @@ private function redirect(Redirection $redirection): Either ->find(Location::class) ->map(static fn($header) => $header->url()) ->map(static fn($url) => Request::of( - $url, + self::resolveUrl($redirection->request()->url(), $url), $redirection->request()->method(), $redirection->request()->protocolVersion(), $redirection->request()->headers(), @@ -122,7 +126,7 @@ private function seeOther(Redirection $redirection): Either ->find(Location::class) ->map(static fn($header) => $header->url()) ->map(static fn($url) => Request::of( - $url, + self::resolveUrl($redirection->request()->url(), $url), Method::get, $redirection->request()->protocolVersion(), $redirection->request()->headers(), @@ -132,4 +136,17 @@ private function seeOther(Redirection $redirection): Either static fn() => Either::left($redirection), ); } + + private static function resolveUrl(Url $request, Url $location): Url + { + if ($location->authority()->equals(Authority::none())) { + $location = $location + ->withScheme($request->scheme()) + ->withAuthority($request->authority()); + } + + return $location->withPath( + $request->path()->resolve($location->path()), + ); + } } diff --git a/src/Logger.php b/src/Logger.php index f7ce2b9..66d7d38 100644 --- a/src/Logger.php +++ b/src/Logger.php @@ -8,7 +8,6 @@ Response, Headers, Header, - Header\Value, }; use Innmind\Immutable\{ Either, diff --git a/src/Transport.php b/src/Transport.php index 43734e9..227e16d 100644 --- a/src/Transport.php +++ b/src/Transport.php @@ -3,10 +3,7 @@ namespace Innmind\HttpTransport; -use Innmind\Http\{ - Request, - Response, -}; +use Innmind\Http\Request; use Innmind\Immutable\Either; /** diff --git a/tests/FollowRedirectionsTest.php b/tests/FollowRedirectionsTest.php index 2d430a4..d34901d 100644 --- a/tests/FollowRedirectionsTest.php +++ b/tests/FollowRedirectionsTest.php @@ -25,7 +25,10 @@ Header\Location, }; use Innmind\Filesystem\File\Content; -use Innmind\Url\Url; +use Innmind\Url\{ + Url, + Authority, +}; use Innmind\Immutable\Either; use PHPUnit\Framework\TestCase; use Innmind\BlackBox\{ @@ -149,10 +152,11 @@ public function testRedirectMaximum5Times() $inner ->expects($matcher = $this->exactly(6)) ->method('__invoke') - ->willReturnCallback(function($request) use ($matcher, $start, $newUrl, $expected) { + ->willReturnCallback(function($request) use ($matcher, $start, $firstUrl, $expected) { match ($matcher->numberOfInvocations()) { 1 => $this->assertSame($start, $request), - default => $this->assertSame($newUrl, $request->url()), + // assertions on the new url are done in self::testRedirect() and self::testRedirectSeeOther() + default => $this->assertNotSame($firstUrl, $request->url()), }; return $expected; @@ -214,7 +218,9 @@ public function testRedirectSeeOther() { $this ->forAll( - FUrl::any(), + FUrl::any() + ->filter(static fn($url) => !$url->authority()->equals(Authority::none())) + ->filter(static fn($url) => $url->path()->absolute()), FUrl::any(), Set\Elements::of(...Method::cases()), Set\Elements::of( @@ -248,7 +254,15 @@ public function testRedirectSeeOther() $this->assertSame($start, $request); } else { $this->assertSame(Method::get, $request->method()); - $this->assertSame($newUrl, $request->url()); + $this->assertFalse($request->url()->authority()->equals(Authority::none())); + $this->assertTrue($request->url()->path()->absolute()); + // not a direct comparison as new url might be a relative path + $this->assertStringEndsWith( + $newUrl->path()->toString(), + $request->url()->path()->toString(), + ); + $this->assertSame($newUrl->query(), $request->url()->query()); + $this->assertSame($newUrl->fragment(), $request->url()->fragment()); $this->assertSame($start->headers(), $request->headers()); $this->assertSame('', $request->body()->toString()); } @@ -279,7 +293,9 @@ public function testRedirect() { $this ->forAll( - FUrl::any(), + FUrl::any() + ->filter(static fn($url) => !$url->authority()->equals(Authority::none())) + ->filter(static fn($url) => $url->path()->absolute()), FUrl::any(), Set\Elements::of(Method::get, Method::head), // unsafe methods are not redirected Set\Elements::of( @@ -319,7 +335,15 @@ public function testRedirect() $this->assertSame($start, $request); } else { $this->assertSame($start->method(), $request->method()); - $this->assertSame($newUrl, $request->url()); + $this->assertFalse($request->url()->authority()->equals(Authority::none())); + $this->assertTrue($request->url()->path()->absolute()); + // not a direct comparison as new url might be a relative path + $this->assertStringEndsWith( + $newUrl->path()->toString(), + $request->url()->path()->toString(), + ); + $this->assertSame($newUrl->query(), $request->url()->query()); + $this->assertSame($newUrl->fragment(), $request->url()->fragment()); $this->assertSame($start->headers(), $request->headers()); $this->assertSame($start->body(), $request->body()); }