In FileBackend:
authorAaron Schulz <aaron@users.mediawiki.org>
Wed, 25 Jan 2012 01:57:28 +0000 (01:57 +0000)
committerAaron Schulz <aaron@users.mediawiki.org>
Wed, 25 Jan 2012 01:57:28 +0000 (01:57 +0000)
* 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
includes/filerepo/backend/FSFile.php
includes/filerepo/backend/FileBackend.php
includes/filerepo/backend/FileOp.php
includes/filerepo/backend/SwiftFileBackend.php
includes/filerepo/backend/TempFSFile.php
tests/phpunit/includes/filerepo/FileBackendTest.php

index 55c5e48..0de03c8 100644 (file)
@@ -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, '.' );
index 38dfbac..54dd135 100644 (file)
@@ -1,14 +1,12 @@
 <?php
 /**
  * @file
- * @ingroup FileRepo
  * @ingroup FileBackend
  */
 
 /**
  * Class representing a non-directory file on the file system
  *
- * @ingroup FileRepo
  * @ingroup FileBackend
  */
 class FSFile {
index ee0d312..afab3b3 100644 (file)
@@ -792,7 +792,7 @@ abstract class FileBackend extends FileBackendBase {
                }
 
                // Build up the temp file using the source chunks (in order)...
-               $tmpHandle = fopen( $tmpPath, 'a' );
+               $tmpHandle = fopen( $tmpPath, 'ab' );
                if ( $tmpHandle === false ) {
                        $status->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
        }
index 08242da..acc2ff2 100644 (file)
@@ -213,7 +213,7 @@ abstract class FileOp {
        }
 
        /**
-        * Get required operation parameters
+        * Get the file operation parameters
         * 
         * @return Array (required params list, optional params list)
         */
index a6ddea9..b5d2aaa 100644 (file)
@@ -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 );
index bee00fa..d9c1906 100644 (file)
@@ -1,7 +1,6 @@
 <?php
 /**
  * @file
- * @ingroup FileRepo
  * @ingroup FileBackend
  */
 
@@ -9,7 +8,6 @@
  * This class is used to hold the location and do limited manipulation
  * of files stored temporarily (usually this will be $wgTmpDirectory)
  *
- * @ingroup FileRepo
  * @ingroup FileBackend
  */
 class TempFSFile extends FSFile {
index 2e0fc7c..b6ff05c 100644 (file)
@@ -412,21 +412,22 @@ class FileBackendTest extends MediaWikiTestCase {
        /**
         * @dataProvider provider_testCreate
         */
-       public function testCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ) {
+       public function testCreate( $op, $alreadyExists, $okStatus, $newSize ) {
                $this->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;