Fixed incorrect usage of || operator, added test

removed spurious use of empty()
listFiles() was broken, now works
followup to r92009

diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index a576859..ce0bc8c 100644 (file)
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
@@ -38,7 +38,7 @@ class UploadFromStash extends UploadBase {
        
        public static function isValidKey( $key ) {
                // this is checked in more detail in UploadStash
-               return preg_match( UploadStash::KEY_FORMAT_REGEX, $key );
+               return preg_match( UploadStash::KEY_FORMAT_REGEX, $key ) ? true : false;
        }
 
        /**
@@ -47,7 +47,10 @@ class UploadFromStash extends UploadBase {
         * @return Boolean
         */
        public static function isValidRequest( $request ) {
-               return self::isValidKey( $request->getText( 'wpFileKey' ) || $request->getText( 'wpSessionKey' ) );
+               // this passes wpSessionKey to getText() as a default when wpFileKey isn't set.
+               // wpSessionKey has no default which guarantees failure if both are missing
+               // (though that should have been caught earlier)
+               return self::isValidKey( $request->getText( 'wpFileKey', $request->getText( 'wpSessionKey' ) ) );
        }
 
        public function initialize( $key, $name = 'upload_file' ) {
@@ -74,12 +77,12 @@ class UploadFromStash extends UploadBase {
         * @param $request WebRequest
         */
        public function initializeFromRequest( &$request ) {
-               $fileKey = $request->getText( 'wpFileKey' ) || $request->getText( 'wpSessionKey' );
+               // sends wpSessionKey as a default when wpFileKey is missing
+               $fileKey = $request->getText( 'wpFileKey', $request->getText( 'wpSessionKey' ) );
+
+               // chooses one of wpDestFile, wpUploadFile, filename in that order.
+               $desiredDestName = $request->getText( 'wpDestFile', $request->getText( 'wpUploadFile', $request->getText( 'filename' ) ) );
 
-               $desiredDestName = $request->getText( 'wpDestFile' );
-               if( !$desiredDestName ) {
-                       $desiredDestName = $request->getText( 'wpUploadFile' ) || $request->getText( 'filename' );
-               }
                return $this->initialize( $fileKey, $desiredDestName );
        }
 
@@ -100,7 +103,7 @@ class UploadFromStash extends UploadBase {
         * There is no need to stash the image twice
         */
        public function stashFile( $key = null ) {
-               if ( !empty( $this->mLocalFile ) ) {
+               if ( !$this->mLocalFile ) {
                        return $this->mLocalFile;
                }
                return parent::stashFile( $key );

diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index b46a6fb..af02fb2 100644 (file)
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
@@ -416,7 +416,7 @@ class UploadStash {
                $res = $dbr->select(
                        'uploadstash',
                        'us_key',
-                       array( 'us_key' => $key ),
+                       array( 'us_user' => $this->userId ),
                        __METHOD__
                );
 

diff --git a/tests/phpunit/includes/upload/UploadStashTest.php b/tests/phpunit/includes/upload/UploadStashTest.php
index 3b227fb..4aa64af 100644 (file)
--- a/tests/phpunit/includes/upload/UploadStashTest.php
+++ b/tests/phpunit/includes/upload/UploadStashTest.php
@@ -45,11 +45,35 @@ class UploadStashTest extends MediaWikiTestCase {
                
                $stash->removeFile( $file->getFileKey() );
        }
+
+       public function testValidRequest() {
+               $request = new FauxRequest( array( 'wpFileKey' => 'foo') );
+               $this->assertFalse( UploadFromStash::isValidRequest($request), 'Check failure on bad wpFileKey' );
+
+               $request = new FauxRequest( array( 'wpSessionKey' => 'foo') );
+               $this->assertFalse( UploadFromStash::isValidRequest($request), 'Check failure on bad wpSessionKey' );
+
+               $request = new FauxRequest( array( 'wpFileKey' => 'testkey-test.test') );
+               $this->assertTrue( UploadFromStash::isValidRequest($request), 'Check good wpFileKey' );
+
+               $request = new FauxRequest( array( 'wpFileKey' => 'testkey-test.test') );
+               $this->assertTrue( UploadFromStash::isValidRequest($request), 'Check good wpSessionKey' );
+
+               $request = new FauxRequest( array( 'wpFileKey' => 'testkey-test.test', 'wpSessionKey' => 'foo') );
+               $this->assertTrue( UploadFromStash::isValidRequest($request), 'Check key precedence' );
+       }
+
+
        
        public function tearDown() {
                parent::tearDown();
                
-               unlink( $this->bug29408File . "." );
-               
+               if( file_exists( $this->bug29408File . "." ) ) {
+                       unlink( $this->bug29408File . "." );
+               }
+
+               if( file_exists( $this->bug29408File ) ) {
+                       unlink( $this->bug29408File );
+               }
        }
 }