exception: Let MediaWiki.php control final output for ErrorPageError

The same way it does already for non-error output. This makes
it so that doPreOutputCommit() consistently happens between
the staging of output and the actual sending of output.

It is still allowed for code to bypass this, such as for fatal
errors and for handlers that disable OutputPage (like Special:Export).

But for cases where we do want to perform doPreOutputCommit(), it
should be run consistently between staging and sending so that it
can make appropiate decisions based on the current state of
OutputPage.

Previously, the state of OutputPage seen by doPreOutputCommit()
would be the broken/incomplete output of a seemingly succesful
(possibly cacheable) user action, which would then, after
doPreOutputCommit() runs, be completely replaced by $e->report()/
$out->showErrorPage().

This is a prerequisite for being able to reliably send cookie-block
cookies on error pages (next patch).

Bug: T233594
Change-Id: Iaeaf5e55a5868e6be534ddda73f3b56b9d6ef8f0

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 28c9e16..1a75714 100644 (file)
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -526,11 +526,15 @@ class MediaWiki {
                        try {
                                $this->main();
                        } catch ( ErrorPageError $e ) {
+                               $out = $this->context->getOutput();
+                               // TODO: Should ErrorPageError::report accept a OutputPage parameter?
+                               $e->report( ErrorPageError::STAGE_OUTPUT );
+
                                // T64091: while exceptions are convenient to bubble up GUI errors,
                                // they are not internal application faults. As with normal requests, this
                                // should commit, print the output, do deferred updates, jobs, and profiling.
                                $this->doPreOutputCommit();
-                               $e->report(); // display the GUI error
+                               $out->output(); // display the GUI error
                        }
                } catch ( Exception $e ) {
                        $context = $this->context;

diff --git a/includes/exception/BadRequestError.php b/includes/exception/BadRequestError.php
index 5fcf0e6..2448421 100644 (file)
--- a/includes/exception/BadRequestError.php
+++ b/includes/exception/BadRequestError.php
@@ -26,9 +26,9 @@
  */
 class BadRequestError extends ErrorPageError {
 
-       public function report() {
+       public function report( $action = self::SEND_OUTPUT ) {
                global $wgOut;
                $wgOut->setStatusCode( 400 );
-               parent::report();
+               parent::report( $action );
        }
 }

diff --git a/includes/exception/ErrorPageError.php b/includes/exception/ErrorPageError.php
index 4b18126..64216a4 100644 (file)
--- a/includes/exception/ErrorPageError.php
+++ b/includes/exception/ErrorPageError.php
@@ -25,6 +25,8 @@
  * @ingroup Exception
  */
 class ErrorPageError extends MWException implements ILocalizedException {
+       const SEND_OUTPUT = 0;
+       const STAGE_OUTPUT = 1;
        public $title, $msg, $params;
 
        /**
@@ -60,13 +62,19 @@ class ErrorPageError extends MWException implements ILocalizedException {
                return wfMessage( $this->msg, $this->params );
        }
 
-       public function report() {
+       public function report( $action = self::SEND_OUTPUT ) {
                if ( self::isCommandLine() || defined( 'MW_API' ) ) {
                        parent::report();
                } else {
                        global $wgOut;
                        $wgOut->showErrorPage( $this->title, $this->msg, $this->params );
-                       $wgOut->output();
+                       // Allow skipping of the final output step, so that web-based page views
+                       // from MediaWiki.php, can inspect the staged OutputPage state, and perform
+                       // graceful shutdown via doPreOutputCommit first, just like for regular
+                       // output when there isn't an error page.
+                       if ( $action === self::SEND_OUTPUT ) {
+                               $wgOut->output();
+                       }
                }
        }
 }

diff --git a/includes/exception/PermissionsError.php b/includes/exception/PermissionsError.php
index 87a3dc2..9fa1c7c 100644 (file)
--- a/includes/exception/PermissionsError.php
+++ b/includes/exception/PermissionsError.php
@@ -67,10 +67,12 @@ class PermissionsError extends ErrorPageError {
                parent::__construct( 'permissionserrors', Message::newFromSpecifier( $errors[0] ) );
        }
 
-       public function report() {
+       public function report( $action = self::SEND_OUTPUT ) {
                global $wgOut;
 
                $wgOut->showPermissionsErrorPage( $this->errors, $this->permission );
-               $wgOut->output();
+               if ( $action === self::SEND_OUTPUT ) {
+                       $wgOut->output();
+               }
        }
 }

diff --git a/includes/exception/ThrottledError.php b/includes/exception/ThrottledError.php
index bec0d90..cdeb402 100644 (file)
--- a/includes/exception/ThrottledError.php
+++ b/includes/exception/ThrottledError.php
@@ -32,9 +32,9 @@ class ThrottledError extends ErrorPageError {
                );
        }
 
-       public function report() {
+       public function report( $action = ErrorPageError::SEND_OUTPUT ) {
                global $wgOut;
                $wgOut->setStatusCode( 429 );
-               parent::report();
+               parent::report( $action );
        }
 }

diff --git a/includes/exception/UserNotLoggedIn.php b/includes/exception/UserNotLoggedIn.php
index 246c944..ff992b0 100644 (file)
--- a/includes/exception/UserNotLoggedIn.php
+++ b/includes/exception/UserNotLoggedIn.php
@@ -75,11 +75,11 @@ class UserNotLoggedIn extends ErrorPageError {
         * Redirect to Special:Userlogin if the specified message is compatible. Otherwise,
         * show an error page as usual.
         */
-       public function report() {
+       public function report( $action = self::SEND_OUTPUT ) {
                // If an unsupported message is used, don't try redirecting to Special:Userlogin,
                // since the message may not be compatible.
                if ( !in_array( $this->msg, LoginHelper::getValidErrorMessages() ) ) {
-                       parent::report();
+                       parent::report( $action );
                        return;
                }
 
@@ -99,6 +99,8 @@ class UserNotLoggedIn extends ErrorPageError {
                        'warning' => $this->msg,
                ] ) );
 
-               $output->output();
+               if ( $action === self::SEND_OUTPUT ) {
+                       $output->output();
+               }
        }
 }

diff --git a/includes/filerepo/file/LocalFileLockError.php b/includes/filerepo/file/LocalFileLockError.php
index 7cfc8c2..b76f3da 100644 (file)
--- a/includes/filerepo/file/LocalFileLockError.php
+++ b/includes/filerepo/file/LocalFileLockError.php
@@ -29,9 +29,9 @@ class LocalFileLockError extends ErrorPageError {
                );
        }
 
-       public function report() {
+       public function report( $action = self::SEND_OUTPUT ) {
                global $wgOut;
                $wgOut->setStatusCode( 429 );
-               parent::report();
+               parent::report( $action );
        }
 }