From f5bc7e139cca6ece29aced04c5e6e5b1601533ce Mon Sep 17 00:00:00 2001 From: Howard Lopez Date: Tue, 21 May 2019 17:37:07 -0700 Subject: [PATCH 1/4] Add StreamRequestPayloadMiddleware --- src/AwsClient.php | 13 ++ .../IncalculablePayloadException.php | 11 ++ src/StreamRequestPayloadMiddleware.php | 84 ++++++++++ tests/StreamRequestPayloadMiddlewareTest.php | 150 ++++++++++++++++++ 4 files changed, 258 insertions(+) create mode 100644 src/Exception/IncalculablePayloadException.php create mode 100644 src/StreamRequestPayloadMiddleware.php create mode 100644 tests/StreamRequestPayloadMiddlewareTest.php diff --git a/src/AwsClient.php b/src/AwsClient.php index 206b036906..76ebc63f47 100644 --- a/src/AwsClient.php +++ b/src/AwsClient.php @@ -184,6 +184,7 @@ public function __construct(array $args) $this->addClientSideMonitoring($args); $this->addEndpointParameterMiddleware($args); $this->addEndpointDiscoveryMiddleware($config, $args); + $this->addStreamRequestPayload(); if (isset($args['with_resolved'])) { $args['with_resolved']($config); @@ -371,6 +372,18 @@ private function addClientSideMonitoring($args) ); } + private function addStreamRequestPayload() + { + $streamRequestPayloadMiddleware = StreamRequestPayloadMiddleware::wrap( + $this->api + ); + + $this->handlerList->prependSign( + $streamRequestPayloadMiddleware, + 'StreamRequestPayloadMiddleware' + ); + } + /** * Returns a service model and doc model with any necessary changes * applied. diff --git a/src/Exception/IncalculablePayloadException.php b/src/Exception/IncalculablePayloadException.php new file mode 100644 index 0000000000..a64e7428f6 --- /dev/null +++ b/src/Exception/IncalculablePayloadException.php @@ -0,0 +1,11 @@ +nextHandler = $nextHandler; + $this->service = $service; + } + + public function __invoke(CommandInterface $command, RequestInterface $request) + { + $nextHandler = $this->nextHandler; + + $operation = $this->service->getOperation($command->getName()); + $contentLength = $request->getHeader('content-length'); + $hasStreaming = false; + $requiresLength = false; + + // Check if any present input member is a stream and requires the + // content length + foreach ($operation->getInput()->getMembers() as $name => $member) { + if (!empty($member['streaming']) && isset($command[$name])) { + $hasStreaming = true; + if (!empty($member['requiresLength'])) { + $requiresLength = true; + } + } + } + + if ($hasStreaming) { + + // Add 'transfer-encoding' header if payload size not required to + // to be calculated and not already known + if (empty($requiresLength) + && empty($contentLength) + && !empty($operation['v4-unsigned-body']) + ) { + $request = $request->withHeader('transfer-encoding', 'chunked'); + + // Otherwise, make sure 'content-length' header is added + } else { + if (empty($contentLength)) { + $size = $request->getBody()->getSize(); + if (is_null($size)) { + throw new IncalculablePayloadException('Payload' + . 'content lenggth is required and can not be' + . 'calculated.'); + } + $request = $request->withHeader( + 'content-length', + $size + ); + } + } + } + + return $nextHandler($command, $request); + } +} diff --git a/tests/StreamRequestPayloadMiddlewareTest.php b/tests/StreamRequestPayloadMiddlewareTest.php new file mode 100644 index 0000000000..6b217c7986 --- /dev/null +++ b/tests/StreamRequestPayloadMiddlewareTest.php @@ -0,0 +1,150 @@ +generateTestService(); + $client = $this->generateTestClient($service); + $command = $client->getCommand( + 'StreamingOp', + [ + 'InputStream' => Psr7\stream_for('test'), +// 'InputString' => 'some_string' + ] + ); + + $list = $command->getHandlerList(); +// $list = new HandlerList(); + $list->setHandler(function ($command, $request) { +// var_dump($request); + return new Result([]); + }); +// $list->appendSign(StreamRequestPayloadMiddleware::wrap($service)); + + $handler = $list->resolve(); + + $result = $handler($command, new Request('POST', 'https://foo.com')); + } + + private function generateTestClient(Service $service, $args = []) + { + return new AwsClient( + array_merge( + [ + 'service' => 'foo', + 'api_provider' => function () use ($service) { + return $service->toArray(); + }, + 'region' => 'us-east-1', + 'version' => 'latest', + ], + $args + ) + ); + } + + private function generateTestService() + { + return new Service( + [ + 'metadata' => [ + "protocol" => "rest-json", + "apiVersion" => "2014-01-01" + ], + 'shapes' => [ + "BlobLengthStream" => [ + "type" => "blob", + "streaming" => true, + "requiresLength" => true, + ], + "BlobStream" => [ + "type" => "blob", + "streaming" => true, + ], + "StreamingInputShape" => [ + "type" => "structure", + "required" => [ + "InputStream", + ], + "members" => [ + "InputStream" => [ + "shape" => "BlobStream", + ], + "InputString" => [ + "shape" => "StringType", + ], + ], + ], + "StreamingLengthInputShape" => [ + "type" => "structure", + "members" => [ + "InputStream" => [ + "shape" => "BlobLengthStream", + ], + ], + ], + "StringType"=> [ + "type" => "string", + ], + ], + 'operations' => [ + "StreamingOp" => [ + "name"=> "StreamingOp", + "http"=> [ + "method"=> "POST", + "requestUri"=> "/", + "responseCode"=> 200 + ], + "input"=> ["shape"=> "StreamingInputShape"], + ], + "StreamingLengthOp" => [ + "name"=> "StreamingLengthOp", + "http"=> [ + "method"=> "POST", + "requestUri"=> "/", + "responseCode"=> 200 + ], + "input"=> ["shape"=> "StreamingLengthInputShape"], + ], + "StreamingUnsignedOp" => [ + "name"=> "StreamingUnsignedOp", + "http"=> [ + "method"=> "POST", + "requestUri"=> "/", + "responseCode"=> 200 + ], + "input"=> ["shape"=> "StreamingInputShape"], + "authtype" => "v4-unsigned-body", + ], + "StreamingLengthUnsignedOp" => [ + "name"=> "StreamingLengthUnsignedOp", + "http"=> [ + "method"=> "POST", + "requestUri"=> "/", + "responseCode"=> 200 + ], + "input"=> ["shape"=> "StreamingLengthInputShape"], + "authtype" => "v4-unsigned-body", + ], + ], + ], + function () { return []; } + ); + } + +} From 52759d85376bc32d289fe2bc7bd085ed9892e97d Mon Sep 17 00:00:00 2001 From: Howard Lopez Date: Wed, 22 May 2019 15:51:52 -0700 Subject: [PATCH 2/4] Add tests for proper header additions --- src/StreamRequestPayloadMiddleware.php | 3 +- tests/StreamRequestPayloadMiddlewareTest.php | 132 ++++++++++++++++--- 2 files changed, 119 insertions(+), 16 deletions(-) diff --git a/src/StreamRequestPayloadMiddleware.php b/src/StreamRequestPayloadMiddleware.php index 1c6c1616dd..8b46369ec4 100644 --- a/src/StreamRequestPayloadMiddleware.php +++ b/src/StreamRequestPayloadMiddleware.php @@ -58,7 +58,8 @@ public function __invoke(CommandInterface $command, RequestInterface $request) // to be calculated and not already known if (empty($requiresLength) && empty($contentLength) - && !empty($operation['v4-unsigned-body']) + && isset($operation['authtype']) + && $operation['authtype'] == 'v4-unsigned-body' ) { $request = $request->withHeader('transfer-encoding', 'chunked'); diff --git a/tests/StreamRequestPayloadMiddlewareTest.php b/tests/StreamRequestPayloadMiddlewareTest.php index 6b217c7986..d0d96ab7b9 100644 --- a/tests/StreamRequestPayloadMiddlewareTest.php +++ b/tests/StreamRequestPayloadMiddlewareTest.php @@ -3,12 +3,16 @@ use Aws\Api\Service; use Aws\AwsClient; +use Aws\ClientResolver; +use Aws\CommandInterface; use Aws\HandlerList; +use Aws\Middleware; use Aws\Result; use Aws\StreamRequestPayloadMiddleware; use GuzzleHttp\Psr7; use GuzzleHttp\Psr7\Request; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\RequestInterface; /** * @covers \Aws\StreamRequestPayloadMiddleware @@ -16,29 +20,107 @@ class StreamRequestPayloadMiddlewareTest extends TestCase { - public function testAddsProperHeaders() - { + /** + * @dataProvider generateTestCases + * + * @param CommandInterface $command + * @param array $expectedHeaders + * @param array $expectedNonHeaders + */ + public function testAddsProperHeaders( + CommandInterface $command, + array $expectedHeaders, + array $expectedNonHeaders + ) { $service = $this->generateTestService(); - $client = $this->generateTestClient($service); - $command = $client->getCommand( - 'StreamingOp', - [ - 'InputStream' => Psr7\stream_for('test'), -// 'InputString' => 'some_string' - ] + $serializer = ClientResolver::_default_serializer([ + 'api' => $service, + 'endpoint' => '' + ]); + + $list = new HandlerList(); + $list->prependBuild(Middleware::requestBuilder($serializer), 'builder'); + $list->prependSign( + StreamRequestPayloadMiddleware::wrap($service), + 'StreamRequestPayloadMiddleware' ); - $list = $command->getHandlerList(); -// $list = new HandlerList(); - $list->setHandler(function ($command, $request) { -// var_dump($request); + $list->setHandler(function ( + CommandInterface $command, + RequestInterface $request + ) use ( + $expectedHeaders, + $expectedNonHeaders + ) { + $this->assertArraySubset($expectedHeaders, $request->getHeaders()); + foreach ($expectedNonHeaders as $header) { + $this->assertArrayNotHasKey($header, $request->getHeaders()); + } return new Result([]); }); -// $list->appendSign(StreamRequestPayloadMiddleware::wrap($service)); $handler = $list->resolve(); + $handler($command, new Request('POST', 'https://foo.com')); + } + + public function generateTestCases() + { + $service = $this->generateTestService(); + $client = $this->generateTestClient($service); + $inputStream = Psr7\stream_for('test'); - $result = $handler($command, new Request('POST', 'https://foo.com')); + return [ + [ + $client->getCommand( + 'NonStreamingOp', + [ + 'InputString' => 'teststring', + ] + ), + [], + [ 'transfer-encoding', 'content-length' ], + ], + [ + $client->getCommand( + 'StreamingOp', + [ + 'InputStream' => $inputStream, + ] + ), + [ 'content-length' => [26] ], + [ 'transfer-encoding' ], + ], + [ + $client->getCommand( + 'StreamingLengthOp', + [ + 'InputStream' => $inputStream, + ] + ), + [ 'content-length' => [26] ], + [ 'transfer-encoding' ], + ], + [ + $client->getCommand( + 'StreamingUnsignedOp', + [ + 'InputStream' => $inputStream, + ] + ), + [ 'transfer-encoding' => ['chunked'] ], + [ 'content-length' ], + ], + [ + $client->getCommand( + 'StreamingLengthUnsignedOp', + [ + 'InputStream' => $inputStream, + ] + ), + [ 'content-length' => [26] ], + [ 'transfer-encoding' ], + ], + ]; } private function generateTestClient(Service $service, $args = []) @@ -76,6 +158,17 @@ private function generateTestService() "type" => "blob", "streaming" => true, ], + "NonStreamingInputShape" => [ + "type" => "structure", + "required" => [ + "InputString", + ], + "members" => [ + "InputString" => [ + "shape" => "StringType", + ], + ], + ], "StreamingInputShape" => [ "type" => "structure", "required" => [ @@ -103,6 +196,15 @@ private function generateTestService() ], ], 'operations' => [ + "NonStreamingOp" => [ + "name"=> "NonStreamingOp", + "http"=> [ + "method"=> "POST", + "requestUri"=> "/", + "responseCode"=> 200 + ], + "input"=> ["shape"=> "NonStreamingInputShape"], + ], "StreamingOp" => [ "name"=> "StreamingOp", "http"=> [ From 03e5dfbe9d670ded61943477291802723e5fda02 Mon Sep 17 00:00:00 2001 From: Howard Lopez Date: Wed, 22 May 2019 17:24:24 -0700 Subject: [PATCH 3/4] Add test for incalculable payload exception --- src/StreamRequestPayloadMiddleware.php | 4 +- tests/StreamRequestPayloadMiddlewareTest.php | 66 ++++++++++++++++---- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/StreamRequestPayloadMiddleware.php b/src/StreamRequestPayloadMiddleware.php index 8b46369ec4..dd63d3a5f6 100644 --- a/src/StreamRequestPayloadMiddleware.php +++ b/src/StreamRequestPayloadMiddleware.php @@ -69,8 +69,8 @@ public function __invoke(CommandInterface $command, RequestInterface $request) $size = $request->getBody()->getSize(); if (is_null($size)) { throw new IncalculablePayloadException('Payload' - . 'content lenggth is required and can not be' - . 'calculated.'); + . ' content length is required and can not be' + . ' calculated.'); } $request = $request->withHeader( 'content-length', diff --git a/tests/StreamRequestPayloadMiddlewareTest.php b/tests/StreamRequestPayloadMiddlewareTest.php index d0d96ab7b9..b6aa921ecc 100644 --- a/tests/StreamRequestPayloadMiddlewareTest.php +++ b/tests/StreamRequestPayloadMiddlewareTest.php @@ -32,18 +32,7 @@ public function testAddsProperHeaders( array $expectedHeaders, array $expectedNonHeaders ) { - $service = $this->generateTestService(); - $serializer = ClientResolver::_default_serializer([ - 'api' => $service, - 'endpoint' => '' - ]); - - $list = new HandlerList(); - $list->prependBuild(Middleware::requestBuilder($serializer), 'builder'); - $list->prependSign( - StreamRequestPayloadMiddleware::wrap($service), - 'StreamRequestPayloadMiddleware' - ); + $list = $this->generateTestHandlerList(); $list->setHandler(function ( CommandInterface $command, @@ -123,6 +112,59 @@ public function generateTestCases() ]; } + /** + * @expectedException \Aws\Exception\IncalculablePayloadException + * @expectedExceptionMessage Payload content length is required and can not be calculated. + */ + public function testThrowsExceptionOnIncalculableSize() + { + $service = $this->generateTestService(); + $client = $this->generateTestClient($service); + $command = $client->getCommand( + 'StreamingOp', + [ + 'InputStream' => Psr7\stream_for('test'), + ] + ); + $middleware = StreamRequestPayloadMiddleware::wrap($service); + $invokable = $middleware(function($cmd, $req) {}); + + // Mock a request with a body whose size returns null + $streamMock = $this->getMockBuilder(\stdClass::class) + ->setMethods(['getSize']) + ->getMock(); + $streamMock->expects($this->any()) + ->method('getSize') + ->willReturn(null); + $requestMock = $this->getMockBuilder(Request::class) + ->setConstructorArgs(['POST', 'https://foo.com']) + ->setMethods(['getBody']) + ->getMock(); + $requestMock->expects($this->any()) + ->method('getBody') + ->willReturn($streamMock); + + $invokable($command, $requestMock); + } + + private function generateTestHandlerList() + { + $service = $this->generateTestService(); + $serializer = ClientResolver::_default_serializer([ + 'api' => $service, + 'endpoint' => '' + ]); + + $list = new HandlerList(); + $list->prependBuild(Middleware::requestBuilder($serializer), 'builder'); + $list->prependSign( + StreamRequestPayloadMiddleware::wrap($service), + 'StreamRequestPayloadMiddleware' + ); + + return $list; + } + private function generateTestClient(Service $service, $args = []) { return new AwsClient( From a187ca4e6ff27cbf8b85ef16ca8864e4184e9959 Mon Sep 17 00:00:00 2001 From: Howard Lopez Date: Wed, 22 May 2019 17:26:59 -0700 Subject: [PATCH 4/4] Add changelog --- .changes/nextrelease/streaming_request_payloads.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changes/nextrelease/streaming_request_payloads.json diff --git a/.changes/nextrelease/streaming_request_payloads.json b/.changes/nextrelease/streaming_request_payloads.json new file mode 100644 index 0000000000..b97b0cd89d --- /dev/null +++ b/.changes/nextrelease/streaming_request_payloads.json @@ -0,0 +1,7 @@ +[ + { + "type": "feature", + "category": "", + "description": "Adds support for 'requiresLength' trait, adding headers as necessary for streaming operations." + } +]