Skip to content

Commit

Permalink
Bugfix: S3 region fix (#2866)
Browse files Browse the repository at this point in the history
  • Loading branch information
stobrien89 authored Jan 25, 2024
1 parent ad1f7be commit 8b34d0a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 4 deletions.
7 changes: 7 additions & 0 deletions .changes/nextrelease/s3-region-fix.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"type": "bugfix",
"category": "S3",
"description": "Fixes warning in S3 client when region is not provided in constructor"
}
]
11 changes: 10 additions & 1 deletion src/S3/S3Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -801,9 +801,18 @@ private function processEndpointV2Model()
*/
private function addBuiltIns($args)
{
if ($args['region'] !== 'us-east-1') {
if (isset($args['region'])
&& $args['region'] !== 'us-east-1'
) {
return false;
}

if (!isset($args['region'])
&& ConfigurationResolver::resolve('region', '', 'string') !== 'us-east-1'
) {
return false;
}

$key = 'AWS::S3::UseGlobalEndpoint';
$result = $args['s3_us_east_1_regional_endpoint'] instanceof \Closure ?
$args['s3_us_east_1_regional_endpoint']()->wait() : $args['s3_us_east_1_regional_endpoint'];
Expand Down
34 changes: 31 additions & 3 deletions tests/S3/S3ClientTest.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<?php
namespace Aws\Test\S3;

use Aws\Arn\ArnParser;
use Aws\Command;
use Aws\CommandInterface;
use Aws\Exception\AwsException;
Expand All @@ -15,7 +14,6 @@
use Aws\S3\RegionalEndpoint\Configuration;
use Aws\S3\S3Client;
use Aws\S3\UseArnRegion\Configuration as UseArnRegionConfiguration;
use Aws\Signature\SignatureV4;
use Aws\Test\UsesServiceTrait;
use GuzzleHttp\Exception\ConnectException;
use GuzzleHttp\Exception\RequestException;
Expand All @@ -25,7 +23,6 @@
use GuzzleHttp\Psr7;
use GuzzleHttp\Psr7\Response;
use GuzzleHttp\Psr7\Uri;
use http\Exception\InvalidArgumentException;
use Psr\Http\Message\RequestInterface;
use Yoast\PHPUnitPolyfills\TestCases\TestCase;
use Aws\Exception\UnresolvedEndpointException;
Expand Down Expand Up @@ -2476,4 +2473,35 @@ public function dotSegmentPathStyleProvider()
];
}

/**
* @dataProvider builtinRegionProvider
*/
public function testCorrectlyResolvesGlobalEndpointWithoutRegionInConstructor(
$region, $expected
){
putenv('AWS_REGION=' . $region);

$s3Client = new S3Client([]);
$builtIns = $s3Client->getClientBuiltIns();
//The UseGlobalEndpoint builtin should be set by default if
//the region provided is us-east-1.
$this->assertEquals($expected, isset($builtIns['AWS::S3::UseGlobalEndpoint']));

//When the UseGlobalEndpoint builtin is set (i.e. us-east-1 is the region)
// the default value should be `true`, unless `s3_us_east_1_regional_endpoint`
// is set to `regional`.
if ($expected) {
$this->assertEquals($expected, $builtIns['AWS::S3::UseGlobalEndpoint']);
}

putenv('AWS_REGION=');
}

public function builtinRegionProvider()
{
return [
['us-east-1' , true],
['us-west-2', false]
];
}
}

0 comments on commit 8b34d0a

Please sign in to comment.