Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Release 7.4.5 #3043

Merged
merged 1 commit into from Jun 20, 2022
Merged

Release 7.4.5 #3043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -2,6 +2,11 @@

Please refer to [UPGRADING](UPGRADING.md) guide for upgrading to a major version.

## 7.4.5 - 2022-06-20

* Fix change in port should be considered a change in origin
* Fix `CURLOPT_HTTPAUTH` option not cleared on change of origin

## 7.4.4 - 2022-06-09

* Fix failure to strip Authorization header on HTTP downgrade
Expand Down
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ We use GitHub issues only to discuss bugs and new features. For support please r

- [Documentation](https://docs.guzzlephp.org)
- [Stack Overflow](https://stackoverflow.com/questions/tagged/guzzle)
- [#guzzle](https://app.slack.com/client/T0D2S9JCT/CE6UAAKL4) channel on [PHP-HTTP Slack](http://slack.httplug.io/)
- [#guzzle](https://app.slack.com/client/T0D2S9JCT/CE6UAAKL4) channel on [PHP-HTTP Slack](https://slack.httplug.io/)
- [Gitter](https://gitter.im/guzzle/guzzle)


Expand Down Expand Up @@ -73,10 +73,10 @@ composer require guzzlehttp/guzzle
[guzzle-5-repo]: https://github.com/guzzle/guzzle/tree/5.3
[guzzle-6-repo]: https://github.com/guzzle/guzzle/tree/6.5
[guzzle-7-repo]: https://github.com/guzzle/guzzle
[guzzle-3-docs]: http://guzzle3.readthedocs.org
[guzzle-5-docs]: http://docs.guzzlephp.org/en/5.3/
[guzzle-6-docs]: http://docs.guzzlephp.org/en/6.5/
[guzzle-7-docs]: http://docs.guzzlephp.org/en/latest/
[guzzle-3-docs]: https://guzzle3.readthedocs.io/
[guzzle-5-docs]: https://docs.guzzlephp.org/en/5.3/
[guzzle-6-docs]: https://docs.guzzlephp.org/en/6.5/
[guzzle-7-docs]: https://docs.guzzlephp.org/en/latest/


## Security
Expand Down
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Expand Up @@ -54,7 +54,7 @@
"php": "^7.2.5 || ^8.0",
"ext-json": "*",
"guzzlehttp/promises": "^1.5",
"guzzlehttp/psr7": "^1.8.3 || ^2.1",
"guzzlehttp/psr7": "^1.9 || ^2.4",
"psr/http-client": "^1.0",
"symfony/deprecation-contracts": "^2.2 || ^3.0"
},
Expand Down
33 changes: 4 additions & 29 deletions src/RedirectMiddleware.php
View file
Open in desktop
Expand Up @@ -88,10 +88,8 @@ 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 host is different.
if ($request->getUri()->getHost() !== $nextRequest->getUri()->getHost()
&& defined('\CURLOPT_HTTPAUTH')
) {
// 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]
Expand Down Expand Up @@ -198,38 +196,15 @@ 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';
}

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.
*/
Expand Down
149 changes: 122 additions & 27 deletions tests/RedirectMiddlewareTest.php
View file
Open in desktop
Expand Up @@ -272,65 +272,105 @@ public function testInvokesOnRedirectForRedirects()
self::assertTrue($call);
}

public function crossOriginRedirectProvider()
/**
* @testWith ["digest"]
* ["ntlm"]
*/
public function testRemoveCurlAuthorizationOptionsOnRedirectCrossHost($auth)
{
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],
];
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]]);
}

/**
* @dataProvider crossOriginRedirectProvider
* @testWith ["digest"]
* ["ntlm"]
*/
public function testHeadersTreatmentOnRedirect($originalUri, $targetUri, $shouldBePresent)
public function testRemoveCurlAuthorizationOptionsOnRedirectCrossPort($auth)
{
$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'));
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($originalUri, ['auth' => ['testuser', 'testpass'], 'headers' => ['Cookie' => 'foo=bar']]);
$client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]);
}

public function testNotRemoveAuthorizationHeaderOnRedirect()
/**
* @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/2']),
static function (RequestInterface $request) {
self::assertTrue($request->hasHeader('Authorization'));
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('http://example.com?a=b', ['auth' => ['testuser', 'testpass']]);
$client->get('https://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]);
}

/**
* @testWith ["digest"]
* ["ntlm"]
*/
public function testRemoveCurlAuthorizationOptionsOnRedirect($auth)
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://test.com']),
new Response(302, ['Location' => 'http://example.com:80?a=b']),
static function (RequestInterface $request, $options) {
self::assertFalse(
isset($options['curl'][\CURLOPT_HTTPAUTH]),
Expand All @@ -345,7 +385,7 @@ static function (RequestInterface $request, $options) {
]);
$handler = HandlerStack::create($mock);
$client = new Client(['handler' => $handler]);
$client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]);
$client->get('https://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]);
}

/**
Expand Down Expand Up @@ -377,6 +417,61 @@ static function (RequestInterface $request, $options) {
$client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass', $auth]]);
}

public function crossOriginRedirectProvider()
{
return [
['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, $isCrossOrigin)
{
$mock = new MockHandler([
new Response(302, ['Location' => $targetUri]),
static function (RequestInterface $request) use ($isCrossOrigin) {
self::assertSame(!$isCrossOrigin, $request->hasHeader('Authorization'));
self::assertSame(!$isCrossOrigin, $request->hasHeader('Cookie'));

return new Response(200);
}
]);
$handler = HandlerStack::create($mock);
$client = new Client(['handler' => $handler]);
$client->get($originalUri, ['auth' => ['testuser', 'testpass'], 'headers' => ['Cookie' => 'foo=bar']]);
}

public function testNotRemoveAuthorizationHeaderOnRedirect()
{
$mock = new MockHandler([
new Response(302, ['Location' => 'http://example.com/2']),
static function (RequestInterface $request) {
self::assertTrue($request->hasHeader('Authorization'));
return new Response(200);
}
]);
$handler = HandlerStack::create($mock);
$client = new Client(['handler' => $handler]);
$client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass']]);
}

/**
* Verifies how RedirectMiddleware::modifyRequest() modifies the method and body of a request issued when
* encountering a redirect response.
Expand Down