From 918e4ff2b7b4970c161311d5d1aab233d9b8ddce Mon Sep 17 00:00:00 2001 From: Sean O'Brien <60306702+stobrien89@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:29:02 -0500 Subject: [PATCH] bugfix: rest path resolution (#3032) Co-authored-by: Sean O'Brien --- .changes/nextrelease/rest-serializer-fix.json | 7 +++ features/smoke/geoplaces.feature | 8 +++ src/Api/Serializer/RestSerializer.php | 47 +++++++++++---- .../Api/Serializer/RestJsonSerializerTest.php | 59 ++++++++++++++++++- tests/EndpointV2/EndpointProviderV2Test.php | 6 ++ 5 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 .changes/nextrelease/rest-serializer-fix.json create mode 100644 features/smoke/geoplaces.feature diff --git a/.changes/nextrelease/rest-serializer-fix.json b/.changes/nextrelease/rest-serializer-fix.json new file mode 100644 index 0000000000..b05ed451ea --- /dev/null +++ b/.changes/nextrelease/rest-serializer-fix.json @@ -0,0 +1,7 @@ +[ + { + "type": "bugfix", + "category": "Api", + "description": "Fixes issue with path resolution in rest protocol services" + } +] diff --git a/features/smoke/geoplaces.feature b/features/smoke/geoplaces.feature new file mode 100644 index 0000000000..956b767a66 --- /dev/null +++ b/features/smoke/geoplaces.feature @@ -0,0 +1,8 @@ +# language: en +@smoke @geoplaces +Feature: Amazon Location Service Places V2 + + Scenario: Handling errors + When I attempt to call the "GetPlace" API with: + | PlaceId | foo | + Then I expect the response error code to be "ValidationException" diff --git a/src/Api/Serializer/RestSerializer.php b/src/Api/Serializer/RestSerializer.php index ee64d92038..b13e1de4cb 100644 --- a/src/Api/Serializer/RestSerializer.php +++ b/src/Api/Serializer/RestSerializer.php @@ -8,7 +8,6 @@ use Aws\Api\StructureShape; use Aws\Api\TimestampShape; use Aws\CommandInterface; -use Aws\EndpointV2\EndpointProviderV2; use Aws\EndpointV2\EndpointV2SerializerTrait; use Aws\EndpointV2\Ruleset\RulesetEndpoint; use GuzzleHttp\Psr7; @@ -43,7 +42,6 @@ public function __construct(Service $api, $endpoint) /** * @param CommandInterface $command Command to serialize into a request. - * @param $endpointProvider Provider used for dynamic endpoint resolution. * @param $clientArgs Client arguments used for dynamic endpoint resolution. * * @return RequestInterface @@ -198,6 +196,7 @@ private function applyQuery($name, Shape $member, $value, array &$opts) private function buildEndpoint(Operation $operation, array $args, array $opts) { + $isModifiedModel = $this->api->isModifiedModel(); // Create an associative array of variable definitions used in expansions $varDefinitions = $this->getVarDefinitions($operation, $args); @@ -226,11 +225,7 @@ function (array $matches) use ($varDefinitions) { $path = $this->endpoint->getPath(); - //Accounts for trailing '/' in path when custom endpoint - //is provided to endpointProviderV2 - if ($this->api->isModifiedModel() - && $this->api->getServiceName() === 's3' - ) { + if ($isModifiedModel && $this->api->getServiceName() === 's3') { if (substr($path, -1) === '/' && $relative[0] === '/') { $path = rtrim($path, '/'); } @@ -246,6 +241,11 @@ function (array $matches) use ($varDefinitions) { return new Uri($this->endpoint->withPath('') . $relative); } } + + if (!$isModifiedModel) { + $relative = $this->prependPath($relative, $path); + } + // If endpoint has path, remove leading '/' to preserve URI resolution. if ($path && $relative[0] === '/') { $relative = substr($relative, 1); @@ -253,9 +253,7 @@ function (array $matches) use ($varDefinitions) { //Append path to endpoint when leading '//...' // present as uri cannot be properly resolved - if ($this->api->isModifiedModel() - && strpos($relative, '//') === 0 - ) { + if ($isModifiedModel && strpos($relative, '//') === 0) { return new Uri($this->endpoint . $relative); } @@ -307,4 +305,33 @@ private function getVarDefinitions($command, $args) } return $varDefinitions; } + + /** + * If non-empty path with at least one segment present, compare + * with relative and prepend if starting segments are not duplicated + * + * @param string $relative + * @param string $path + * + * @return string + */ + private function prependPath(string $relative, string $path): string + { + if (empty($relative) || $relative === '/' + || empty($path) || $path === '/' + ) { + return $relative; + } + + $normalizedPath = rtrim($path, '/'); + $normalizedRelative = ltrim($relative, '/'); + + // Check if $relative starts with $path + if (strpos($normalizedRelative, ltrim($normalizedPath, '/')) === 0) { + // $relative already starts with $path, return $relative + return $relative; + } + + return $normalizedPath . '/' . $normalizedRelative; + } } diff --git a/tests/Api/Serializer/RestJsonSerializerTest.php b/tests/Api/Serializer/RestJsonSerializerTest.php index 3f6ea6c33f..0660fde503 100644 --- a/tests/Api/Serializer/RestJsonSerializerTest.php +++ b/tests/Api/Serializer/RestJsonSerializerTest.php @@ -59,6 +59,13 @@ private function getTestService() 'boolHeader' => [ 'http' => ['method' => 'POST'], 'input' => ['shape' => 'BoolHeaderInput'] + ], + 'requestUriOperation' =>[ + 'http' => [ + 'method' => 'POST', + 'requestUri' => 'foo/{PathSegment}' + ], + 'input' => ['shape' => 'RequestUriOperationInput'], ] ], 'shapes' => [ @@ -76,6 +83,17 @@ private function getTestService() ] ] ], + 'RequestUriOperationInput' => [ + 'required' => ['PathSegment'], + 'type' => 'structure', + 'members' => [ + "PathSegment" => [ + "shape" => "PathSegmentShape", + "location" => 'uri' + ], + 'baz' => ['shape' => 'BazShape'] + ] + ], "DocumentType" => [ "type" => "structure", "document" => true @@ -139,6 +157,7 @@ private function getTestService() 'BlobShape' => ['type' => 'blob'], 'BazShape' => ['type' => 'string'], 'BoolShape' => ['type' => 'boolean'], + 'PathSegmentShape' => ['type' => 'string'], ] ], function () {} @@ -153,11 +172,12 @@ private function getRequest($commandName, $input) return $j($command); } - private function getPathEndpointRequest($commandName, $input) + private function getPathEndpointRequest($commandName, $input, $options = []) { $service = $this->getTestService(); $command = new Command($commandName, $input); - $j = new RestJsonSerializer($service, 'http://foo.com/bar'); + $path = $options['path'] ?? 'bar'; + $j = new RestJsonSerializer($service, 'http://foo.com/' . $path); return $j($command); } @@ -185,6 +205,41 @@ public function testPreparesRequestsWithEndpointWithPath() ); } + public function testPreparesRequestsWithEndpointWithRequestUriAndPath(): void + { + $request = $this->getPathEndpointRequest( + 'requestUriOperation', + ['PathSegment' => 'bar', 'baz' => 'bar'] + ); + $this->assertSame('POST', $request->getMethod()); + $this->assertSame('http://foo.com/bar/foo/bar', (string) $request->getUri()); + $this->assertSame('{"baz":"bar"}', (string) $request->getBody()); + $this->assertSame( + 'application/json', + $request->getHeaderLine('Content-Type') + ); + } + + /** + * Simulates a custom endpoint provided with a starting path segment matching the + * modeled `RequestUri` starting path segment + */ + public function testPreparesRequestsWithEndpointWithDuplicateRequestUriAndPath(): void + { + $request = $this->getPathEndpointRequest( + 'requestUriOperation', + ['PathSegment' => 'bar', 'baz' => 'bar'], + ['path' => 'foo'] + ); + $this->assertSame('POST', $request->getMethod()); + $this->assertSame('http://foo.com/foo/bar', (string) $request->getUri()); + $this->assertSame('{"baz":"bar"}', (string) $request->getBody()); + $this->assertSame( + 'application/json', + $request->getHeaderLine('Content-Type') + ); + } + public function testPreparesRequestsWithBlobButNoForcedContentType() { $request = $this->getRequest('bar', ['baz' => 'bar']); diff --git a/tests/EndpointV2/EndpointProviderV2Test.php b/tests/EndpointV2/EndpointProviderV2Test.php index c0b4f91f38..ed9badfe1d 100644 --- a/tests/EndpointV2/EndpointProviderV2Test.php +++ b/tests/EndpointV2/EndpointProviderV2Test.php @@ -262,11 +262,17 @@ public function testRulesetProtocolEndpointAndErrorCases($service, $clientArgs, $list->appendSign(Middleware::tap(function($cmd, $req) use ($service, $expected) { $expectedEndpoint = $expected['endpoint']; $expectedUri = new Uri($expected['endpoint']['url']); + $expectedPath = $expectedUri->getPath(); + $this->assertStringContainsString( $expectedUri->getHost(), $req->getUri()->getHost() ); + if (!empty($expectedPath)) { + $this->assertStringStartsWith($expectedPath, $req->getUri()->getPath()); + } + if (isset($expectedEndpoint['properties']['authSchemes'])) { $expectedAuthScheme = null; foreach ($expectedEndpoint['properties']['authSchemes'] as $authScheme) {