From 139292dadd13c98a0bf9d43f15e0d0284886dcbe Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 9 Mar 2013 15:09:25 -0800 Subject: [PATCH] [FileBackend] Replaced redundant "disposition" param with "headers". * Deprecated the "disposition" parameter. * This also adds "headers" to to copy/move operations. Change-Id: I63faa1dbbadb42c401b6ed4ac58776dc336ba3ab --- includes/filebackend/FileBackend.php | 34 ++++++++--------- includes/filebackend/FileBackendStore.php | 18 +++++---- includes/filebackend/FileOp.php | 10 ++--- includes/filebackend/SwiftFileBackend.php | 46 ++++++++++++----------- 4 files changed, 55 insertions(+), 53 deletions(-) diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index f40b8c1623..903befcf1b 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -191,7 +191,6 @@ abstract class FileBackend { * 'content' => , * 'overwrite' => , * 'overwriteSame' => , - * 'disposition' => , * 'headers' => # since 1.21 * ); * @endcode @@ -204,7 +203,6 @@ abstract class FileBackend { * 'dst' => , * 'overwrite' => , * 'overwriteSame' => , - * 'disposition' => , * 'headers' => # since 1.21 * ) * @endcode @@ -218,7 +216,7 @@ abstract class FileBackend { * 'overwrite' => , * 'overwriteSame' => , * 'ignoreMissingSource' => , # since 1.21 - * 'disposition' => + * 'headers' => # since 1.21 * ) * @endcode * @@ -231,7 +229,7 @@ abstract class FileBackend { * 'overwrite' => , * 'overwriteSame' => , * 'ignoreMissingSource' => , # since 1.21 - * 'disposition' => + * 'headers' => # since 1.21 * ) * @endcode * @@ -249,7 +247,6 @@ abstract class FileBackend { * array( * 'op' => 'describe', * 'src' => , - * 'disposition' => , * 'headers' => * ) * @endcode @@ -268,13 +265,11 @@ abstract class FileBackend { * - overwriteSame : An error will not be given if a file already * exists at the destination that has the same * contents as the new contents to be written there. - * - disposition : If supplied, the backend will return a Content-Disposition - * header when GETs/HEADs of the destination file are made. - * Backends that don't support metadata ignore this. - * See http://tools.ietf.org/html/rfc6266. (since 1.20) * - headers : If supplied, the backend will return these headers when * GETs/HEADs of the destination file are made. Header values * should be smaller than 256 bytes, often options or numbers. + * Content-Disposition headers can be longer, though the system + * might ignore or truncate ones that are too long to store. * Existing headers will remain, but these will replace any * conflicting previous headers, and headers will be removed * if they are set to an empty string. @@ -321,6 +316,11 @@ abstract class FileBackend { if ( empty( $opts['force'] ) ) { // sanity unset( $opts['nonLocking'] ); } + foreach ( $ops as &$op ) { + if ( isset( $op['disposition'] ) ) { // b/c (MW 1.20) + $op['headers']['Content-Disposition'] = $op['disposition']; + } + } $scope = $this->getScopedPHPBehaviorForOps(); // try to ignore client aborts return $this->doOperationsInternal( $ops, $opts ); } @@ -452,7 +452,6 @@ abstract class FileBackend { * 'op' => 'create', * 'dst' => , * 'content' => , - * 'disposition' => , * 'headers' => # since 1.21 * ) * @endcode @@ -463,7 +462,6 @@ abstract class FileBackend { * 'op' => 'store', * 'src' => , * 'dst' => , - * 'disposition' => , * 'headers' => # since 1.21 * ) * @endcode @@ -475,7 +473,7 @@ abstract class FileBackend { * 'src' => , * 'dst' => , * 'ignoreMissingSource' => , # since 1.21 - * 'disposition' => + * 'headers' => # since 1.21 * ) * @endcode * @@ -486,7 +484,7 @@ abstract class FileBackend { * 'src' => , * 'dst' => , * 'ignoreMissingSource' => , # since 1.21 - * 'disposition' => + * 'headers' => # since 1.21 * ) * @endcode * @@ -504,7 +502,6 @@ abstract class FileBackend { * array( * 'op' => 'describe', * 'src' => , - * 'disposition' => , * 'headers' => * ) * @endcode @@ -519,13 +516,11 @@ abstract class FileBackend { * @par Boolean flags for operations (operation-specific): * - ignoreMissingSource : The operation will simply succeed and do * nothing if the source file does not exist. - * - disposition : When supplied, the backend will add a Content-Disposition - * header when GETs/HEADs of the destination file are made. - * Backends that don't support file metadata will ignore this. - * See http://tools.ietf.org/html/rfc6266 (since 1.20). * - headers : If supplied with a header name/value map, the backend will * reply with these headers when GETs/HEADs of the destination * file are made. Header values should be smaller than 256 bytes. + * Content-Disposition headers can be longer, though the system + * might ignore or truncate ones that are too long to store. * Existing headers will remain, but these will replace any * conflicting previous headers, and headers will be removed * if they are set to an empty string. @@ -551,6 +546,9 @@ abstract class FileBackend { } foreach ( $ops as &$op ) { $op['overwrite'] = true; // avoids RTTs in key/value stores + if ( isset( $op['disposition'] ) ) { // b/c (MW 1.20) + $op['headers']['Content-Disposition'] = $op['disposition']; + } } $scope = $this->getScopedPHPBehaviorForOps(); // try to ignore client aborts return $this->doQuickOperationsInternal( $ops ); diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 3f1d185757..1a9ac3b7cd 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -92,7 +92,6 @@ abstract class FileBackendStore extends FileBackend { * $params include: * - content : the raw file contents * - dst : destination storage path - * - disposition : Content-Disposition header value for the destination * - headers : HTTP header name/value map * - async : Status will be returned immediately if supported. * If the status is OK, then its value field will be @@ -135,7 +134,6 @@ abstract class FileBackendStore extends FileBackend { * $params include: * - src : source path on disk * - dst : destination storage path - * - disposition : Content-Disposition header value for the destination * - headers : HTTP header name/value map * - async : Status will be returned immediately if supported. * If the status is OK, then its value field will be @@ -179,7 +177,7 @@ abstract class FileBackendStore extends FileBackend { * - src : source storage path * - dst : destination storage path * - ignoreMissingSource : do nothing if the source file does not exist - * - disposition : Content-Disposition header value for the destination + * - headers : HTTP header name/value map * - async : Status will be returned immediately if supported. * If the status is OK, then its value field will be * set to a FileBackendStoreOpHandle object. @@ -248,7 +246,7 @@ abstract class FileBackendStore extends FileBackend { * - src : source storage path * - dst : destination storage path * - ignoreMissingSource : do nothing if the source file does not exist - * - disposition : Content-Disposition header value for the destination + * - headers : HTTP header name/value map * - async : Status will be returned immediately if supported. * If the status is OK, then its value field will be * set to a FileBackendStoreOpHandle object. @@ -294,7 +292,6 @@ abstract class FileBackendStore extends FileBackend { * * $params include: * - src : source storage path - * - disposition : Content-Disposition header value for the destination * - headers : HTTP header name/value map * - async : Status will be returned immediately if supported. * If the status is OK, then its value field will be @@ -1277,15 +1274,20 @@ abstract class FileBackendStore extends FileBackend { } /** - * Strip long HTTP headers from a file operation + * Strip long HTTP headers from a file operation. + * Most headers are just numbers, but some are allowed to be long. + * This function is useful for cleaning up headers and avoiding backend + * specific errors, especially in the middle of batch file operations. * * @param array $op Same format as doOperation() * @return Array */ protected function stripInvalidHeadersFromOp( array $op ) { - if ( isset( $op['headers'] ) ) { + static $longs = array( 'Content-Disposition' ); + if ( isset( $op['headers'] ) ) { // op sets HTTP headers foreach ( $op['headers'] as $name => $value ) { - if ( strlen( $name ) > 255 || strlen( $value ) > 255 ) { + $maxHVLen = in_array( $name, $longs ) ? INF : 255; + if ( strlen( $name ) > 255 || strlen( $value ) > $maxHVLen ) { trigger_error( "Header '$name: $value' is too long." ); unset( $op['headers'][$name] ); } elseif ( !strlen( $value ) ) { diff --git a/includes/filebackend/FileOp.php b/includes/filebackend/FileOp.php index bb0ab578e9..e019324df8 100644 --- a/includes/filebackend/FileOp.php +++ b/includes/filebackend/FileOp.php @@ -457,7 +457,7 @@ abstract class FileOp { class CreateFileOp extends FileOp { protected function allowedParams() { return array( array( 'content', 'dst' ), - array( 'overwrite', 'overwriteSame', 'disposition', 'headers' ) ); + array( 'overwrite', 'overwriteSame', 'headers' ) ); } protected function doPrecheck( array &$predicates ) { @@ -521,7 +521,7 @@ class StoreFileOp extends FileOp { */ protected function allowedParams() { return array( array( 'src', 'dst' ), - array( 'overwrite', 'overwriteSame', 'disposition', 'headers' ) ); + array( 'overwrite', 'overwriteSame', 'headers' ) ); } /** @@ -596,7 +596,7 @@ class CopyFileOp extends FileOp { */ protected function allowedParams() { return array( array( 'src', 'dst' ), - array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'disposition' ) ); + array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'headers' ) ); } /** @@ -673,7 +673,7 @@ class MoveFileOp extends FileOp { */ protected function allowedParams() { return array( array( 'src', 'dst' ), - array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'disposition' ) ); + array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'headers' ) ); } /** @@ -813,7 +813,7 @@ class DescribeFileOp extends FileOp { * @return array */ protected function allowedParams() { - return array( array( 'src' ), array( 'disposition', 'headers' ) ); + return array( array( 'src' ), array( 'headers' ) ); } /** diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php index 0f3d97a310..aadad5c616 100644 --- a/includes/filebackend/SwiftFileBackend.php +++ b/includes/filebackend/SwiftFileBackend.php @@ -194,7 +194,19 @@ class SwiftFileBackend extends FileBackendStore { } /** - * @param string $disposition Content-Disposition header value + * @param $headers array + * @return array + */ + protected function sanitizeHdrs( array $headers ) { + // By default, Swift has annoyingly low maximum header value limits + if ( isset( $headers['Content-Disposition'] ) ) { + $headers['Content-Disposition'] = $this->truncDisp( $headers['Content-Disposition'] ); + } + return $headers; + } + + /** + * @param $disposition string Content-Disposition header value * @return string Truncated Content-Disposition header value to meet Swift limits */ protected function truncDisp( $disposition ) { @@ -252,13 +264,9 @@ class SwiftFileBackend extends FileBackendStore { if ( !strlen( $obj->content_type ) ) { // special case $obj->content_type = 'unknown/unknown'; } - // Set the Content-Disposition header if requested - if ( isset( $params['disposition'] ) ) { - $obj->headers['Content-Disposition'] = $this->truncDisp( $params['disposition'] ); - } // Set any other custom headers if requested if ( isset( $params['headers'] ) ) { - $obj->headers += $params['headers']; + $obj->headers += $this->sanitizeHdrs( $params['headers'] ); } if ( !empty( $params['async'] ) ) { // deferred $op = $obj->write_async( $params['content'] ); @@ -337,13 +345,9 @@ class SwiftFileBackend extends FileBackendStore { if ( !strlen( $obj->content_type ) ) { // special case $obj->content_type = 'unknown/unknown'; } - // Set the Content-Disposition header if requested - if ( isset( $params['disposition'] ) ) { - $obj->headers['Content-Disposition'] = $this->truncDisp( $params['disposition'] ); - } // Set any other custom headers if requested if ( isset( $params['headers'] ) ) { - $obj->headers += $params['headers']; + $obj->headers += $this->sanitizeHdrs( $params['headers'] ); } if ( !empty( $params['async'] ) ) { // deferred wfSuppressWarnings(); @@ -424,8 +428,9 @@ class SwiftFileBackend extends FileBackendStore { try { $dstObj = new CF_Object( $dContObj, $dstRel, false, false ); // skip HEAD $hdrs = array(); // source file headers to override with new values - if ( isset( $params['disposition'] ) ) { - $hdrs['Content-Disposition'] = $this->truncDisp( $params['disposition'] ); + // Set any other custom headers if requested + if ( isset( $params['headers'] ) ) { + $hdrs += $this->sanitizeHdrs( $params['headers'] ); } if ( !empty( $params['async'] ) ) { // deferred $op = $sContObj->copy_object_to_async( $srcRel, $dContObj, $dstRel, null, $hdrs ); @@ -497,8 +502,9 @@ class SwiftFileBackend extends FileBackendStore { $srcObj = new CF_Object( $sContObj, $srcRel, false, false ); // skip HEAD $dstObj = new CF_Object( $dContObj, $dstRel, false, false ); // skip HEAD $hdrs = array(); // source file headers to override with new values - if ( isset( $params['disposition'] ) ) { - $hdrs['Content-Disposition'] = $this->truncDisp( $params['disposition'] ); + // Set any other custom headers if requested + if ( isset( $params['headers'] ) ) { + $hdrs += $this->sanitizeHdrs( $params['headers'] ); } if ( !empty( $params['async'] ) ) { // deferred $op = $sContObj->move_object_to_async( $srcRel, $dContObj, $dstRel, null, $hdrs ); @@ -603,19 +609,15 @@ class SwiftFileBackend extends FileBackendStore { return $status; } - $hdrs = isset( $params['headers'] ) ? $params['headers'] : array(); - // Set the Content-Disposition header if requested - if ( isset( $params['disposition'] ) ) { - $hdrs['Content-Disposition'] = $this->truncDisp( $params['disposition'] ); - } - try { $sContObj = $this->getContainer( $srcCont ); // Get the latest version of the current metadata $srcObj = $sContObj->get_object( $srcRel, $this->headersFromParams( array( 'latest' => true ) ) ); // Merge in the metadata updates... - $srcObj->headers = $hdrs + $srcObj->headers; + if ( isset( $params['headers'] ) ) { + $srcObj->headers = $this->sanitizeHdrs( $params['headers'] ) + $srcObj->headers; + } $srcObj->sync_metadata(); // save to Swift $this->purgeCDNCache( array( $srcObj ) ); } catch ( CDNNotEnabledException $e ) { -- 2.20.1