Skip to content

Commit

Permalink
Merge branch 'develop'
Browse files Browse the repository at this point in the history
* develop:
  specify next release
  update changelog
  make sure the initial request path is absolute
  make sure the initial request url has an authority
  make sure there is always an authority when following redirections
  CS
  make sure the initial request has an authority
  • Loading branch information
Baptouuuu committed Mar 1, 2024
2 parents 6b50e1d + 14d4b83 commit f991228
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 33 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 7.2.1 - 2024-03-01

### 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
Expand Down
16 changes: 4 additions & 12 deletions src/Curl.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -26,11 +22,7 @@
Capabilities,
Streams,
};
use Innmind\Immutable\{
Either,
Str,
Sequence,
};
use Innmind\Immutable\Either;

/**
* @psalm-import-type Errors from Transport
Expand Down
1 change: 0 additions & 1 deletion src/Curl/Ready.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
use Innmind\Filesystem\File\Content;
use Innmind\IO\IO;
use Innmind\Stream\{
Readable,
Writable,
Bidirectional,
};
Expand Down
1 change: 0 additions & 1 deletion src/Curl/Scheduled.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
use Innmind\Immutable\{
Either,
Str,
Sequence,
};

/**
Expand Down
6 changes: 1 addition & 5 deletions src/ExponentialBackoff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 19 additions & 2 deletions src/FollowRedirections.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
Response\StatusCode,
Header\Location,
};
use Innmind\Url\{
Url,
Authority,
};
use Innmind\Immutable\{
Either,
Sequence,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand All @@ -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()),
);
}
}
1 change: 0 additions & 1 deletion src/Logger.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
Response,
Headers,
Header,
Header\Value,
};
use Innmind\Immutable\{
Either,
Expand Down
5 changes: 1 addition & 4 deletions src/Transport.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@

namespace Innmind\HttpTransport;

use Innmind\Http\{
Request,
Response,
};
use Innmind\Http\Request;
use Innmind\Immutable\Either;

/**
Expand Down
38 changes: 31 additions & 7 deletions tests/FollowRedirectionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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\{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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());
}
Expand Down

0 comments on commit f991228

Please sign in to comment.