From: Cormac Parle <cparle@wikimedia.org>
Date: Mon, 20 Nov 2017 11:25:20 +0000 (+0000)
Subject: Do not strip Content-Type header for POST requests to swift
X-Git-Tag: 1.31.0-rc.0~1384
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=d36d987c4ee02b7793ae579b43d2d0acd0241d6b

Do not strip Content-Type header for POST requests to swift

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
---

diff --git a/includes/libs/filebackend/SwiftFileBackend.php b/includes/libs/filebackend/SwiftFileBackend.php
index 373ad93a34..a3f121e70e 100644
--- a/includes/libs/filebackend/SwiftFileBackend.php
+++ b/includes/libs/filebackend/SwiftFileBackend.php
@@ -181,6 +181,29 @@ class SwiftFileBackend extends FileBackendStore {
 	 * @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'] )
@@ -197,7 +220,7 @@ class SwiftFileBackend extends FileBackendStore {
 		// 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
@@ -276,7 +299,7 @@ class SwiftFileBackend extends FileBackendStore {
 				'etag' => md5( $params['content'] ),
 				'content-type' => $contentType,
 				'x-object-meta-sha1base36' => $sha1Hash
-			] + $this->sanitizeHdrs( $params ),
+			] + $this->sanitizeHdrsStrict( $params ),
 			'body' => $params['content']
 		] ];
 
@@ -340,7 +363,7 @@ class SwiftFileBackend extends FileBackendStore {
 				'etag' => md5_file( $params['src'] ),
 				'content-type' => $contentType,
 				'x-object-meta-sha1base36' => $sha1Hash
-			] + $this->sanitizeHdrs( $params ),
+			] + $this->sanitizeHdrsStrict( $params ),
 			'body' => $handle // resource
 		] ];
 
@@ -391,7 +414,7 @@ class SwiftFileBackend extends FileBackendStore {
 			'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__;
@@ -440,7 +463,7 @@ class SwiftFileBackend extends FileBackendStore {
 				'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}" ) {
diff --git a/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php b/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
index 9cd2b100cd..b57af25a49 100644
--- a/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
@@ -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
 	 */
@@ -54,6 +116,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
 					'x-content-custom' => 'hello'
 				],
 				[
+					'content-type'   => 'image+bitmap/jpeg',
 					'content-disposition' => 'inline',
 					'content-duration' => 35.6363,
 					'content-custom' => 'hello',
@@ -70,6 +133,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
 					'x-content-custom' => 'hello'
 				],
 				[
+					'content-type'   => 'image+bitmap/jpeg',
 					'content-disposition' => 'inline;filename=xxx',
 					'content-duration' => 35.6363,
 					'content-custom' => 'hello',
@@ -86,6 +150,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
 					'x-content-custom' => 'hello'
 				],
 				[
+					'content-type'   => 'image+bitmap/jpeg',
 					'content-disposition' => '',
 					'content-duration' => 35.6363,
 					'content-custom' => 'hello',