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

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index fc41e4e..b15b998 100644 (file)
--- 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 e2914be..9519b7f 100644 (file)
--- 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 5b15e82..fb9dcf5 100644 (file)
--- 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