validate non empty bucket

.changes/nextrelease/validate-nonempty-bucket.json

@@ -0,0 +1,8 @@
+[
+  {
+    "type": "enhancement",
+    "category": "S3",
+    "description": "Added validation that required S3 parameters are non empty"
+  }
+]
+

src/InputValidationMiddleware.php

@@ -0,0 +1,76 @@
+<?php
+namespace Aws;
+
+use Aws\Api\Service;
+
+/**
+ * Validates the required input parameters of commands are non empty
+ *
+ * @internal
+ */
+class InputValidationMiddleware
+{
+
+    /** @var callable */
+    private $nextHandler;
+
+    /** @var array */
+    private $mandatoryAttributeList;
+
+    /** @var Service */
+    private $service;
+
+    /**
+     * Create a middleware wrapper function.
+     *
+     * @param Service $service
+     * @param array $mandatoryAttributeList
+     * @return callable     */
+    public static function wrap(Service $service, $mandatoryAttributeList) {
+        if (!is_array($mandatoryAttributeList) ||
+            array_filter($mandatoryAttributeList, 'is_string') !== $mandatoryAttributeList
+        ) {
+            throw new \InvalidArgumentException(
+                "The mandatory attribute list must be an array of strings"
+            );
+        }
+        return function (callable $handler) use ($service, $mandatoryAttributeList) {
+            return new self($handler, $service, $mandatoryAttributeList);
+        };
+    }
+
+    public function __construct(
+        callable $nextHandler,
+        Service $service,
+        $mandatoryAttributeList
+    ) {
+        $this->service = $service;
+        $this->nextHandler = $nextHandler;
+        $this->mandatoryAttributeList = $mandatoryAttributeList;
+    }
+
+    public function __invoke(CommandInterface $cmd) {
+        $nextHandler = $this->nextHandler;
+        $op = $this->service->getOperation($cmd->getName())->toArray();
+        if (!empty($op['input']['shape'])) {
+            $service = $this->service->toArray();
+            if (!empty($input = $service['shapes'][$op['input']['shape']])) {
+                if (!empty($input['required'])) {
+                    foreach ($input['required'] as $key => $member) {
+                        if (in_array($member, $this->mandatoryAttributeList)) {
+                            $argument = is_string($cmd[$member]) ?  trim($cmd[$member]) : $cmd[$member];
+                            if ($argument === '' || $argument === null) {
+                                $commandName = $cmd->getName();
+                                throw new \InvalidArgumentException(
+                                    "The {$commandName} operation requires non-empty parameter: {$member}"
+                                );
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        return $nextHandler($cmd);
+    }
+
+}

src/S3/S3Client.php

@@ -10,6 +10,7 @@
 use Aws\Command;
 use Aws\Exception\AwsException;
 use Aws\HandlerList;
+use Aws\InputValidationMiddleware;
 use Aws\Middleware;
 use Aws\Retry\QuotaManager;
 use Aws\RetryMiddleware;
@@ -215,6 +216,9 @@ class S3Client extends AwsClient implements S3ClientInterface
 {
     use S3ClientTrait;
 
+    /** @var array */
+    private static $mandatoryAttributes = ['Bucket', 'Key'];
+
     public static function getArguments()
     {
         $args = parent::getArguments();
@@ -374,6 +378,11 @@ public function __construct(array $args)
             ),
             's3.bucket_endpoint_arn'
         );
+
+        $stack->appendValidate(
+            InputValidationMiddleware::wrap($this->getApi(), self::$mandatoryAttributes),
+            'input_validation_middleware'
+        );
         $stack->appendSign(PutObjectUrlMiddleware::wrap(), 's3.put_object_url');
         $stack->appendSign(PermanentRedirectMiddleware::wrap(), 's3.permanent_redirect');
         $stack->appendInit(Middleware::sourceFile($this->getApi()), 's3.source_file');

tests/InputValidationMiddlewareTest.php

@@ -0,0 +1,188 @@
+<?php
+namespace Aws\Test;
+
+use Aws\Api\DateTimeResult;
+use Aws\AwsClient;
+use Aws\EndpointParameterMiddleware;
+use Aws\HandlerList;
+use Aws\Api\Service;
+use Aws\InputValidationMiddleware;
+use Cassandra\Time;
+use GuzzleHttp\Psr7\Request;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Aws\InputValidationMiddleware
+ */
+class InputValidationMiddlewareTest extends TestCase
+{
+    /**
+     * Data provider for exceptions treated as invalid argument exceptions
+     *
+     * @return array
+     */
+    public function getInvalidEndpointExceptions()
+    {
+        return [
+            [''],
+            [' '],
+            ['  '],
+            [null],
+        ];
+    }
+
+    /**
+     * Data provider for exceptions treated as invalid argument exceptions
+     *
+     * @return array
+     */
+    public function getValidInputs()
+    {
+        return [
+            ['existing data'],
+            [' q '],
+            [[]],
+            [['abc']],
+            [0],
+            [DateTimeResult::fromEpoch(time())],
+        ];
+    }
+
+    /**
+     * @dataProvider getInvalidEndpointExceptions
+     *
+     * @param $input
+     */
+    public function testThrowsExceptions($input)
+    {
+        $service = $this->generateTestService();
+        $client = $this->generateTestClient($service);
+        $command = $client->getCommand('RequiredOp', ['InputParameter' => $input]);
+        $mandatoryInputList = ['InputParameter'];
+
+        $list = new HandlerList();
+        $list->setHandler(function ($command) {
+            return;
+        });
+        $list->appendValidate(
+            InputValidationMiddleware::wrap($service, $mandatoryInputList)
+        );
+        $handler = $list->resolve();
+
+        try {
+            $handler($command, new Request('POST', 'https://foo.com'));
+            $this->fail('Test should have thrown an InvalidArgumentException for not having required parameter set.');
+        } catch (\InvalidArgumentException $e) {
+            $this->assertEquals(
+                "The RequiredOp operation requires non-empty parameter: InputParameter",
+                $e->getMessage()
+            );
+        }
+    }
+
+    /**
+     * @dataProvider getInvalidEndpointExceptions
+     *
+     * @param $input
+     */
+    public function testNoValidationWithoutInputList($input)
+    {
+        $service = $this->generateTestService();
+        $client = $this->generateTestClient($service);
+        $command = $client->getCommand('RequiredOp', ['InputParameter' => $input]);
+        $mandatoryInputList = [];
+        $list = new HandlerList();
+        $list->setHandler(function ($command) {
+            return "success";
+        });
+        $list->appendValidate(
+            InputValidationMiddleware::wrap($service, $mandatoryInputList)
+        );
+        $handler = $list->resolve();
+        $result = $handler($command, new Request('POST', 'https://foo.com'));
+        self::assertSame($result, "success");
+    }
+
+    /**
+     * @dataProvider getValidInputs
+     *
+     * @param $input
+     */
+    public function testPassingValidations($input)
+    {
+        $service = $this->generateTestService();
+        $client = $this->generateTestClient($service);
+        $command = $client->getCommand('RequiredOp', ['InputParameter' => $input]);
+        $mandatoryInputList = ['InputParameter'];
+
+        $list = new HandlerList();
+        $list->setHandler(function ($command) {
+            return "success";
+        });
+        $list->appendValidate(
+            InputValidationMiddleware::wrap($service, $mandatoryInputList)
+        );
+
+        $handler = $list->resolve();
+
+        $result = $handler($command, new Request('POST', 'https://foo.com'));
+        self::assertSame($result, "success");
+    }
+
+    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" => "json",
+                    "apiVersion" => "2014-01-01"
+                ],
+                'shapes' => [
+                    "InputShape" => [
+                        "type" => "structure",
+                        "required" => [
+                            "InputParameter"
+                        ],
+                        "members" => [
+                            "RequiredInputParameter" => [
+                                "shape" => "StringType"
+                            ],
+                        ],
+                    ],
+                    "StringType"=> [
+                        "type"=> "string"
+                    ],
+                ],
+                'operations' => [
+                    "RequiredOp"=> [
+                        "name"=> "RequiredOp",
+                        "http"=> [
+                            "method"=> "POST",
+                            "requestUri"=> "/",
+                            "responseCode"=> 200
+                        ],
+                        "input"=> ["shape"=> "InputShape"],
+                    ],
+                ],
+            ],
+            function () { return []; }
+        );
+    }
+}

tests/Multipart/TestUploader.php

@@ -65,6 +65,7 @@ protected function createPart($seekable, $number)
         return [
             'PartNumber' => $number,
             'Body'       => $body,
+            'UploadId' => 'baz'
         ];
     }
 
@@ -81,7 +82,8 @@ protected function getCompleteParams()
         return [
             'MultipartUpload' => [
                 'Parts' => $this->state->getUploadedParts()
-            ]
+            ],
+            'UploadId' => 'baz'
         ];
     }
 }

