From e18604e1114d6b279be2915d3d6a44152ae1c2c4 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Mon, 20 Jun 2022 23:16:13 +0100 Subject: [PATCH] Release 7.4.5 (#3043) (cherry picked from commit 1dd98b0564cb3f6bd16ce683cb755f94c10fbd82) --- composer.json | 2 +- src/RedirectMiddleware.php | 35 ++---- tests/RedirectMiddlewareTest.php | 177 ++++++++++++++++++++++++++++--- 3 files changed, 176 insertions(+), 38 deletions(-) diff --git a/composer.json b/composer.json index 5da35a5bc..b2623a943 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "php": "^7.2.5 || ^8.0", "ext-json": "*", "guzzlehttp/promises": "^1.4", - "guzzlehttp/psr7": "^1.7 || ^2.0", + "guzzlehttp/psr7": "^1.9 || ^2.4", "psr/http-client": "^1.0" }, "provide": { diff --git a/src/RedirectMiddleware.php b/src/RedirectMiddleware.php index 3ee8e8ead..f67d448be 100644 --- a/src/RedirectMiddleware.php +++ b/src/RedirectMiddleware.php @@ -88,6 +88,14 @@ public function checkRedirect(RequestInterface $request, array $options, Respons $this->guardMax($request, $response, $options); $nextRequest = $this->modifyRequest($request, $options, $response); + // If authorization is handled by curl, unset it if URI is cross-origin. + if (Psr7\UriComparator::isCrossOrigin($request->getUri(), $nextRequest->getUri()) && defined('\CURLOPT_HTTPAUTH')) { + unset( + $options['curl'][\CURLOPT_HTTPAUTH], + $options['curl'][\CURLOPT_USERPWD] + ); + } + if (isset($options['allow_redirects']['on_redirect'])) { ($options['allow_redirects']['on_redirect'])( $request, @@ -188,8 +196,8 @@ public function modifyRequest(RequestInterface $request, array $options, Respons $modify['remove_headers'][] = 'Referer'; } - // Remove Authorization and Cookie headers if required. - if (self::shouldStripSensitiveHeaders($request->getUri(), $modify['uri'])) { + // Remove Authorization and Cookie headers if URI is cross-origin. + if (Psr7\UriComparator::isCrossOrigin($request->getUri(), $modify['uri'])) { $modify['remove_headers'][] = 'Authorization'; $modify['remove_headers'][] = 'Cookie'; } @@ -197,29 +205,6 @@ public function modifyRequest(RequestInterface $request, array $options, Respons return Psr7\Utils::modifyRequest($request, $modify); } - /** - * 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 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. */ diff --git a/tests/RedirectMiddlewareTest.php b/tests/RedirectMiddlewareTest.php index dcd98cce1..5ca36ccb7 100644 --- a/tests/RedirectMiddlewareTest.php +++ b/tests/RedirectMiddlewareTest.php @@ -272,30 +272,183 @@ public function testInvokesOnRedirectForRedirects() self::assertTrue($call); } + /** + * @testWith ["digest"] + * ["ntlm"] + */ + public function testRemoveCurlAuthorizationOptionsOnRedirectCrossHost($auth) + { + if (!defined('\CURLOPT_HTTPAUTH')) { + self::markTestSkipped('ext-curl is required for this test'); + } + + $mock = new MockHandler([ + new Response(302, ['Location' => 'http://test.com']), + static function (RequestInterface $request, $options) { + self::assertFalse( + isset($options['curl'][\CURLOPT_HTTPAUTH]), + 'curl options still contain CURLOPT_HTTPAUTH entry' + ); + self::assertFalse( + isset($options['curl'][\CURLOPT_USERPWD]), + 'curl options still contain CURLOPT_USERPWD entry' + ); + return new Response(200); + } + ]); + $handler = HandlerStack::create($mock); + $client = new Client(['handler' => $handler]); + $client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]); + } + + /** + * @testWith ["digest"] + * ["ntlm"] + */ + public function testRemoveCurlAuthorizationOptionsOnRedirectCrossPort($auth) + { + if (!defined('\CURLOPT_HTTPAUTH')) { + self::markTestSkipped('ext-curl is required for this test'); + } + + $mock = new MockHandler([ + new Response(302, ['Location' => 'http://example.com:81/']), + static function (RequestInterface $request, $options) { + self::assertFalse( + isset($options['curl'][\CURLOPT_HTTPAUTH]), + 'curl options still contain CURLOPT_HTTPAUTH entry' + ); + self::assertFalse( + isset($options['curl'][\CURLOPT_USERPWD]), + 'curl options still contain CURLOPT_USERPWD entry' + ); + return new Response(200); + } + ]); + $handler = HandlerStack::create($mock); + $client = new Client(['handler' => $handler]); + $client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]); + } + + /** + * @testWith ["digest"] + * ["ntlm"] + */ + public function testRemoveCurlAuthorizationOptionsOnRedirectCrossScheme($auth) + { + if (!defined('\CURLOPT_HTTPAUTH')) { + self::markTestSkipped('ext-curl is required for this test'); + } + + $mock = new MockHandler([ + new Response(302, ['Location' => 'http://example.com?a=b']), + static function (RequestInterface $request, $options) { + self::assertFalse( + isset($options['curl'][\CURLOPT_HTTPAUTH]), + 'curl options still contain CURLOPT_HTTPAUTH entry' + ); + self::assertFalse( + isset($options['curl'][\CURLOPT_USERPWD]), + 'curl options still contain CURLOPT_USERPWD entry' + ); + return new Response(200); + } + ]); + $handler = HandlerStack::create($mock); + $client = new Client(['handler' => $handler]); + $client->get('https://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]); + } + + /** + * @testWith ["digest"] + * ["ntlm"] + */ + public function testRemoveCurlAuthorizationOptionsOnRedirectCrossSchemeSamePort($auth) + { + if (!defined('\CURLOPT_HTTPAUTH')) { + self::markTestSkipped('ext-curl is required for this test'); + } + + $mock = new MockHandler([ + new Response(302, ['Location' => 'http://example.com:80?a=b']), + static function (RequestInterface $request, $options) { + self::assertFalse( + isset($options['curl'][\CURLOPT_HTTPAUTH]), + 'curl options still contain CURLOPT_HTTPAUTH entry' + ); + self::assertFalse( + isset($options['curl'][\CURLOPT_USERPWD]), + 'curl options still contain CURLOPT_USERPWD entry' + ); + return new Response(200); + } + ]); + $handler = HandlerStack::create($mock); + $client = new Client(['handler' => $handler]); + $client->get('https://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]); + } + + /** + * @testWith ["digest"] + * ["ntlm"] + */ + public function testNotRemoveCurlAuthorizationOptionsOnRedirect($auth) + { + if (!defined('\CURLOPT_HTTPAUTH') || !defined('\CURLOPT_USERPWD')) { + self::markTestSkipped('ext-curl is required for this test'); + } + + $mock = new MockHandler([ + new Response(302, ['Location' => 'http://example.com/2']), + static function (RequestInterface $request, $options) { + self::assertTrue( + isset($options['curl'][\CURLOPT_HTTPAUTH]), + 'curl options does not contain expected CURLOPT_HTTPAUTH entry' + ); + self::assertTrue( + isset($options['curl'][\CURLOPT_USERPWD]), + 'curl options does not contain expected CURLOPT_USERPWD entry' + ); + return new Response(200); + } + ]); + $handler = HandlerStack::create($mock); + $client = new Client(['handler' => $handler]); + $client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]); + } + 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], + ['http://example.com/123', 'http://example.com/', false], + ['http://example.com/123', 'http://example.com:80/', false], + ['http://example.com:80/123', 'http://example.com/', false], + ['http://example.com:80/123', 'http://example.com:80/', false], + ['http://example.com/123', 'https://example.com/', true], + ['http://example.com/123', 'http://www.example.com/', true], + ['http://example.com/123', 'http://example.com:81/', true], + ['http://example.com:80/123', 'http://example.com:81/', true], + ['https://example.com/123', 'https://example.com/', false], + ['https://example.com/123', 'https://example.com:443/', false], + ['https://example.com:443/123', 'https://example.com/', false], + ['https://example.com:443/123', 'https://example.com:443/', false], + ['https://example.com/123', 'http://example.com/', true], + ['https://example.com/123', 'https://www.example.com/', true], + ['https://example.com/123', 'https://example.com:444/', true], + ['https://example.com:443/123', 'https://example.com:444/', true], ]; } /** * @dataProvider crossOriginRedirectProvider */ - public function testHeadersTreatmentOnRedirect($originalUri, $targetUri, $shouldBePresent) + public function testHeadersTreatmentOnRedirect($originalUri, $targetUri, $isCrossOrigin) { $mock = new MockHandler([ new Response(302, ['Location' => $targetUri]), - static function (RequestInterface $request) use ($shouldBePresent) { - self::assertSame($shouldBePresent, $request->hasHeader('Authorization')); - self::assertSame($shouldBePresent, $request->hasHeader('Cookie')); + static function (RequestInterface $request) use ($isCrossOrigin) { + self::assertSame(!$isCrossOrigin, $request->hasHeader('Authorization')); + self::assertSame(!$isCrossOrigin, $request->hasHeader('Cookie')); return new Response(200); }