From 51e837f68f6df7fdc6cb35803e497bfc0532c861 Mon Sep 17 00:00:00 2001
From: Tim Starling <tstarling@wikimedia.org>
Date: Fri, 26 Jul 2019 15:07:41 +1000
Subject: [PATCH] Don't try to store File objects to the upload session

File objects can contain closures which can't be serialized.

Instead, add makeWarningsSerializable(), which converts the warnings
to a serializable array. Make ApiUpload::transformWarnings() act on this
serializable array instead. For consistency, ApiUpload::getApiWarnings()
also needs to convert the result of checkWarnings() before transforming
it.

Bug: T228749
Change-Id: I8236aaf3683f93a03a5505803f4638e022cf6d85
---
 includes/api/ApiUpload.php                    | 14 ++++------
 .../jobqueue/jobs/AssembleUploadChunksJob.php |  4 ++-
 includes/upload/UploadBase.php                | 27 +++++++++++++++++++
 3 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index fc41e4ea6a..b15b9989a0 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -658,7 +658,7 @@ class ApiUpload extends ApiBase {
 	 * @return array
 	 */
 	protected function getApiWarnings() {
-		$warnings = $this->mUpload->checkWarnings();
+		$warnings = UploadBase::makeWarningsSerializable( $this->mUpload->checkWarnings() );
 
 		return $this->transformWarnings( $warnings );
 	}
@@ -670,9 +670,8 @@ class ApiUpload extends ApiBase {
 
 			if ( isset( $warnings['duplicate'] ) ) {
 				$dupes = [];
-				/** @var File $dupe */
 				foreach ( $warnings['duplicate'] as $dupe ) {
-					$dupes[] = $dupe->getName();
+					$dupes[] = $dupe['fileName'];
 				}
 				ApiResult::setIndexedTagName( $dupes, 'duplicate' );
 				$warnings['duplicate'] = $dupes;
@@ -681,27 +680,24 @@ class ApiUpload extends ApiBase {
 			if ( isset( $warnings['exists'] ) ) {
 				$warning = $warnings['exists'];
 				unset( $warnings['exists'] );
-				/** @var LocalFile $localFile */
 				$localFile = $warning['normalizedFile'] ?? $warning['file'];
-				$warnings[$warning['warning']] = $localFile->getName();
+				$warnings[$warning['warning']] = $localFile['fileName'];
 			}
 
 			if ( isset( $warnings['no-change'] ) ) {
-				/** @var File $file */
 				$file = $warnings['no-change'];
 				unset( $warnings['no-change'] );
 
 				$warnings['nochange'] = [
-					'timestamp' => wfTimestamp( TS_ISO_8601, $file->getTimestamp() )
+					'timestamp' => wfTimestamp( TS_ISO_8601, $file['timestamp'] )
 				];
 			}
 
 			if ( isset( $warnings['duplicate-version'] ) ) {
 				$dupes = [];
-				/** @var File $dupe */
 				foreach ( $warnings['duplicate-version'] as $dupe ) {
 					$dupes[] = [
-						'timestamp' => wfTimestamp( TS_ISO_8601, $dupe->getTimestamp() )
+						'timestamp' => wfTimestamp( TS_ISO_8601, $dupe['timestamp'] )
 					];
 				}
 				unset( $warnings['duplicate-version'] );
diff --git a/includes/jobqueue/jobs/AssembleUploadChunksJob.php b/includes/jobqueue/jobs/AssembleUploadChunksJob.php
index e2914be5e4..9519b7ffd3 100644
--- a/includes/jobqueue/jobs/AssembleUploadChunksJob.php
+++ b/includes/jobqueue/jobs/AssembleUploadChunksJob.php
@@ -76,7 +76,9 @@ class AssembleUploadChunksJob extends Job {
 
 			// We can only get warnings like 'duplicate' after concatenating the chunks
 			$status = Status::newGood();
-			$status->value = [ 'warnings' => $upload->checkWarnings() ];
+			$status->value = [
+				'warnings' => UploadBase::makeWarningsSerializable( $upload->checkWarnings() )
+			];
 
 			// We have a new filekey for the fully concatenated file
 			$newFileKey = $upload->getStashFile()->getFileKey();
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 5b15e82f34..fb9dcf56d2 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -704,6 +704,33 @@ abstract class UploadBase {
 		return $warnings;
 	}
 
+	/**
+	 * Convert the warnings array returned by checkWarnings() to something that
+	 * can be serialized. File objects will be converted to an associative array
+	 * with the following keys:
+	 *
+	 *   - fileName: The name of the file
+	 *   - timestamp: The upload timestamp
+	 *
+	 * @param mixed[] $warnings
+	 * @return mixed[]
+	 */
+	public static function makeWarningsSerializable( $warnings ) {
+		array_walk_recursive( $warnings, function ( &$param, $key ) {
+			if ( $param instanceof File ) {
+				$param = [
+					'fileName' => $param->getName(),
+					'timestamp' => $param->getTimestamp()
+				];
+			} elseif ( is_object( $param ) ) {
+				throw new InvalidArgumentException(
+					'UploadBase::makeWarningsSerializable: ' .
+					'Unexpected object of class ' . get_class( $param ) );
+			}
+		} );
+		return $warnings;
+	}
+
 	/**
 	 * Check whether the resulting filename is different from the desired one,
 	 * but ignore things like ucfirst() and spaces/underscore things
-- 
2.20.1