From 8a8f0149b5bf53b330db2ba70c12b350b6b051b3 Mon Sep 17 00:00:00 2001
From: Ian Dunn <iandunn@git.wordpress.org>
Date: Tue, 6 Jul 2021 20:20:51 +0000
Subject: [PATCH] Multisite: Log error/warnings/notices from `ms-files.php`.

Previously errors were not displayed or logged, but the original intention was only to prevent them from being displayed. Hiding them from logs makes problems like #53492 much harder to debug.

This makes the handling of errors in `ms-files` consistent with the REST API, `admin-ajax`, and XML-RPC.

Props iandunn, johnjamesjacoby.
Fixes #53493.


git-svn-id: https://develop.svn.wordpress.org/trunk@51358 602fd350-edb4-49c9-b593-d223f7449a82
---
 src/wp-includes/load.php                 |  7 ++-
 src/wp-includes/ms-files.php             |  3 +-
 tests/phpunit/tests/load/wpDebugMode.php | 62 ++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100644 tests/phpunit/tests/load/wpDebugMode.php

diff --git a/src/wp-includes/load.php b/src/wp-includes/load.php
index 812569305a..ffbc1d2290 100644
--- a/src/wp-includes/load.php
+++ b/src/wp-includes/load.php
@@ -415,7 +415,7 @@ function timer_stop( $display = 0, $precision = 3 ) {
  * When `WP_DEBUG_LOG` is true, errors will be logged to `wp-content/debug.log`.
  * When `WP_DEBUG_LOG` is a valid path, errors will be logged to the specified file.
  *
- * Errors are never displayed for XML-RPC, REST, and Ajax requests.
+ * Errors are never displayed for XML-RPC, REST, `ms-files.php`, and Ajax requests.
  *
  * @since 3.0.0
  * @since 5.1.0 `WP_DEBUG_LOG` can be a file path.
@@ -481,7 +481,10 @@ function wp_debug_mode() {
 		error_reporting( E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_ERROR | E_WARNING | E_PARSE | E_USER_ERROR | E_USER_WARNING | E_RECOVERABLE_ERROR );
 	}
 
-	if ( defined( 'XMLRPC_REQUEST' ) || defined( 'REST_REQUEST' ) || ( defined( 'WP_INSTALLING' ) && WP_INSTALLING ) || wp_doing_ajax() || wp_is_json_request() ) {
+	if (
+		defined( 'XMLRPC_REQUEST' ) || defined( 'REST_REQUEST' ) || defined( 'MS_FILES_REQUEST' ) ||
+		( defined( 'WP_INSTALLING' ) && WP_INSTALLING ) ||
+		wp_doing_ajax() || wp_is_json_request() ) {
 		ini_set( 'display_errors', 0 );
 	}
 }
diff --git a/src/wp-includes/ms-files.php b/src/wp-includes/ms-files.php
index 03af0843f7..6fb7e4d276 100644
--- a/src/wp-includes/ms-files.php
+++ b/src/wp-includes/ms-files.php
@@ -8,6 +8,7 @@
  * @subpackage Multisite
  */
 
+define( 'MS_FILES_REQUEST', true );
 define( 'SHORTINIT', true );
 require_once dirname( __DIR__ ) . '/wp-load.php';
 
@@ -17,8 +18,6 @@ if ( ! is_multisite() ) {
 
 ms_file_constants();
 
-error_reporting( 0 );
-
 if ( '1' == $current_blog->archived || '1' == $current_blog->spam || '1' == $current_blog->deleted ) {
 	status_header( 404 );
 	die( '404 &#8212; File not found.' );
diff --git a/tests/phpunit/tests/load/wpDebugMode.php b/tests/phpunit/tests/load/wpDebugMode.php
new file mode 100644
index 0000000000..d527235e55
--- /dev/null
+++ b/tests/phpunit/tests/load/wpDebugMode.php
@@ -0,0 +1,62 @@
+<?php
+
+/**
+ * Unit tests for `wp_debug_mode()`.
+ *
+ * @package WordPress
+ * @subpackage UnitTests
+ * @since 5.9.0
+ */
+
+/**
+ * Class Test_WP_Debug_Mode.
+ *
+ * @group load.php
+ * @group wp-debug-mode
+ * @covers ::wp_debug_mode
+ *
+ * @since 5.9.0
+ */
+class Test_WP_Debug_Mode extends WP_UnitTestCase {
+	/**
+	 * Test: `wp_debug_mode()` should log, but not display, errors for `ms-files.php`.
+	 *
+	 * @ticket 53493
+	 *
+	 * @since 5.9.0
+	 */
+	public function test_ms_files_logs_but_doesnt_display_errors() {
+		/*
+		 * Global constants can't be mocked in PHPUnit, so this can only run with the expected
+		 * values already set in `wp-tests-config.php`. Unfortunately, that means it won't run in
+		 * automated workflows, but it's still useful when testing locally.
+		 *
+		 * It may be possible to enable automated workflows by mocking `define()`, or by setting up
+		 * addition automated flows that initialize the tests with different values for the constants.
+		 * At the moment, though, neither of those seem to provide enough benefit to justify the time
+		 * investment.
+		 *
+		 * @link https://theaveragedev.com/mocking-constants-in-tests/
+		 */
+		if ( true !== WP_DEBUG || true !== WP_DEBUG_DISPLAY || true !== WP_DEBUG_LOG ) {
+			$this->markTestSkipped( 'Test requires setting `WP_DEBUG_*` constants in `wp-tests-config.php` to expected values.' );
+		}
+
+		// `display_errors` should be _on_ because of `WP_DEBUG_DISPLAY`.
+		wp_debug_mode();
+
+		$this->assertSame( E_ALL, (int) ini_get( 'error_reporting' ) );
+		$this->assertSame( '1', ini_get( 'display_errors' ) );
+		$this->assertSame( '1', ini_get( 'log_errors' ) );
+		$this->assertStringContainsString( 'debug.log', ini_get( 'error_log' ) );
+
+		// `display_errors` should be _off_ now, because of `MS_FILES_REQUEST`.
+		define( 'MS_FILES_REQUEST', true );
+		wp_debug_mode();
+
+		$this->assertSame( E_ALL, (int) ini_get( 'error_reporting' ) );
+		$this->assertSame( '0', ini_get( 'display_errors' ) );
+		$this->assertSame( '1', ini_get( 'log_errors' ) );
+		$this->assertStringContainsString( 'debug.log', ini_get( 'error_log' ) );
+	}
+}