diff --git a/spec/Plugin/RedirectPluginSpec.php b/spec/Plugin/RedirectPluginSpec.php index e24b6b1..de9b30a 100644 --- a/spec/Plugin/RedirectPluginSpec.php +++ b/spec/Plugin/RedirectPluginSpec.php @@ -73,50 +73,19 @@ public function it_redirects_on_302( $finalPromise->wait()->shouldReturn($finalResponse); } - public function it_use_storage_on_301(UriInterface $uri, UriInterface $uriRedirect, RequestInterface $request, RequestInterface $modifiedRequest) - { - $this->beAnInstanceOf(RedirectPluginStub::class); - $this->beConstructedWith($uriRedirect, '/original', '301'); - - $next = function () { - throw new \Exception('Must not be called'); - }; - - $request->getUri()->willReturn($uri); - $uri->__toString()->willReturn('/original'); - $request->withUri($uriRedirect)->willReturn($modifiedRequest); - - $modifiedRequest->getUri()->willReturn($uriRedirect); - $modifiedRequest->getMethod()->willReturn('GET'); - - $uriRedirect->__toString()->willReturn('/redirect'); - - $this->handleRequest($request, $next, PluginStub::first()); - } - - public function it_stores_a_301( + public function it_use_storage_on_301( UriInterface $uri, UriInterface $uriRedirect, RequestInterface $request, - ResponseInterface $responseRedirect, RequestInterface $modifiedRequest, ResponseInterface $finalResponse, - Promise $promise + ResponseInterface $redirectResponse ) { - $this->beAnInstanceOf(RedirectPluginStub::class); - $this->beConstructedWith($uriRedirect, '', '301'); - $request->getUri()->willReturn($uri); - $uri->__toString()->willReturn('/301-url'); - - $responseRedirect->getStatusCode()->willReturn('301'); - $responseRedirect->hasHeader('Location')->willReturn(true); - $responseRedirect->getHeaderLine('Location')->willReturn('/redirect'); - + $uri->__toString()->willReturn('/original'); $uri->withPath('/redirect')->willReturn($uriRedirect); - $uriRedirect->withFragment('')->willReturn($uriRedirect); $uriRedirect->withQuery('')->willReturn($uriRedirect); - + $uriRedirect->withFragment('')->willReturn($uriRedirect); $request->withUri($uriRedirect)->willReturn($modifiedRequest); $modifiedRequest->getUri()->willReturn($uriRedirect); @@ -124,23 +93,55 @@ public function it_stores_a_301( $uriRedirect->__toString()->willReturn('/redirect'); - $next = function (RequestInterface $receivedRequest) use ($request, $responseRedirect) { - if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) { - return new HttpFulfilledPromise($responseRedirect->getWrappedObject()); - } - }; + $finalResponse->getStatusCode()->willReturn(200); - $first = function (RequestInterface $receivedRequest) use ($modifiedRequest, $promise) { - if (Argument::is($modifiedRequest->getWrappedObject())->scoreArgument($receivedRequest)) { - return $promise->getWrappedObject(); + $redirectResponse->getStatusCode()->willReturn(301); + $redirectResponse->hasHeader('Location')->willReturn(true); + $redirectResponse->getHeaderLine('Location')->willReturn('/redirect'); + + $nextCalled = false; + $next = function (RequestInterface $request) use (&$nextCalled, $finalResponse, $redirectResponse): Promise { + switch ($request->getUri()) { + case '/original': + if ($nextCalled) { + throw new \Exception('Must only be called once'); + } + $nextCalled = true; + + return new HttpFulfilledPromise($redirectResponse->getWrappedObject()); + case '/redirect': + + return new HttpFulfilledPromise($finalResponse->getWrappedObject()); + default: + throw new \Exception('Test setup error with request uri '.$request->getUri()); } }; + $first = $this->buildFirst($modifiedRequest, $next); - $promise->getState()->willReturn(Promise::FULFILLED); - $promise->wait()->shouldBeCalled()->willReturn($finalResponse); + $this->handleRequest($request, $next, $first); + // rebuild first as this is expected to be called again + $first = $this->buildFirst($modifiedRequest, $next); + // next should not be called again $this->handleRequest($request, $next, $first); - $this->hasStorage('/301-url')->shouldReturn(true); + } + + private function buildFirst(RequestInterface $modifiedRequest, callable $next): callable + { + $redirectPlugin = $this; + $firstCalled = false; + + return function (RequestInterface $request) use (&$modifiedRequest, $redirectPlugin, $next, &$firstCalled) { + if ($firstCalled) { + throw new \Exception('Only one restart expected'); + } + $firstCalled = true; + if ($modifiedRequest->getWrappedObject() !== $request) { + //throw new \Exception('Redirection failed'); + } + + return $redirectPlugin->getWrappedObject()->handleRequest($request, $next, $this); + }; } public function it_replace_full_url( @@ -359,35 +360,100 @@ public function it_clears_headers( $this->handleRequest($request, $next, $first); } - public function it_throws_circular_redirection_exception(UriInterface $uri, UriInterface $uriRedirect, RequestInterface $request, ResponseInterface $responseRedirect, RequestInterface $modifiedRequest) - { - $first = function () {}; + /** + * This is the "redirection does not redirect case. + */ + public function it_throws_circular_redirection_exception_on_redirect_that_does_not_change_url( + UriInterface $redirectUri, + RequestInterface $request, + ResponseInterface $redirectResponse + ) { + $redirectResponse->getStatusCode()->willReturn(302); + $redirectResponse->hasHeader('Location')->willReturn(true); + $redirectResponse->getHeaderLine('Location')->willReturn('/redirect'); - $this->beAnInstanceOf(RedirectPluginStubCircular::class); - $this->beConstructedWith(spl_object_hash((object) $first)); + $next = function () use ($redirectResponse): Promise { + return new HttpFulfilledPromise($redirectResponse->getWrappedObject()); + }; - $request->getUri()->willReturn($uri); - $uri->__toString()->willReturn('/original'); + $first = function () { + throw new \Exception('First should never be called'); + }; - $responseRedirect->getStatusCode()->willReturn('302'); - $responseRedirect->hasHeader('Location')->willReturn(true); - $responseRedirect->getHeaderLine('Location')->willReturn('/redirect'); + $request->getUri()->willReturn($redirectUri); + $redirectUri->__toString()->willReturn('/redirect'); - $uri->withPath('/redirect')->willReturn($uriRedirect); - $uriRedirect->withFragment('')->willReturn($uriRedirect); - $uriRedirect->withQuery('')->willReturn($uriRedirect); + $redirectUri->withPath('/redirect')->willReturn($redirectUri); + $redirectUri->withFragment('')->willReturn($redirectUri); + $redirectUri->withQuery('')->willReturn($redirectUri); - $request->withUri($uriRedirect)->willReturn($modifiedRequest); - $modifiedRequest->getUri()->willReturn($uriRedirect); - $uriRedirect->__toString()->willReturn('/redirect'); - $modifiedRequest->getMethod()->willReturn('GET'); + $request->withUri($redirectUri)->willReturn($request); + $redirectUri->__toString()->willReturn('/redirect'); + $request->getMethod()->willReturn('GET'); - $next = function (RequestInterface $receivedRequest) use ($request, $responseRedirect) { - if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) { - return new HttpFulfilledPromise($responseRedirect->getWrappedObject()); + $promise = $this->handleRequest($request, $next, $first); + $promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class); + $promise->shouldThrow(CircularRedirectionException::class)->duringWait(); + } + + /** + * This is a redirection flipping back and forth between two paths. + * + * There could be a larger loop but the logic in the plugin stays the same with as many redirects as needed. + */ + public function it_throws_circular_redirection_exception_on_alternating_redirect( + UriInterface $uri, + UriInterface $redirectUri, + RequestInterface $request, + ResponseInterface $redirectResponse1, + ResponseInterface $redirectResponse2, + RequestInterface $modifiedRequest + ) { + $redirectResponse1->getStatusCode()->willReturn(302); + $redirectResponse1->hasHeader('Location')->willReturn(true); + $redirectResponse1->getHeaderLine('Location')->willReturn('/redirect'); + + $redirectResponse2->getStatusCode()->willReturn(302); + $redirectResponse2->hasHeader('Location')->willReturn(true); + $redirectResponse2->getHeaderLine('Location')->willReturn('/original'); + + $next = function (RequestInterface $currentRequest) use ($request, $redirectResponse1, $redirectResponse2): Promise { + return ($currentRequest === $request->getWrappedObject()) + ? new HttpFulfilledPromise($redirectResponse1->getWrappedObject()) + : new HttpFulfilledPromise($redirectResponse2->getWrappedObject()) + ; + }; + + $redirectPlugin = $this; + $firstCalled = false; + $first = function (RequestInterface $request) use (&$firstCalled, $redirectPlugin, $next, &$first) { + if ($firstCalled) { + throw new \Exception('only one redirect expected'); } + $firstCalled = true; + + return $redirectPlugin->getWrappedObject()->handleRequest($request, $next, $first); }; + $request->getUri()->willReturn($uri); + $uri->__toString()->willReturn('/original'); + + $modifiedRequest->getUri()->willReturn($redirectUri); + $redirectUri->__toString()->willReturn('/redirect'); + + $uri->withPath('/redirect')->willReturn($redirectUri); + $redirectUri->withFragment('')->willReturn($redirectUri); + $redirectUri->withQuery('')->willReturn($redirectUri); + + $redirectUri->withPath('/original')->willReturn($uri); + $uri->withFragment('')->willReturn($uri); + $uri->withQuery('')->willReturn($uri); + + $request->withUri($redirectUri)->willReturn($modifiedRequest); + $request->getMethod()->willReturn('GET'); + $modifiedRequest->withUri($uri)->willReturn($request); + $modifiedRequest->getMethod()->willReturn('GET'); + $promise = $this->handleRequest($request, $next, $first); $promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class); $promise->shouldThrow(CircularRedirectionException::class)->duringWait(); @@ -440,33 +506,3 @@ public function it_redirects_http_to_https( $finalPromise->wait()->shouldReturn($finalResponse); } } - -class RedirectPluginStub extends RedirectPlugin -{ - public function __construct(UriInterface $uri, $storedUrl, $status, array $config = []) - { - parent::__construct($config); - - $this->redirectStorage[$storedUrl] = [ - 'uri' => $uri, - 'status' => $status, - ]; - } - - public function hasStorage($url) - { - return isset($this->redirectStorage[$url]); - } -} - -class RedirectPluginStubCircular extends RedirectPlugin -{ - public function __construct($chainHash) - { - $this->circularDetection = [ - $chainHash => [ - '/redirect', - ], - ]; - } -} diff --git a/src/Plugin/RedirectPlugin.php b/src/Plugin/RedirectPlugin.php index 80c5251..89b0f39 100644 --- a/src/Plugin/RedirectPlugin.php +++ b/src/Plugin/RedirectPlugin.php @@ -18,14 +18,14 @@ * * @author Joel Wurtz */ -class RedirectPlugin implements Plugin +final class RedirectPlugin implements Plugin { /** * Rule on how to redirect, change method for the new request. * * @var array */ - protected $redirectCodes = [ + private $redirectCodes = [ 300 => [ 'switch' => [ 'unless' => ['GET', 'HEAD'], @@ -79,26 +79,26 @@ class RedirectPlugin implements Plugin * false will ditch all previous headers * string[] will keep only headers with the specified names */ - protected $preserveHeader; + private $preserveHeader; /** * Store all previous redirect from 301 / 308 status code. * * @var array */ - protected $redirectStorage = []; + private $redirectStorage = []; /** * Whether the location header must be directly used for a multiple redirection status code (300). * * @var bool */ - protected $useDefaultForMultiple; + private $useDefaultForMultiple; /** * @var array */ - protected $circularDetection = []; + private $circularDetection = []; /** * @param array $config { @@ -143,7 +143,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl return $first($redirectRequest); } - return $next($request)->then(function (ResponseInterface $response) use ($request, $first) { + return $next($request)->then(function (ResponseInterface $response) use ($request, $first): ResponseInterface { $statusCode = $response->getStatusCode(); if (!array_key_exists($statusCode, $this->redirectCodes)) { @@ -171,7 +171,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl ]; } - // Call redirect request in synchrone + // Call redirect request synchronously $redirectPromise = $first($redirectRequest); return $redirectPromise->wait(); @@ -187,7 +187,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl * * @return MessageInterface|RequestInterface */ - protected function buildRedirectRequest(RequestInterface $request, UriInterface $uri, $statusCode) + private function buildRedirectRequest(RequestInterface $request, UriInterface $uri, $statusCode) { $request = $request->withUri($uri);