From d1b159ef63539b035d6ff98d6fbce5ba9d66ebbe Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 25 Jan 2012 01:57:28 +0000 Subject: [PATCH] In FileBackend: * Use 'b' param in some fopen() calls as needed for Windows and newline handling. * Removed some useless padding code in FileBackend::getContainerShard(). Initialized $m to make IDE happy. * Updated some code comments. In SwiftFileBackend: * Manually set the ETag when using php-cloudfiles for creating files to avoid https://github.com/rackspace/php-cloudfiles/issues/59. * Manually set the content type based on how StreamFile::getType(). This makes it safe to read files directly out of the proxy to end-users. The streamFile() backend functions already uses a similar content-type check. --- includes/StreamFile.php | 7 ++-- includes/filerepo/backend/FSFile.php | 2 - includes/filerepo/backend/FileBackend.php | 5 ++- includes/filerepo/backend/FileOp.php | 2 +- .../filerepo/backend/SwiftFileBackend.php | 13 +++++- includes/filerepo/backend/TempFSFile.php | 2 - .../includes/filerepo/FileBackendTest.php | 41 +++++++++++-------- 7 files changed, 45 insertions(+), 27 deletions(-) diff --git a/includes/StreamFile.php b/includes/StreamFile.php index 55c5e48fbc..0de03c8324 100644 --- a/includes/StreamFile.php +++ b/includes/StreamFile.php @@ -74,7 +74,7 @@ class StreamFile { // Cancel output buffering and gzipping if set wfResetOutputBuffers(); - $type = self::getType( $path ); + $type = self::contentTypeFromPath( $path ); if ( $type && $type != 'unknown/unknown' ) { header( "Content-type: $type" ); } else { @@ -115,12 +115,13 @@ class StreamFile { } /** - * Determine the filetype we're dealing with + * Determine the file type of a file based on the path + * * @param $filename string Storage path or file system path * @param $safe bool Whether to do retroactive upload blacklist checks * @return null|string */ - protected static function getType( $filename, $safe = true ) { + public static function contentTypeFromPath( $filename, $safe = true ) { global $wgTrivialMimeDetection; $ext = strrchr( $filename, '.' ); diff --git a/includes/filerepo/backend/FSFile.php b/includes/filerepo/backend/FSFile.php index 38dfbac0b0..54dd135996 100644 --- a/includes/filerepo/backend/FSFile.php +++ b/includes/filerepo/backend/FSFile.php @@ -1,14 +1,12 @@ fatal( 'backend-fail-opentemp', $tmpPath ); return $status; @@ -1448,8 +1448,9 @@ abstract class FileBackend extends FileBackendBase { // Allow certain directories to be above the hash dirs so as // to work with FileRepo (e.g. "archive/a/ab" or "temp/a/ab"). // They must be 2+ chars to avoid any hash directory ambiguity. + $m = array(); if ( preg_match( "!^(?:[^/]{2,}/)*$hashDirRegex(?:/|$)!", $relPath, $m ) ) { - return '.' . str_pad( $m['shard'], $hashLevels, '0', STR_PAD_LEFT ); + return '.' . $m['shard']; } return null; // failed to match } diff --git a/includes/filerepo/backend/FileOp.php b/includes/filerepo/backend/FileOp.php index 08242da5c3..acc2ff2406 100644 --- a/includes/filerepo/backend/FileOp.php +++ b/includes/filerepo/backend/FileOp.php @@ -213,7 +213,7 @@ abstract class FileOp { } /** - * Get required operation parameters + * Get the file operation parameters * * @return Array (required params list, optional params list) */ diff --git a/includes/filerepo/backend/SwiftFileBackend.php b/includes/filerepo/backend/SwiftFileBackend.php index a6ddea97e8..b5d2aaae50 100644 --- a/includes/filerepo/backend/SwiftFileBackend.php +++ b/includes/filerepo/backend/SwiftFileBackend.php @@ -138,6 +138,12 @@ class SwiftFileBackend extends FileBackend { $obj = new CF_Object( $dContObj, $dstRel, false, false ); // skip HEAD // Note: metadata keys stored as [Upper case char][[Lower case char]...] $obj->metadata = array( 'Sha1base36' => $sha1Hash ); + // Manually set the ETag (https://github.com/rackspace/php-cloudfiles/issues/59). + // The MD5 here will be checked within Swift against its own MD5. + $obj->set_etag( md5( $params['content'] ) ); + // Use the same content type as StreamFile for security + $obj->content_type = StreamFile::contentTypeFromPath( $params['dst'] ); + // Actually write the object in Swift $obj->write( $params['content'] ); } catch ( BadContentTypeException $e ) { $status->fatal( 'backend-fail-contenttype', $params['dst'] ); @@ -201,6 +207,11 @@ class SwiftFileBackend extends FileBackend { $obj = new CF_Object( $dContObj, $dstRel, false, false ); // skip HEAD // Note: metadata keys stored as [Upper case char][[Lower case char]...] $obj->metadata = array( 'Sha1base36' => $sha1Hash ); + // The MD5 here will be checked within Swift against its own MD5. + $obj->set_etag( md5_file( $params['src'] ) ); + // Use the same content type as StreamFile for security + $obj->content_type = StreamFile::contentTypeFromPath( $params['dst'] ); + // Actually write the object in Swift $obj->load_from_filename( $params['src'], True ); // calls $obj->write() } catch ( BadContentTypeException $e ) { $status->fatal( 'backend-fail-contenttype', $params['dst'] ); @@ -585,7 +596,7 @@ class SwiftFileBackend extends FileBackend { // Create a new temporary file... $tmpFile = TempFSFile::factory( wfBaseName( $srcRel ) . '_', $ext ); if ( $tmpFile ) { - $handle = fopen( $tmpFile->getPath(), 'w' ); + $handle = fopen( $tmpFile->getPath(), 'wb' ); if ( $handle ) { $obj->stream( $handle, $this->headersFromParams( $params ) ); fclose( $handle ); diff --git a/includes/filerepo/backend/TempFSFile.php b/includes/filerepo/backend/TempFSFile.php index bee00fa784..d9c1906bdf 100644 --- a/includes/filerepo/backend/TempFSFile.php +++ b/includes/filerepo/backend/TempFSFile.php @@ -1,7 +1,6 @@ backend = $this->singleBackend; $this->tearDownFiles(); - $this->doTestCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ); + $this->doTestCreate( $op, $alreadyExists, $okStatus, $newSize ); $this->tearDownFiles(); $this->backend = $this->multiBackend; $this->tearDownFiles(); - $this->doTestCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ); + $this->doTestCreate( $op, $alreadyExists, $okStatus, $newSize ); $this->tearDownFiles(); } - private function doTestCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ) { + private function doTestCreate( $op, $alreadyExists, $okStatus, $newSize ) { $backendName = $this->backendClass(); + $dest = $op['dst']; $this->backend->prepare( array( 'dir' => dirname( $dest ) ) ); $oldText = 'blah...blah...waahwaah'; @@ -477,44 +478,52 @@ class FileBackendTest extends MediaWikiTestCase { public function provider_testCreate() { $cases = array(); - $source = $this->baseStorePath() . '/unittest-cont2/myspacefile.txt'; + $dest = $this->baseStorePath() . '/unittest-cont2/myspacefile.txt'; - $dummyText = 'hey hey'; - $op = array( 'op' => 'create', 'content' => $dummyText, 'dst' => $source ); + $op = array( 'op' => 'create', 'content' => 'test test testing', 'dst' => $dest ); $cases[] = array( $op, // operation - $source, // source false, // no dest already exists true, // succeeds - strlen( $dummyText ) + strlen( $op['content'] ) ); + $op2 = $op; + $op2['content'] = "\n"; $cases[] = array( - $op, // operation - $source, // source + $op2, // operation + false, // no dest already exists + true, // succeeds + strlen( $op2['content'] ) + ); + + $op2 = $op; + $op2['content'] = "fsf\n waf 3kt"; + $cases[] = array( + $op2, // operation true, // dest already exists false, // fails - strlen( $dummyText ) + strlen( $op2['content'] ) ); $op2 = $op; + $op2['content'] = "egm'g gkpe gpqg eqwgwqg"; $op2['overwrite'] = true; $cases[] = array( $op2, // operation - $source, // source true, // dest already exists true, // succeeds - strlen( $dummyText ) + strlen( $op2['content'] ) ); $op2 = $op; + $op2['content'] = "39qjmg3-qg"; $op2['overwriteSame'] = true; $cases[] = array( $op2, // operation - $source, // source true, // dest already exists false, // succeeds - strlen( $dummyText ) + strlen( $op2['content'] ) ); return $cases; -- 2.20.1