From 971e6f319a29305d889599c215813b7c56594ea1 Mon Sep 17 00:00:00 2001 From: Sean O'Brien <60306702+stobrien89@users.noreply.github.com> Date: Tue, 6 Aug 2024 15:21:27 -0400 Subject: [PATCH] feat: s3 expires customization (#2955) --- .../nextrelease/expires-customization.json | 7 ++ src/Api/Service.php | 3 +- src/InputValidationMiddleware.php | 1 - src/S3/ExpiresParsingMiddleware.php | 56 +++++++++++++ src/S3/S3Client.php | 69 +++++++++++++--- tests/Api/ServiceTest.php | 26 ++++++ tests/S3/ExpiresParsingMiddlewareTest.php | 56 +++++++++++++ tests/S3/S3ClientTest.php | 81 +++++++++++++++++++ 8 files changed, 286 insertions(+), 13 deletions(-) create mode 100644 .changes/nextrelease/expires-customization.json create mode 100644 src/S3/ExpiresParsingMiddleware.php create mode 100644 tests/S3/ExpiresParsingMiddlewareTest.php diff --git a/.changes/nextrelease/expires-customization.json b/.changes/nextrelease/expires-customization.json new file mode 100644 index 0000000000..2761e2a8ec --- /dev/null +++ b/.changes/nextrelease/expires-customization.json @@ -0,0 +1,7 @@ +[ + { + "type": "feature", + "category": "S3", + "description": "Adds customization to output structures for `Expires` parsing which adds an additional shape `ExpiresString`" + } +] diff --git a/src/Api/Service.php b/src/Api/Service.php index 0945d92faa..52cc731776 100644 --- a/src/Api/Service.php +++ b/src/Api/Service.php @@ -284,7 +284,7 @@ public function getOperation($name) $this->definition['operations'][$name], $this->shapeMap ); - } else if ($this->modifiedModel) { + } elseif ($this->modifiedModel) { $this->operations[$name] = new Operation( $this->definition['operations'][$name], $this->shapeMap @@ -517,6 +517,7 @@ public function getDefinition() public function setDefinition($definition) { $this->definition = $definition; + $this->shapeMap = new ShapeMap($definition['shapes']); $this->modifiedModel = true; } diff --git a/src/InputValidationMiddleware.php b/src/InputValidationMiddleware.php index bd46582df2..f1933d6dfb 100644 --- a/src/InputValidationMiddleware.php +++ b/src/InputValidationMiddleware.php @@ -72,5 +72,4 @@ public function __invoke(CommandInterface $cmd) { } return $nextHandler($cmd); } - } diff --git a/src/S3/ExpiresParsingMiddleware.php b/src/S3/ExpiresParsingMiddleware.php new file mode 100644 index 0000000000..5ceccd1eaa --- /dev/null +++ b/src/S3/ExpiresParsingMiddleware.php @@ -0,0 +1,56 @@ +nextHandler = $nextHandler; + } + + public function __invoke(CommandInterface $command, RequestInterface $request = null) + { + $next = $this->nextHandler; + return $next($command, $request)->then( + function (ResultInterface $result) { + if (empty($result['Expires']) && !empty($result['ExpiresString'])) { + trigger_error( + "Failed to parse the `expires` header as a timestamp due to " + . " an invalid timestamp format.\nPlease refer to `ExpiresString` " + . "for the unparsed string format of this header.\n" + , E_USER_WARNING + ); + } + return $result; + } + ); + } +} diff --git a/src/S3/S3Client.php b/src/S3/S3Client.php index 235bac3b4c..2948802ff2 100644 --- a/src/S3/S3Client.php +++ b/src/S3/S3Client.php @@ -437,6 +437,7 @@ public function __construct(array $args) InputValidationMiddleware::wrap($this->getApi(), self::$mandatoryAttributes), 'input_validation_middleware' ); + $stack->appendSign(ExpiresParsingMiddleware::wrap(), 's3.expires_parsing'); $stack->appendSign(PutObjectUrlMiddleware::wrap(), 's3.put_object_url'); $stack->appendSign(PermanentRedirectMiddleware::wrap(), 's3.permanent_redirect'); $stack->appendInit(Middleware::sourceFile($this->getApi()), 's3.source_file'); @@ -444,8 +445,8 @@ public function __construct(array $args) $stack->appendInit($this->getLocationConstraintMiddleware(), 's3.location'); $stack->appendInit($this->getEncodingTypeMiddleware(), 's3.auto_encode'); $stack->appendInit($this->getHeadObjectMiddleware(), 's3.head_object'); + $this->processModel($this->isUseEndpointV2()); if ($this->isUseEndpointV2()) { - $this->processEndpointV2Model(); $stack->after('builder', 's3.check_empty_path_with_query', $this->getEmptyPathWithQuery()); @@ -763,28 +764,51 @@ public static function _default_s3_express_identity_provider(array $args) } /** - * Modifies API definition to remove `Bucket` from request URIs. + * If EndpointProviderV2 is used, removes `Bucket` from request URIs. * This is now handled by the endpoint ruleset. * + * Additionally adds a synthetic shape `ExpiresString` and modifies + * `Expires` type to ensure it remains set to `timestamp`. + * + * @param array $args * @return void * * @internal */ - private function processEndpointV2Model() + private function processModel(bool $isUseEndpointV2): void { $definition = $this->getApi()->getDefinition(); - foreach($definition['operations'] as &$operation) { - if (isset($operation['http']['requestUri'])) { - $requestUri = $operation['http']['requestUri']; - if ($requestUri === "/{Bucket}") { - $requestUri = str_replace('/{Bucket}', '/', $requestUri); - } else { - $requestUri = str_replace('/{Bucket}', '', $requestUri); + if ($isUseEndpointV2) { + foreach($definition['operations'] as &$operation) { + if (isset($operation['http']['requestUri'])) { + $requestUri = $operation['http']['requestUri']; + if ($requestUri === "/{Bucket}") { + $requestUri = str_replace('/{Bucket}', '/', $requestUri); + } else { + $requestUri = str_replace('/{Bucket}', '', $requestUri); + } + $operation['http']['requestUri'] = $requestUri; + } + } + } + + foreach ($definition['shapes'] as $key => &$value) { + $suffix = 'Output'; + if (substr($key, -strlen($suffix)) === $suffix) { + if (isset($value['members']['Expires'])) { + $value['members']['Expires']['deprecated'] = true; + $value['members']['ExpiresString'] = [ + 'shape' => 'ExpiresString', + 'location' => 'header', + 'locationName' => 'Expires' + ]; } - $operation['http']['requestUri'] = $requestUri; } } + $definition['shapes']['ExpiresString']['type'] = 'string'; + $definition['shapes']['Expires']['type'] = 'timestamp'; + $this->getApi()->setDefinition($definition); } @@ -1035,6 +1059,29 @@ public static function applyDocFilters(array $api, array $docs) 'shapes' => ['PutObjectRequest', 'UploadPartRequest'] ]; + // Add `ExpiresString` shape to output structures which contain `Expires` + // Deprecate existing `Expires` shapes in output structures + // Add/Update documentation for both `ExpiresString` and `Expires` + // Ensure `Expires` type remains timestamp + foreach ($api['shapes'] as $key => &$value) { + $suffix = 'Output'; + if (substr($key, -strlen($suffix)) === $suffix) { + if (isset($value['members']['Expires'])) { + $value['members']['Expires']['deprecated'] = true; + $value['members']['ExpiresString'] = [ + 'shape' => 'ExpiresString', + 'location' => 'header', + 'locationName' => 'Expires' + ]; + $docs['shapes']['Expires']['refs'][$key . '$Expires'] + .= '

