Manually redirect in UploadFromUrl

The upload function saved the redirect page(s) content together with the upload file content which would produce an incorrect MIME type (text/html instead of actual upload MIME type). Disabled the automatic redirect and reset the file content until the final file location or the maximum number of redirect is reached.

This patch also touches WatchedItemQueryServiceIntegrationTest to make
it more robust. Without this change, UploadFromUrlTest interfered with
WatchedItemQueryServiceIntegrationTest in some way, causing it to fail.

Bug: T258122
Change-Id: I1de709576c02ce5b31b356751680cbd23689a3fa

1 files changed, 61 insertions, 13 deletions

diff --git a/tests/phpunit/includes/upload/UploadFromUrlTest.php b/tests/phpunit/includes/upload/UploadFromUrlTest.php
index 14c81a042f75..5a4e0b224909 100644
--- a/tests/phpunit/includes/upload/UploadFromUrlTest.php
+++ b/tests/phpunit/includes/upload/UploadFromUrlTest.php
@@ -35,7 +35,7 @@ class UploadFromUrlTest extends ApiTestCase {
 	}
 
 	/**
-	 * @param null|string|array| $request
+	 * @param null|string|array|callable|MWHttpRequest $request
 	 *
 	 * @return void
 	 * @throws Exception
@@ -54,8 +54,7 @@ class UploadFromUrlTest extends ApiTestCase {
 			->getMock();
 
 		if ( $request === null ) {
-			$mockHttpRequestFactory->method( 'create' )
-				->willThrowException( new LogicException( 'No Fake MWHttpRequest provided!' ) );
+			$mockHttpRequestFactory->expects( $this->never() )->method( 'create' );
 		} elseif ( $request instanceof MWHttpRequest ) {
 			$mockHttpRequestFactory->method( 'create' )
 				->willReturn( $request );
@@ -64,7 +63,7 @@ class UploadFromUrlTest extends ApiTestCase {
 				->willReturnCallback( $request );
 		} elseif ( is_array( $request ) ) {
 			$mockHttpRequestFactory->method( 'create' )
-				->willReturnOnConsecutiveCalls( $request );
+				->willReturnOnConsecutiveCalls( ...$request );
 		} elseif ( is_string( $request ) ) {
 			$mockHttpRequestFactory->method( 'create' )
 				->willReturn( $this->makeFakeHttpRequest( $request ) );
@@ -83,15 +82,16 @@ class UploadFromUrlTest extends ApiTestCase {
 	 * @return MWHttpRequest
 	 */
 	private function makeFakeHttpRequest( $body = 'Lorem Ipsum', $statusCode = 200, $headers = [] ) {
-		$options = [
-			'timeout' => 1,
-			'connectTimeout' => 1,
-		];
-
-		$mockHttpRequest = $this->getMockBuilder( MWHttpRequest::class )
-			->setConstructorArgs( [ 'http://www.example.com/test.png', $options ] )
-			->onlyMethods( [ 'execute', 'setCallback' ] )
-			->getMock();
+		$mockHttpRequest = $this->createNoOpMock(
+			MWHttpRequest::class,
+			[ 'execute', 'setCallback', 'isRedirect', 'getFinalUrl' ]
+		);
+
+		$mockHttpRequest->method( 'isRedirect' )->willReturn(
+			$statusCode >= 300 && $statusCode < 400
+		);
+
+		$mockHttpRequest->method( 'getFinalUrl' )->willReturn( $headers[ 'Location' ] ?? '' );
 
 		$dataCallback = null;
 		$mockHttpRequest->method( 'setCallback' )
@@ -243,6 +243,18 @@ class UploadFromUrlTest extends ApiTestCase {
 		$this->assertTrue( $exception, "Got exception" );
 	}
 
+	private function assertUploadOk( UploadBase $upload ) {
+		$verificationResult = $upload->verifyUpload();
+
+		if ( $verificationResult['status'] !== UploadBase::OK ) {
+			$this->fail(
+				'Upload verification returned ' . $upload->getVerificationErrorCode(
+					$verificationResult['status']
+				)
+			);
+		}
+	}
+
 	/**
 	 * @depends testClearQueue
 	 */
@@ -283,4 +295,40 @@ class UploadFromUrlTest extends ApiTestCase {
 
 		$this->assertFalse( $t->exists(), "File '$name' was deleted" );
 	}
+
+	public function testUploadFromUrl() {
+		$file = __DIR__ . '/../../data/upload/png-plain.png';
+		$this->installMockHttp( file_get_contents( $file ) );
+
+		$upload = new UploadFromUrl();
+		$upload->initialize( 'Test.png', 'http://www.example.com/test.png' );
+		$status = $upload->fetchFile();
+
+		$this->assertTrue( $status->isOK() );
+		$this->assertUploadOk( $upload );
+	}
+
+	public function testUploadFromUrlWithRedirect() {
+		$file = __DIR__ . '/../../data/upload/png-plain.png';
+		$this->installMockHttp( [
+			// First response is a redirect
+			$this->makeFakeHttpRequest(
+				'Blaba',
+				302,
+				[ 'Location' => 'http://static.example.com/files/test.png' ]
+			),
+			// Second response is a file
+			$this->makeFakeHttpRequest(
+				file_get_contents( $file )
+			),
+		] );
+
+		$upload = new UploadFromUrl();
+		$upload->initialize( 'Test.png', 'http://www.example.com/test.png' );
+		$status = $upload->fetchFile();
+
+		$this->assertTrue( $status->isOK() );
+		$this->assertUploadOk( $upload );
+	}
+
 }