Addressing the double encoding issue in SigV4.

Adding override for S3 as it is not affected. Adding test for S3 sigv4. Updating compliance tests to match reality and strip trailing newlines. Removing the need to have a test_services.json file.
acinader · Jul 10, 2014 · c4bf949 · c4bf949
1 parent fd101ae
commit c4bf949
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 20 deletions.
diff --git a/src/Aws/Common/Signature/SignatureV4.php b/src/Aws/Common/Signature/SignatureV4.php
@@ -254,6 +254,13 @@ protected function getPresignedPayload(RequestInterface $request)
         return $this->getPayload($request);
     }
 
+    protected function createCanonicalizedPath(RequestInterface $request)
+    {
+        $doubleEncoded = rawurlencode(ltrim($request->getPath(), '/'));
+
+        return '/' . str_replace('%2F', '/', $doubleEncoded);
+    }
+
     private function createStringToSign($longDate, $credentialScope, $creq)
     {
         return "AWS4-HMAC-SHA256\n{$longDate}\n{$credentialScope}\n"
@@ -299,7 +306,7 @@ private function createSigningContext(RequestInterface $request, $payload)
     {
         // Normalize the path as required by SigV4 and ensure it's absolute
         $canon = $request->getMethod() . "\n"
-            . '/' . ltrim($request->getPath(), '/') . "\n"
+            . $this->createCanonicalizedPath($request) . "\n"
             . $this->getCanonicalizedQueryString($request) . "\n";
 
         // Create the canonical headers

diff --git a/src/Aws/S3/S3SignatureV4.php b/src/Aws/S3/S3SignatureV4.php
@@ -53,4 +53,12 @@ protected function getPresignedPayload(RequestInterface $request)
 
         return $result;
     }
+
+    /**
+     * Amazon S3 does not double-encode the path component in the canonical req
+     */
+    protected function createCanonicalizedPath(RequestInterface $request)
+    {
+        return '/' . ltrim($request->getPath(), '/');
+    }
 }