tests/S3/BatchDeleteTest.php

@@ -148,7 +148,7 @@ public function testCanCreateFromListObjects()
         $this->assertEquals('foo', $last['Bucket']);
     }
 
-    public function testDeletesYieldedCommadnsWhenEachCallbackReturns()
+    public function testDeletesYieldedCommandsWhenEachCallbackReturns()
     {
         $client = $this->getTestClient('s3');
         $this->addMockResults($client, [

tests/S3/S3ClientTest.php

@@ -336,6 +336,20 @@ public function testReturnsObjectUrlViaPathWithPathStyle()
         );
     }
 
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage The DeleteObject operation requires non-empty parameter: Bucket
+     */
+    public function testEnsuresMandatoryInputVariables()
+    {
+        /** @var S3Client $client */
+        $client = $this->getTestClient('S3');
+        $client->deleteObject([
+            'Bucket' => "",
+            'Key' => "key"]
+        );
+    }
+
     /**
      * @expectedException \RuntimeException
      */

tests/S3/S3EndpointMiddlewareTest.php

@@ -490,6 +490,7 @@ public function testPassesCompliance(
         $useDualstack,
         $useS3Accelerate
     ) {
+        $key = 'key';
         $client = new S3Client([
             'region' => $region,
             'version' => 'latest',
@@ -500,15 +501,15 @@ public function testPassesCompliance(
             'handler' => function (
                 CommandInterface $cmd,
                 RequestInterface $req
-            ) use ($expectedUri) {
-                $this->assertEquals($expectedUri, trim($req->getUri(), '/'));
+            ) use ($key, $expectedUri) {
+                $this->assertEquals($expectedUri . '/' . $key, trim($req->getUri(), '/'));
                 return Promise\promise_for(new Result());
             },
         ]);
 
         $client->getObject([
             'Bucket' => $bucket,
-            'Key' => '',
+            'Key' => $key,
         ]);
     }
 }