From 6ec26562019454483db79132a5c076cfa87dfe34 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Fri, 1 Apr 2016 11:03:21 +1300
Subject: [PATCH] BUG fix ErrorControlChain causing errors to be displayed if
 display_errors in php.ini is false Fixes #5250

---
 core/startup/ErrorControlChain.php           | 16 ++++++-
 tests/core/startup/ErrorControlChainTest.php | 44 ++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/core/startup/ErrorControlChain.php b/core/startup/ErrorControlChain.php
index 4ac510cbb..a8cf7277f 100644
--- a/core/startup/ErrorControlChain.php
+++ b/core/startup/ErrorControlChain.php
@@ -36,9 +36,21 @@ class ErrorControlChain {
 		$this->error = (bool)$error;
 	}
 
+	/**
+	 * Sets whether errors are suppressed or not
+	 * Notes:
+	 * - Errors cannot be suppressed if not handling errors.
+	 * - Errors cannot be un-suppressed if original mode dis-allowed visible errors
+	 *
+	 * @param bool $suppression
+	 */
 	public function setSuppression($suppression) {
 		$this->suppression = (bool)$suppression;
-		if ($this->handleFatalErrors) ini_set('display_errors', !$suppression);
+		// Don't modify errors unless handling fatal errors, and if errors were
+		// originally allowed to be displayed.
+		if ($this->handleFatalErrors && $this->originalDisplayErrors) {
+			ini_set('display_errors', !$suppression);
+		}
 	}
 
 	/**
@@ -112,7 +124,7 @@ class ErrorControlChain {
 		$this->handleFatalErrors = true;
 
 		$this->originalDisplayErrors = ini_get('display_errors');
-		ini_set('display_errors', !$this->suppression);
+		$this->setSuppression($this->suppression);
 
 		$this->step();
 	}
diff --git a/tests/core/startup/ErrorControlChainTest.php b/tests/core/startup/ErrorControlChainTest.php
index 843fc6b69..d8e0df7f9 100644
--- a/tests/core/startup/ErrorControlChainTest.php
+++ b/tests/core/startup/ErrorControlChainTest.php
@@ -63,7 +63,11 @@ require_once '$classpath';
 
 class ErrorControlChainTest extends SapphireTest {
 
+	protected $displayErrors = null;
+
 	function setUp() {
+		$this->displayErrors = (bool)ini_get('display_errors');
+
 		// Check we can run PHP at all
 		$null = is_writeable('/dev/null') ? '/dev/null' : 'NUL';
 		exec("php -v 2> $null", $out, $rv);
@@ -76,8 +80,48 @@ class ErrorControlChainTest extends SapphireTest {
 		parent::setUp();
 	}
 
+	public function tearDown() {
+		if($this->displayErrors !== null) {
+			ini_set('display_errors', $this->displayErrors);
+			$this->displayErrors = null;
+		}
+		parent::tearDown(); // TODO: Change the autogenerated stub
+	}
+
 	function testErrorSuppression() {
 
+		// Errors disabled by default
+		ini_set('display_errors', false);
+		$chain = new ErrorControlChain();
+		$whenNotSuppressed = null;
+		$whenSuppressed = null;
+		$chain->then(function($chain) use(&$whenNotSuppressed, &$whenSuppressed) {
+			$chain->setSuppression(true);
+			$whenSuppressed = ini_get('display_errors');
+			$chain->setSuppression(false);
+			$whenNotSuppressed = ini_get('display_errors');
+		})->execute();
+
+		// Disabled errors never un-disable
+		$this->assertFalse((bool)$whenNotSuppressed);
+		$this->assertFalse((bool)$whenSuppressed);
+
+		// Errors enabled by default
+		ini_set('display_errors', true);
+		$chain = new ErrorControlChain();
+		$whenNotSuppressed = null;
+		$whenSuppressed = null;
+		$chain->then(function($chain) use(&$whenNotSuppressed, &$whenSuppressed) {
+			$chain->setSuppression(true);
+			$whenSuppressed = ini_get('display_errors');
+			$chain->setSuppression(false);
+			$whenNotSuppressed = ini_get('display_errors');
+		})->execute();
+
+		// Errors can be suppressed an un-suppressed when initially enabled
+		$this->assertTrue((bool)$whenNotSuppressed);
+		$this->assertFalse((bool)$whenSuppressed);
+
 		// Fatal error
 
 		$chain = new ErrorControlChainTest_Chain();