diff --git a/tests/Aws/Tests/CognitoSync/Integration/IntegrationTest.php b/tests/Aws/Tests/CognitoSync/Integration/IntegrationTest.php
@@ -24,8 +24,8 @@ public function testErrorsAreParsedCorrectly()
     {
         try {
             $this->client->deleteDataset(array(
-                'IdentityPoolId' => 'abc:abc',
-                'IdentityId'     => 'foo',
+                'IdentityPoolId' => 'abc:123af',
+                'IdentityId'     => 'foo:123af',
                 'DatasetName'    => 'bar',
             ));
             $this->fail('An exception should have been thrown.');

diff --git a/tests/Aws/Tests/Common/Signature/SignatureV4Test.php b/tests/Aws/Tests/Common/Signature/SignatureV4Test.php
@@ -100,13 +100,16 @@ public function testSignsRequestsProperly($group)
         $context = $request->getParams()->get('aws.signature');
 
         // Test that the canonical request is correct
-        $this->assertEquals(str_replace("\r", '', file_get_contents($group['creq'])), $context['canonical_request']);
+        $this->assertEquals(str_replace("\r", '', trim(file_get_contents($group['creq']))), $context['canonical_request']);
 
         // Test that the string to sign is correct
-        $this->assertEquals(str_replace("\r", '', file_get_contents($group['sts'])), $context['string_to_sign']);
+        $this->assertEquals(str_replace("\r", '', trim(file_get_contents($group['sts']))), $context['string_to_sign']);
 
         // Test that the authorization header is correct
-        $this->assertEquals(str_replace("\r", '', file_get_contents($group['authz'])), $request->getHeader('Authorization'));
+        $this->assertEquals(
+            str_replace("\r", '', trim(file_get_contents($group['authz']))),
+            (string) $request->getHeader('Authorization')
+        );
 
         $parser->setUtf8Support(false);
     }

diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.authz b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.authz
@@ -1 +1 @@
-AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=date;host, Signature=f309cfbd10197a230c42dd17dbf5cca8a0722564cb40a872d25623cfa758e374
+AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=date;host, Signature=2be9e587d1ee1f0fd02128724b86b4358ba3630d39f493b6e06610a19a50779e
diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.creq b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.creq
@@ -1,8 +1,8 @@
 GET
-/%20/foo
+/%2520/foo
 
 date:Mon, 09 Sep 2011 23:36:00 GMT
 host:host.foo.com
 
 date;host
-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.sreq b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.sreq
@@ -1,5 +1,4 @@
-GET /%20/foo http/1.1
+GET /%2520/foo http/1.1
 Date:Mon, 09 Sep 2011 23:36:00 GMT
 Host:host.foo.com
-Authorization: AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=date;host, Signature=f309cfbd10197a230c42dd17dbf5cca8a0722564cb40a872d25623cfa758e374
-
+AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=date;host, Signature=2be9e587d1ee1f0fd02128724b86b4358ba3630d39f493b6e06610a19a50779e
diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.sts b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-space.sts
@@ -1,4 +1,4 @@
 AWS4-HMAC-SHA256
 20110909T233600Z
 20110909/us-east-1/host/aws4_request
-69c45fb9fe3fd76442b5086e50b2e9fec8298358da957b293ef26e506fdfb54b
+35975885599128f8963ce9be7219598b2b5a344321bae304529556c891e9e6ba
diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-utf8.authz b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-utf8.authz
@@ -1 +1 @@
-AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=date;host, Signature=8d6634c189aa8c75c2e51e106b6b5121bed103fdb351f7d7d4381c738823af74
+AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/us-east-1/host/aws4_request, SignedHeaders=date;host, Signature=57b1eb3df3b16a8479c0097b66c0b261cfd65b829419a3f2c3d05319722490b8
diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-utf8.creq b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-utf8.creq
@@ -1,8 +1,8 @@
 GET
-/%E1%88%B4
+/%25E1%2588%25B4
 
 date:Mon, 09 Sep 2011 23:36:00 GMT
 host:host.foo.com
 
 date;host
-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-utf8.sts b/tests/Aws/Tests/Common/Signature/aws4_testsuite/get-utf8.sts
@@ -1,4 +1,4 @@
 AWS4-HMAC-SHA256
 20110909T233600Z
 20110909/us-east-1/host/aws4_request
-27ba31df5dbc6e063d8f87d62eb07143f7f271c5330a917840586ac1c85b6f6b
+47f4456c465167f4db3490a68903fe057412e6e030de38f71d9006467eb4922a
diff --git a/tests/Aws/Tests/S3/Integration/S3_20060301_Test.php b/tests/Aws/Tests/S3/Integration/S3_20060301_Test.php
@@ -16,6 +16,7 @@
 
 namespace Aws\S3\Integration;
 
+use Aws\S3\S3Client;
 use Aws\S3\Model\ClearBucket;
 use Guzzle\Http\EntityBody;
 
@@ -624,10 +625,26 @@ public function testCreatePresignedUrlWithAcl()
         curl_exec($ch);
     }
 
+    /**
+     * @depends testGetObjectUrlWithSessionCredentials
+     */
+    public function testPutObjectSigV4()
+    {
+        $client = $this->getServiceBuilder()->get('s3', array(
+            'signature' => 'v4'
+        ));
+        $client->waitUntil('BucketExists', array('Bucket' => $this->bucket));
+        $client->putObject(array(
+            'Bucket' => $this->bucket,
+            'Key'    => 'foo:bar.txt',
+            'Body'   => 'Hello!'
+        ));
+    }
+
     /**
      * Clear the contents and delete a bucket
      *
-     * @depends testGetObjectUrlWithSessionCredentials
+     * @depends testPutObjectSigV4
      * @example Aws\S3\S3Client::clearBucket
      * @example Aws\S3\S3Client::deleteBucket
      * @example Aws\S3\S3Client::waitUntilBucketNotExists

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -42,7 +42,7 @@
 
 // Set the service configuration file if it was not provided from the CLI
 if (!isset($_SERVER['CONFIG'])) {
-    $serviceConfig = $_SERVER['CONFIG'] = dirname(__DIR__) . '/test_services.json';
+    $serviceConfig = dirname(__DIR__) . '/test_services.json';
     if (file_exists($serviceConfig)) {
         $_SERVER['CONFIG'] = $serviceConfig;
     }
@@ -54,7 +54,7 @@
 }
 
 // Instantiate the service builder
-$aws = Aws\Common\Aws::factory(isset($_SERVER['CONFIG']) ? $_SERVER['CONFIG'] : null);
+$aws = Aws\Common\Aws::factory(isset($_SERVER['CONFIG']) ? $_SERVER['CONFIG'] : 'test_services.dist.json');
 
 // Turn on wire logging if configured
 $aws->getEventDispatcher()->addListener('service_builder.create_client', function (\Guzzle\Common\Event $event) {