From 9cb2d4743a994012a819153021eb0b7a308b5ead Mon Sep 17 00:00:00 2001
From: Ian Baker <raindrift@users.mediawiki.org>
Date: Mon, 15 Aug 2011 18:10:10 +0000
Subject: [PATCH] Fixed incorrect usage of || operator, added test removed
 spurious use of empty() listFiles() was broken, now works followup to r92009

---
 includes/upload/UploadFromStash.php           | 19 +++++++------
 includes/upload/UploadStash.php               |  2 +-
 .../includes/upload/UploadStashTest.php       | 28 +++++++++++++++++--
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index a576859357..ce0bc8c906 100644
--- 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 b46a6fb9c3..af02fb21f9 100644
--- 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 3b227fb951..4aa64afe7d 100644
--- 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 );
+		}
 	}
 }
-- 
2.20.1