Do not strip Content-Type header for POST requests to swift
authorCormac Parle <cparle@wikimedia.org>
Mon, 20 Nov 2017 11:25:20 +0000 (11:25 +0000)
committerGilles <gdubuc@wikimedia.org>
Tue, 28 Nov 2017 14:24:27 +0000 (14:24 +0000)
libcurl adds 'Content-Type: application/x-www-form-urlencoded'
to a POST request if the 'Content-Type' header is not set
manually. Because data in swift is updated via POST, the
Content-Type header must be set explicitly to stop a run of
refreshFileHeaders.php from changing the Content-Type of all
files in swift to application/x-www-form-urlencoded

Bug: T178849
Change-Id: I43c21bc1b73e37104cf07cd5f1c1557f472b9898

includes/libs/filebackend/SwiftFileBackend.php
tests/phpunit/includes/filebackend/SwiftFileBackendTest.php

index 373ad93..a3f121e 100644 (file)
@@ -181,6 +181,29 @@ class SwiftFileBackend extends FileBackendStore {
         * @param array $params
         * @return array Sanitized value of 'headers' field in $params
         */
         * @param array $params
         * @return array Sanitized value of 'headers' field in $params
         */
+       protected function sanitizeHdrsStrict( array $params ) {
+               if ( !isset( $params['headers'] ) ) {
+                       return [];
+               }
+               $headers = $this->getCustomHeaders( $params ['headers'] );
+               if ( isset( $headers[ 'content-type' ] ) ) {
+                       unset( $headers[ 'content-type' ] );
+               }
+               return $headers;
+       }
+
+       /**
+        * Sanitize and filter the custom headers from a $params array.
+        * Only allows certain "standard" Content- and X-Content- headers.
+        *
+        * When POSTing data, libcurl adds Content-Type: application/x-www-form-urlencoded
+        * if Content-Type is not set, which overwrites the stored Content-Type header
+        * in Swift - therefore for POSTing data do not strip the Content-Type header (the
+        * previously-stored header that has been already read back from swift is sent)
+        *
+        * @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'] )
        protected function sanitizeHdrs( array $params ) {
                return isset( $params['headers'] )
                        ? $this->getCustomHeaders( $params['headers'] )
@@ -197,7 +220,7 @@ class SwiftFileBackend extends FileBackendStore {
                // Normalize casing, and strip out illegal headers
                foreach ( $rawHeaders as $name => $value ) {
                        $name = strtolower( $name );
                // Normalize casing, and strip out illegal headers
                foreach ( $rawHeaders as $name => $value ) {
                        $name = strtolower( $name );
-                       if ( preg_match( '/^content-(type|length)$/', $name ) ) {
+                       if ( preg_match( '/^content-length$/', $name ) ) {
                                continue; // blacklisted
                        } elseif ( preg_match( '/^(x-)?content-/', $name ) ) {
                                $headers[$name] = $value; // allowed
                                continue; // blacklisted
                        } elseif ( preg_match( '/^(x-)?content-/', $name ) ) {
                                $headers[$name] = $value; // allowed
@@ -276,7 +299,7 @@ class SwiftFileBackend extends FileBackendStore {
                                'etag' => md5( $params['content'] ),
                                'content-type' => $contentType,
                                'x-object-meta-sha1base36' => $sha1Hash
                                'etag' => md5( $params['content'] ),
                                'content-type' => $contentType,
                                'x-object-meta-sha1base36' => $sha1Hash
-                       ] + $this->sanitizeHdrs( $params ),
+                       ] + $this->sanitizeHdrsStrict( $params ),
                        'body' => $params['content']
                ] ];
 
                        'body' => $params['content']
                ] ];
 
@@ -340,7 +363,7 @@ class SwiftFileBackend extends FileBackendStore {
                                'etag' => md5_file( $params['src'] ),
                                'content-type' => $contentType,
                                'x-object-meta-sha1base36' => $sha1Hash
                                'etag' => md5_file( $params['src'] ),
                                'content-type' => $contentType,
                                'x-object-meta-sha1base36' => $sha1Hash
-                       ] + $this->sanitizeHdrs( $params ),
+                       ] + $this->sanitizeHdrsStrict( $params ),
                        'body' => $handle // resource
                ] ];
 
                        'body' => $handle // resource
                ] ];
 
@@ -391,7 +414,7 @@ class SwiftFileBackend extends FileBackendStore {
                        'headers' => [
                                'x-copy-from' => '/' . rawurlencode( $srcCont ) .
                                        '/' . str_replace( "%2F", "/", rawurlencode( $srcRel ) )
                        'headers' => [
                                'x-copy-from' => '/' . rawurlencode( $srcCont ) .
                                        '/' . str_replace( "%2F", "/", rawurlencode( $srcRel ) )
-                       ] + $this->sanitizeHdrs( $params ), // extra headers merged into object
+                       ] + $this->sanitizeHdrsStrict( $params ), // extra headers merged into object
                ] ];
 
                $method = __METHOD__;
                ] ];
 
                $method = __METHOD__;
@@ -440,7 +463,7 @@ class SwiftFileBackend extends FileBackendStore {
                                'headers' => [
                                        'x-copy-from' => '/' . rawurlencode( $srcCont ) .
                                                '/' . str_replace( "%2F", "/", rawurlencode( $srcRel ) )
                                'headers' => [
                                        'x-copy-from' => '/' . rawurlencode( $srcCont ) .
                                                '/' . str_replace( "%2F", "/", rawurlencode( $srcRel ) )
-                               ] + $this->sanitizeHdrs( $params ) // extra headers merged into object
+                               ] + $this->sanitizeHdrsStrict( $params ) // extra headers merged into object
                        ]
                ];
                if ( "{$srcCont}/{$srcRel}" !== "{$dstCont}/{$dstRel}" ) {
                        ]
                ];
                if ( "{$srcCont}/{$srcRel}" !== "{$dstCont}/{$dstRel}" ) {
index 9cd2b10..b57af25 100644 (file)
@@ -33,6 +33,68 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                );
        }
 
                );
        }
 
+       /**
+        * @dataProvider provider_testSanitizeHdrsStrict
+        */
+       public function testSanitizeHdrsStrict( $raw, $sanitized ) {
+               $hdrs = $this->backend->sanitizeHdrsStrict( [ 'headers' => $raw ] );
+
+               $this->assertEquals( $hdrs, $sanitized, 'sanitizeHdrsStrict() has expected result' );
+       }
+
+       public static function provider_testSanitizeHdrsStrict() {
+               return [
+                       [
+                               [
+                                       'content-length' => 345,
+                                       'content-type'   => 'image+bitmap/jpeg',
+                                       'content-disposition' => 'inline',
+                                       'content-duration' => 35.6363,
+                                       'content-Custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ],
+                               [
+                                       'content-disposition' => 'inline',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ]
+                       ],
+                       [
+                               [
+                                       'content-length' => 345,
+                                       'content-type'   => 'image+bitmap/jpeg',
+                                       'content-Disposition' => 'inline; filename=xxx; ' . str_repeat( 'o', 1024 ),
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ],
+                               [
+                                       'content-disposition' => 'inline;filename=xxx',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ]
+                       ],
+                       [
+                               [
+                                       'content-length' => 345,
+                                       'content-type'   => 'image+bitmap/jpeg',
+                                       'content-disposition' => 'filename=' . str_repeat( 'o', 1024 ) . ';inline',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ],
+                               [
+                                       'content-disposition' => '',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ]
+                       ]
+               ];
+       }
+
        /**
         * @dataProvider provider_testSanitizeHdrs
         */
        /**
         * @dataProvider provider_testSanitizeHdrs
         */
@@ -54,6 +116,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'x-content-custom' => 'hello'
                                ],
                                [
                                        'x-content-custom' => 'hello'
                                ],
                                [
+                                       'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => 'inline',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
                                        'content-disposition' => 'inline',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
@@ -70,6 +133,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'x-content-custom' => 'hello'
                                ],
                                [
                                        'x-content-custom' => 'hello'
                                ],
                                [
+                                       'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => 'inline;filename=xxx',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
                                        'content-disposition' => 'inline;filename=xxx',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
@@ -86,6 +150,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'x-content-custom' => 'hello'
                                ],
                                [
                                        'x-content-custom' => 'hello'
                                ],
                                [
+                                       'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => '',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
                                        'content-disposition' => '',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',