This output shape has been deprecated. Please refer to ExpiresString instead.

.'; + } + } + } + $api['shapes']['ExpiresString']['type'] = 'string'; + $docs['shapes']['ExpiresString']['base'] = 'The unparsed string value of the Expires output member.'; + $api['shapes']['Expires']['type'] = 'timestamp'; + return [ new Service($api, ApiProvider::defaultProvider()), new DocModel($docs) diff --git a/tests/Api/ServiceTest.php b/tests/Api/ServiceTest.php index 78c9b67bd6..08c9b187c6 100644 --- a/tests/Api/ServiceTest.php +++ b/tests/Api/ServiceTest.php @@ -269,14 +269,40 @@ public function testModifyModel() 'signingName' => 'qux', 'protocol' => 'yak', 'uid' => 'foo-2016-12-09' + ], + 'operations' => [ + 'FooOperation' => [ + 'name' => 'FooOperation', + 'output' => [ + 'shape' => 'FooOperationOutput' + ] + ] + ], + 'shapes' => [ + 'FooOperationOutput' => [ + 'type' => 'structure', + 'members' => [ + 'Expires' => [ + 'shape' => 'Expires', + ] + ] + ], + 'Expires' => [ + 'type' => 'string' + ] ] ], function () { return []; } ); $definition = $s->getDefinition(); $definition['metadata']['serviceId'] = 'bar'; + $definition['shapes']['Expires']['type'] = 'timestamp'; $s->setDefinition($definition); $this->assertTrue($s->isModifiedModel()); $this->assertEquals( 'bar', $s->getMetadata('serviceId')); + $this->assertEquals( + 'timestamp', + $s->getOperation('FooOperation')->getOutput()->getMember('Expires')->getType() + ); } } diff --git a/tests/S3/ExpiresParsingMiddlewareTest.php b/tests/S3/ExpiresParsingMiddlewareTest.php new file mode 100644 index 0000000000..6b1aaa6e99 --- /dev/null +++ b/tests/S3/ExpiresParsingMiddlewareTest.php @@ -0,0 +1,56 @@ +expectWarning(); + $this->expectWarningMessage( + "Failed to parse the `expires` header as a timestamp due to " + . " an invalid timestamp format.\nPlease refer to `ExpiresString` " + . "for the unparsed string format of this header.\n" + ); + + $command = $this->getMockBuilder(CommandInterface::class)->getMock(); + $request = $this->getMockBuilder(RequestInterface::class)->getMock(); + $nextHandler = function ($cmd, $request) { + return Promise\Create::promiseFor(new Result([ + 'ExpiresString' => 'not-a-timestamp' + ])); + }; + + $mw = new ExpiresParsingMiddleware($nextHandler); + $mw($command, $request)->wait(); + } + + public function testDoesNotEmitWarningWhenExpiresPresent() + { + $command = $this->getMockBuilder(CommandInterface::class)->getMock(); + $request = $this->getMockBuilder(RequestInterface::class)->getMock(); + $nextHandler = function ($cmd, $request) { + return Promise\Create::promiseFor(new Result([ + 'ExpiresString' => 'test', + 'Expires' => 'test' + ])); + }; + + $mw = new ExpiresParsingMiddleware($nextHandler); + $result = $mw($command, $request)->wait(); + $this->assertEquals('test', $result['Expires']); + $this->assertEquals('test', $result['ExpiresString']); + } +} diff --git a/tests/S3/S3ClientTest.php b/tests/S3/S3ClientTest.php index 7eddeebfc6..d78ca73250 100644 --- a/tests/S3/S3ClientTest.php +++ b/tests/S3/S3ClientTest.php @@ -1,6 +1,7 @@ 'us-east-1', + 'http_handler' => function (RequestInterface $request) { + return Promise\Create::promiseFor(new Response( + 200, + ['expires' => '1989-08-05'] + )); + }, + ]); + $result = $client->headObject(['Bucket' => 'foo', 'Key' => 'bar']); + $this->assertInstanceOf(DateTimeResult::class, $result['Expires']); + $this->assertEquals('1989-08-05', $result['ExpiresString']); + } + + public function testEmitsWarningWhenExpiresUnparseable() + { + $this->expectWarning(); + $this->expectWarningMessage( + "Failed to parse the `expires` header as a timestamp due to " + . " an invalid timestamp format.\nPlease refer to `ExpiresString` " + . "for the unparsed string format of this header.\n" + ); + + $client = new S3Client([ + 'region' => 'us-east-1', + 'http_handler' => function (RequestInterface $request) { + return Promise\Create::promiseFor(new Response( + 200, + ['expires' => 'this-is-not-a-timestamp'] + )); + }, + ]); + + $client->headObject(['Bucket' => 'foo', 'Key' => 'bar']); + } + + public function testExpiresRemainsTimestamp() { + //S3 will be changing `Expires` type from `timestamp` to `string` + // soon. This test ensures backward compatibility + $apiProvider = static function () { + return [ + 'metadata' => [ + 'signatureVersion' => 'v4', + 'protocol' => 'rest-xml' + ], + 'shapes' => [ + 'Expires' => [ + 'type' => 'string' + ], + ], + ]; + }; + + $s3Client = new S3Client([ + 'region' => 'us-west-2', + 'api_provider' => $apiProvider + ]); + + $api = $s3Client->getApi(); + $expiresType = $api->getDefinition()['shapes']['Expires']['type']; + $this->assertEquals('timestamp', $expiresType); + } + + public function testBucketNotModifiedWithLegacyEndpointProvider() + { + $client = new S3Client([ + 'region' => 'us-west-2', + 'endpoint_provider' => PartitionEndpointProvider::defaultProvider() + ]); + + $operations = $client->getApi()->getDefinition()['operations']; + $this->assertEquals('/{Bucket}', $operations['ListObjects']['http']['requestUri']); + $this->assertEquals( + '/{Bucket}?versions', + $operations['ListObjectVersions']['http']['requestUri'] + ); + } + public function builtinRegionProvider() { return [