diff --git a/.changes/nextrelease/s3-locationconstraint.json b/.changes/nextrelease/s3-locationconstraint.json new file mode 100644 index 0000000000..f4b66e1d68 --- /dev/null +++ b/.changes/nextrelease/s3-locationconstraint.json @@ -0,0 +1,7 @@ +[ + { + "type": "bugfix", + "category": "S3", + "description": "Updates location constraint middleware to exclude directory buckets and retain original configuration." + } +] \ No newline at end of file diff --git a/src/S3/S3Client.php b/src/S3/S3Client.php index bcd0bbcc6f..7b4dd4be33 100644 --- a/src/S3/S3Client.php +++ b/src/S3/S3Client.php @@ -234,6 +234,8 @@ */ class S3Client extends AwsClient implements S3ClientInterface { + public const DIRECTORY_BUCKET_REGEX = '/^[a-zA-Z0-9_-]+--[a-z0-9]+-az\d+--x-s3' + .'(?!.*(?:-s3alias|--ol-s3|\.mrap))$/'; use S3ClientTrait; /** @var array */ @@ -574,14 +576,20 @@ private function getLocationConstraintMiddleware() $region = $this->getRegion(); return static function (callable $handler) use ($region) { return function (Command $command, $request = null) use ($handler, $region) { - if ($command->getName() === 'CreateBucket') { + if ($command->getName() === 'CreateBucket' + && !self::isDirectoryBucket($command['Bucket']) + ) { $locationConstraint = $command['CreateBucketConfiguration']['LocationConstraint'] ?? null; if ($locationConstraint === 'us-east-1') { unset($command['CreateBucketConfiguration']); } elseif ('us-east-1' !== $region && empty($locationConstraint)) { - $command['CreateBucketConfiguration'] = ['LocationConstraint' => $region]; + if(empty($command['CreateBucketConfiguration'])) { + $command['CreateBucketConfiguration'] = ['LocationConstraint' => $region]; + } else { + $command['CreateBucketConfiguration']['LocationConstraint'] = $region; + } } } @@ -858,6 +866,18 @@ private function addBuiltIns($args) $this->clientBuiltIns[$key] = $value; } + /** + * Determines whether a bucket is a directory bucket. + * Only considers the availability zone/suffix format + * + * @param string $bucket + * @return bool + */ + public static function isDirectoryBucket(string $bucket): bool + { + return preg_match(self::DIRECTORY_BUCKET_REGEX, $bucket) === 1; + } + /** @internal */ public static function _applyRetryConfig($value, $args, HandlerList $list) { diff --git a/tests/S3/S3ClientTest.php b/tests/S3/S3ClientTest.php index f5341557c5..ac01d578a2 100644 --- a/tests/S3/S3ClientTest.php +++ b/tests/S3/S3ClientTest.php @@ -511,9 +511,16 @@ public function testProxiesToObjectCopy() public function testAddsLocationConstraintAutomatically($region, $target, $command, $contains) { $client = $this->getTestClient('S3', ['region' => $region]); - $params = ['Bucket' => 'foo']; + $params = [ + 'Bucket' => 'foo', + 'CreateBucketConfiguration' => [ + 'Bucket' => [ + 'Type' => 'foo' + ] + ] + ]; if ($region !== $target) { - $params['CreateBucketConfiguration'] = ['LocationConstraint' => $target]; + $params['CreateBucketConfiguration']['LocationConstraint'] = $target; } $command = $client->getCommand($command, $params); @@ -524,6 +531,10 @@ public function testAddsLocationConstraintAutomatically($region, $target, $comma } else { $this->assertStringNotContainsString($text, $body); } + //ensures original configuration is not overwritten + if ($command === 'CreateBucket') { + $this->assertStringContainsString('foo', $body); + } } public function getTestCasesForLocationConstraints() @@ -537,6 +548,29 @@ public function getTestCasesForLocationConstraints() ]; } + /** + * @dataProvider directoryBucketLocationConstraintProvider + */ + public function testDoesNotAddLocationConstraintForDirectoryBuckets($bucket) + { + $client = $this->getTestClient('s3'); + $params = ['Bucket' => $bucket]; + $command = $client->getCommand('CreateBucket', $params); + $body = (string) \Aws\serialize($command)->getBody(); + $this->assertStringNotContainsString('LocationConstraint', $body); + } + + public function directoryBucketLocationConstraintProvider() + { + return [ + ['bucket-base-name--usw2-az1--x-s3'], + ['mybucket123--euw1-az2--x-s3'], + ['test_bucket_name--apne1-az3--x-s3'], + ['valid-name--aps1-az4--x-s3'], + ['s3_express_demo_directory_bucket--usw2-az1--x-s3'] + ]; + } + public function testSaveAsParamAddsSink() { $client = $this->getTestClient('S3', [ @@ -2491,4 +2525,35 @@ public function testS3RetriesOnNotParsableBody(array $retrySettings) $client->listBuckets(); $this->assertEquals(0, $retries); } + + /** + * @param string $bucketName + * @param bool $expected + * @return void + * + * @dataProvider directoryBucketProvider + */ + public function testIsDirectoryBucket(string $bucketName, bool $expected): void + { + $client = $this->getTestClient('s3'); + $this->assertEquals($expected, $client::isDirectoryBucket($bucketName)); + } + + public function directoryBucketProvider() + { + return [ + ['bucket-base-name--usw2-az1--x-s3', true], + ['mybucket123--euw1-az2--x-s3', true], + ['test_bucket_name--apne1-az3--x-s3', true], + ['valid-name--aps1-az4--x-s3', true], + ['s3_express_demo_directory_bucket--usw2-az1--x-s3', true], + ['bucket-name--usw2-az1--s3alias', false], // ends with -s3alias + ['bucketname--usw2-az1--ol-s3', false], // ends with --ol-s3 + ['bucketname--usw2-az1.mrap', false], // ends with .mrap + ['invalid_bucket_name--az1--x-s3', false], // missing region in azid + ['name--usw2-az1', false], // missing --x-s3 at the end + ['wrong-format--usw2az1--x-s3', false], // missing hyphen in azid part + ['too-short--x-s3', false], // invalid azid format, missing prefix + ]; + } }