Cleaned up and split up Swift header parsing methods a bit

* Added tests for the metadata headers portion of the code

Change-Id: I8ac65e31212b4cca4592f963e0ca5ad30e1349f7

diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php
index dca1b4c..6a0f2ee 100644 (file)
--- a/includes/filebackend/SwiftFileBackend.php
+++ b/includes/filebackend/SwiftFileBackend.php
@@ -173,25 +173,34 @@ class SwiftFileBackend extends FileBackendStore {
 
        /**
         * Sanitize and filter the custom headers from a $params array.
-        * We only allow certain Content- and X-Content- headers.
+        * Only allows certain "standard" Content- and X-Content- headers.
         *
         * @param array $params
         * @return array Sanitized value of 'headers' field in $params
         */
        protected function sanitizeHdrs( array $params ) {
+               return isset( $params['headers'] )
+                       ? $this->getCustomHeaders( $params['headers'] )
+                       : array();
+
+       }
+
+       /**
+        * @param array $rawHeaders
+        * @return array Custom non-metadata HTTP headers
+        */
+       protected function getCustomHeaders( array $rawHeaders ) {
                $headers = array();
 
                // Normalize casing, and strip out illegal headers
-               if ( isset( $params['headers'] ) ) {
-                       foreach ( $params['headers'] as $name => $value ) {
-                               $name = strtolower( $name );
-                               if ( preg_match( '/^content-(type|length)$/', $name ) ) {
-                                       continue; // blacklisted
-                               } elseif ( preg_match( '/^(x-)?content-/', $name ) ) {
-                                       $headers[$name] = $value; // allowed
-                               } elseif ( preg_match( '/^content-(disposition)/', $name ) ) {
-                                       $headers[$name] = $value; // allowed
-                               }
+               foreach ( $rawHeaders as $name => $value ) {
+                       $name = strtolower( $name );
+                       if ( preg_match( '/^content-(type|length)$/', $name ) ) {
+                               continue; // blacklisted
+                       } elseif ( preg_match( '/^(x-)?content-/', $name ) ) {
+                               $headers[$name] = $value; // allowed
+                       } elseif ( preg_match( '/^content-(disposition)/', $name ) ) {
+                               $headers[$name] = $value; // allowed
                        }
                }
                // By default, Swift has annoyingly low maximum header value limits
@@ -213,6 +222,35 @@ class SwiftFileBackend extends FileBackendStore {
                return $headers;
        }
 
+       /**
+        * @param array $rawHeaders
+        * @return array Custom metadata headers
+        */
+       protected function getMetadataHeaders( array $rawHeaders ) {
+               $headers = array();
+               foreach ( $rawHeaders as $name => $value ) {
+                       $name = strtolower( $name );
+                       if ( strpos( $name, 'x-object-meta-' ) === 0 ) {
+                               $headers[$name] = $value;
+                       }
+               }
+
+               return $headers;
+       }
+
+       /**
+        * @param array $rawHeaders
+        * @return array Custom metadata headers with prefix removed
+        */
+       protected function getMetadata( array $rawHeaders ) {
+               $metadata = array();
+               foreach ( $this->getMetadataHeaders( $rawHeaders ) as $name => $value ) {
+                       $metadata[substr( $name, strlen( 'x-object-meta-' ) )] = $value;
+               }
+
+               return $metadata;
+       }
+
        protected function doCreateInternal( array $params ) {
                $status = Status::newGood();
 
@@ -1551,22 +1589,16 @@ class SwiftFileBackend extends FileBackendStore {
         */
        protected function getStatFromHeaders( array $rhdrs ) {
                // Fetch all of the custom metadata headers
-               $metadata = array();
-               foreach ( $rhdrs as $name => $value ) {
-                       if ( strpos( $name, 'x-object-meta-' ) === 0 ) {
-                               $metadata[substr( $name, strlen( 'x-object-meta-' ) )] = $value;
-                       }
-               }
+               $metadata = $this->getMetadata( $rhdrs );
                // Fetch all of the custom raw HTTP headers
                $headers = $this->sanitizeHdrs( array( 'headers' => $rhdrs ) );
+
                return array(
                        // Convert various random Swift dates to TS_MW
                        'mtime' => $this->convertSwiftDate( $rhdrs['last-modified'], TS_MW ),
                        // Empty objects actually return no content-length header in Ceph
                        'size'  => isset( $rhdrs['content-length'] ) ? (int)$rhdrs['content-length'] : 0,
-                       'sha1'  => isset( $rhdrs['x-object-meta-sha1base36'] )
-                               ? $rhdrs['x-object-meta-sha1base36']
-                               : null,
+                       'sha1'  => isset( $metadata['sha1base36'] ) ? $metadata['sha1base36'] : null,
                        // Note: manifiest ETags are not an MD5 of the file
                        'md5'   => ctype_xdigit( $rhdrs['etag'] ) ? $rhdrs['etag'] : null,
                        'xattr' => array( 'metadata' => $metadata, 'headers' => $headers )

diff --git a/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php b/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
index 38000f6..a618889 100644 (file)
--- a/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
@@ -6,7 +6,7 @@
  * @group medium
  */
 class SwiftFileBackendTest extends MediaWikiTestCase {
-       /** @var SwiftFileBackend */
+       /** @var TestingAccessWrapper Proxy to SwiftFileBackend */
        private $backend;
 
        protected function setUp() {
@@ -29,6 +29,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
        /**
         * @dataProvider provider_testSanitzeHdrs
         * @covers SwiftFileBackend::sanitzeHdrs
+        * @covers SwiftFileBackend::getCustomHeaders
         */
        public function testSanitzeHdrs( $raw, $sanitized ) {
                $hdrs = $this->backend->sanitizeHdrs( array( 'headers' => $raw ) );
@@ -44,7 +45,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => 'inline',
                                        'content-duration' => 35.6363,
-                                       'content-custom' => 'hello',
+                                       'content-Custom' => 'hello',
                                        'x-content-custom' => 'hello'
                                ),
                                array(
@@ -58,7 +59,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                array(
                                        'content-length' => 345,
                                        'content-type'   => 'image+bitmap/jpeg',
-                                       'content-disposition' => 'inline; filename=xxx; ' . str_repeat( 'o', 1024 ),
+                                       'content-Disposition' => 'inline; filename=xxx; ' . str_repeat( 'o', 1024 ),
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
                                        'x-content-custom' => 'hello'
@@ -88,4 +89,60 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                        )
                );
        }
+
+       /**
+        * @dataProvider provider_testGetMetadataHeaders
+        * @covers SwiftFileBackend::getMetadataHeaders
+        */
+       public function testGetMetadataHeaders( $raw, $sanitized ) {
+               $hdrs = $this->backend->getMetadataHeaders( $raw );
+
+               $this->assertEquals( $hdrs, $sanitized, 'getMetadataHeaders() has expected result' );
+       }
+
+       public static function provider_testGetMetadataHeaders() {
+               return array(
+                       array(
+                               array(
+                                       'content-length' => 345,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello',
+                                       'x-object-meta-custom' => 5,
+                                       'x-object-meta-sha1Base36' => 'a3deadfg...',
+                               ),
+                               array(
+                                       'x-object-meta-custom' => 5,
+                                       'x-object-meta-sha1base36' => 'a3deadfg...',
+                               )
+                       )
+               );
+       }
+
+       /**
+        * @dataProvider provider_testGetMetadata
+        * @covers SwiftFileBackend::getMetadata
+        */
+       public function testGetMetadata( $raw, $sanitized ) {
+               $hdrs = $this->backend->getMetadata( $raw );
+
+               $this->assertEquals( $hdrs, $sanitized, 'getMetadata() has expected result' );
+       }
+
+       public static function provider_testGetMetadata() {
+               return array(
+                       array(
+                               array(
+                                       'content-length' => 345,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello',
+                                       'x-object-meta-custom' => 5,
+                                       'x-object-meta-sha1Base36' => 'a3deadfg...',
+                               ),
+                               array(
+                                       'custom' => 5,
+                                       'sha1base36' => 'a3deadfg...',
+                               )
+                       )
+               );
+       }
 }
\ No newline at end of file