diff --git a/src/RedirectMiddleware.php b/src/RedirectMiddleware.php index 1dd38614f..3ee8e8ead 100644 --- a/src/RedirectMiddleware.php +++ b/src/RedirectMiddleware.php @@ -132,7 +132,7 @@ static function (ResponseInterface $response) use ($uri, $statusCode) { } /** - * Check for too many redirects + * Check for too many redirects. * * @throws TooManyRedirectsException Too many redirects. */ @@ -168,7 +168,7 @@ public function modifyRequest(RequestInterface $request, array $options, Respons $modify['body'] = ''; } - $uri = $this->redirectUri($request, $response, $protocols); + $uri = self::redirectUri($request, $response, $protocols); if (isset($options['idn_conversion']) && ($options['idn_conversion'] !== false)) { $idnOptions = ($options['idn_conversion'] === true) ? \IDNA_DEFAULT : $options['idn_conversion']; $uri = Utils::idnUriConvert($uri, $idnOptions); @@ -188,19 +188,46 @@ public function modifyRequest(RequestInterface $request, array $options, Respons $modify['remove_headers'][] = 'Referer'; } - // Remove Authorization header if host is different. - if ($request->getUri()->getHost() !== $modify['uri']->getHost()) { + // Remove Authorization and Cookie headers if required. + if (self::shouldStripSensitiveHeaders($request->getUri(), $modify['uri'])) { $modify['remove_headers'][] = 'Authorization'; + $modify['remove_headers'][] = 'Cookie'; } return Psr7\Utils::modifyRequest($request, $modify); } /** - * Set the appropriate URL on the request based on the location header + * Determine if we should strip sensitive headers from the request. + * + * We return true if either of the following conditions are true: + * + * 1. the host is different; + * 2. the scheme has changed, and now is non-https. */ - private function redirectUri(RequestInterface $request, ResponseInterface $response, array $protocols): UriInterface - { + private static function shouldStripSensitiveHeaders( + UriInterface $originalUri, + UriInterface $modifiedUri + ): bool { + if (\strcasecmp($originalUri->getHost(), $modifiedUri->getHost()) !== 0) { + return true; + } + + if ($originalUri->getScheme() !== $modifiedUri->getScheme() && 'https' !== $modifiedUri->getScheme()) { + return true; + } + + return false; + } + + /** + * Set the appropriate URL on the request based on the location header. + */ + private static function redirectUri( + RequestInterface $request, + ResponseInterface $response, + array $protocols + ): UriInterface { $location = Psr7\UriResolver::resolve( $request->getUri(), new Psr7\Uri($response->getHeaderLine('Location')) diff --git a/tests/RedirectMiddlewareTest.php b/tests/RedirectMiddlewareTest.php index bc587c0a2..dcd98cce1 100644 --- a/tests/RedirectMiddlewareTest.php +++ b/tests/RedirectMiddlewareTest.php @@ -272,18 +272,37 @@ public function testInvokesOnRedirectForRedirects() self::assertTrue($call); } - public function testRemoveAuthorizationHeaderOnRedirect() + public function crossOriginRedirectProvider() + { + return [ + ['http://example.com?a=b', 'http://test.com/', false], + ['https://example.com?a=b', 'https://test.com/', false], + ['http://example.com?a=b', 'https://test.com/', false], + ['https://example.com?a=b', 'http://test.com/', false], + ['http://example.com?a=b', 'http://example.com/', true], + ['https://example.com?a=b', 'https://example.com/', true], + ['http://example.com?a=b', 'https://example.com/', true], + ['https://example.com?a=b', 'http://example.com/', false], + ]; + } + + /** + * @dataProvider crossOriginRedirectProvider + */ + public function testHeadersTreatmentOnRedirect($originalUri, $targetUri, $shouldBePresent) { $mock = new MockHandler([ - new Response(302, ['Location' => 'http://test.com']), - static function (RequestInterface $request) { - self::assertFalse($request->hasHeader('Authorization')); + new Response(302, ['Location' => $targetUri]), + static function (RequestInterface $request) use ($shouldBePresent) { + self::assertSame($shouldBePresent, $request->hasHeader('Authorization')); + self::assertSame($shouldBePresent, $request->hasHeader('Cookie')); + return new Response(200); } ]); $handler = HandlerStack::create($mock); $client = new Client(['handler' => $handler]); - $client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass']]); + $client->get($originalUri, ['auth' => ['testuser', 'testpass'], 'headers' => ['Cookie' => 'foo=bar']]); } public function testNotRemoveAuthorizationHeaderOnRedirect()