diff --git a/ChangeLog.md b/ChangeLog.md index 4ae32fb2b..be3f21f08 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -23,6 +23,7 @@ MicrosoftAzure\Storage\Queue\Models\GetQueueMetadataResult.setMetadata MicrosoftAzure\Storage\Queue\Models\Queue.setMetadata ``` * Removed test code from composer package. +* `StorageAuthScheme::computeCanonicalizedResource` assumes that the query parameters are already grouped. That is, multi-value query parameters must be assembled using `ServiceRestProxy::groupQueryValues`. This fixes an issue with other single-value query parameters that might contain the separator character in the value. Blob * Added support for user to upload large files with minimum memory usage. diff --git a/src/Blob/BlobRestProxy.php b/src/Blob/BlobRestProxy.php index 40954b0ae..a1cef6050 100644 --- a/src/Blob/BlobRestProxy.php +++ b/src/Blob/BlobRestProxy.php @@ -1173,7 +1173,7 @@ public function listBlobs($container, $options = null) $includeSnapshots = $options->getIncludeSnapshots(); $includeUncommittedBlobs = $options->getIncludeUncommittedBlobs(); - $includeValue = $this->groupQueryValues( + $includeValue = static::groupQueryValues( array( $includeMetadata ? 'metadata' : null, $includeSnapshots ? 'snapshots' : null, diff --git a/src/Common/Internal/Authentication/StorageAuthScheme.php b/src/Common/Internal/Authentication/StorageAuthScheme.php index 7dd4b1f2c..7cb3d3a94 100644 --- a/src/Common/Internal/Authentication/StorageAuthScheme.php +++ b/src/Common/Internal/Authentication/StorageAuthScheme.php @@ -181,12 +181,9 @@ protected function computeCanonicalizedResource($url, $queryParams) // 9. Group query parameters // 10. Append a new line character (\n) after each name-value pair. foreach ($queryParams as $key => $value) { - // Grouping query parameters - $values = explode(Resources::SEPARATOR, $value); - sort($values); - $separated = implode(Resources::SEPARATOR, $values); - - $canonicalizedResource .= "\n" . $key . ':' . $separated; + // $value must already be ordered lexicographically + // See: ServiceRestProxy::groupQueryValues + $canonicalizedResource .= "\n" . $key . ':' . $value; } return $canonicalizedResource; diff --git a/src/Common/Internal/ServiceRestProxy.php b/src/Common/Internal/ServiceRestProxy.php index 5b946ec81..9c5d02e41 100644 --- a/src/Common/Internal/ServiceRestProxy.php +++ b/src/Common/Internal/ServiceRestProxy.php @@ -503,11 +503,13 @@ public function addPostParameter( * * @return string */ - public function groupQueryValues($values) + public static function groupQueryValues($values) { Validate::isArray($values, 'values'); $joined = Resources::EMPTY_STRING; + sort($values); + foreach ($values as $value) { if (!is_null($value) && !empty($value)) { $joined .= $value . Resources::SEPARATOR; diff --git a/tests/unit/Common/Internal/Authentication/StorageAuthSchemeTest.php b/tests/unit/Common/Internal/Authentication/StorageAuthSchemeTest.php index 48c42467e..a0e8fe472 100644 --- a/tests/unit/Common/Internal/Authentication/StorageAuthSchemeTest.php +++ b/tests/unit/Common/Internal/Authentication/StorageAuthSchemeTest.php @@ -24,6 +24,7 @@ namespace MicrosoftAzure\Storage\Tests\Unit\Common\Internal\Authentication; use MicrosoftAzure\Storage\Common\Internal\Authentication\StorageAuthScheme; +use MicrosoftAzure\Storage\Common\Internal\ServiceRestProxy; use MicrosoftAzure\Storage\Tests\Unit\Utilities; use MicrosoftAzure\Storage\Tests\Mock\Common\Internal\Authentication\StorageAuthSchemeMock; use MicrosoftAzure\Storage\Tests\Framework\TestResources; @@ -94,7 +95,13 @@ public function testComputeCanonicalizedResourceMockMultipleValues() { $queryVariables = array(); $queryVariables['COMP'] = 'list'; - $queryVariables[Resources::QP_INCLUDE] = 'snapshots,metadata,uncommittedblobs'; + $queryVariables[Resources::QP_INCLUDE] = ServiceRestProxy::groupQueryValues( + array( + 'snapshots', + 'metadata', + 'uncommittedblobs' + ) + ); $expectedQueryPart = "comp:list\ninclude:metadata,snapshots,uncommittedblobs"; $accountName = TestResources::ACCOUNT_NAME; $url = TestResources::URI1; diff --git a/tests/unit/Common/Internal/ServiceRestProxyTest.php b/tests/unit/Common/Internal/ServiceRestProxyTest.php index 336d82df2..3e00e75dc 100644 --- a/tests/unit/Common/Internal/ServiceRestProxyTest.php +++ b/tests/unit/Common/Internal/ServiceRestProxyTest.php @@ -147,16 +147,31 @@ public function testAddOptionalSourceAccessContitionHeader($restRestProxy) /** * @covers MicrosoftAzure\Storage\Common\Internal\ServiceRestProxy::groupQueryValues - * @depends test__construct */ - public function testGroupQueryValues($restRestProxy) + public function testGroupQueryValues() { // Setup $values = array('A', 'B', 'C'); $expected = 'A,B,C'; // Test - $actual = $restRestProxy->groupQueryValues($values); + $actual = ServiceRestProxy::groupQueryValues($values); + + // Assert + $this->assertEquals($expected, $actual); + } + + /** + * @covers MicrosoftAzure\Storage\Common\Internal\ServiceRestProxy::groupQueryValues + */ + public function testGroupQueryValuesWithUnorderedValues() + { + // Setup + $values = array('B', 'C', 'A'); + $expected = 'A,B,C'; + + // Test + $actual = ServiceRestProxy::groupQueryValues($values); // Assert $this->assertEquals($expected, $actual); @@ -164,15 +179,14 @@ public function testGroupQueryValues($restRestProxy) /** * @covers MicrosoftAzure\Storage\Common\Internal\ServiceRestProxy::groupQueryValues - * @depends test__construct */ - public function testGroupQueryValuesWithNulls($restRestProxy) + public function testGroupQueryValuesWithNulls() { // Setup $values = array(null, '', null); // Test - $actual = $restRestProxy->groupQueryValues($values); + $actual = ServiceRestProxy::groupQueryValues($values); // Assert $this->assertTrue(empty($actual)); @@ -180,16 +194,15 @@ public function testGroupQueryValuesWithNulls($restRestProxy) /** * @covers MicrosoftAzure\Storage\Common\Internal\ServiceRestProxy::groupQueryValues - * @depends test__construct */ - public function testGroupQueryValuesWithMix($restRestProxy) + public function testGroupQueryValuesWithMix() { // Setup $values = array(null, 'B', 'C', ''); $expected = 'B,C'; // Test - $actual = $restRestProxy->groupQueryValues($values); + $actual = ServiceRestProxy::groupQueryValues($values); // Assert $this->assertEquals($expected, $actual);