From 746ca01423553ab9341ccfe38e6df0b7c32e6849 Mon Sep 17 00:00:00 2001
From: MDW <mdeweerd@users.noreply.github.com>
Date: Sat, 9 Mar 2024 01:27:01 +0100
Subject: [PATCH] Fix: Correctly close active output buffer. (#28723)

# Fix: Correctly close active output buffer.

Use ob_get_clean(), not ob_get_contents() and ob_clean().

Tests were failing with:
FunctionsLibTest::testVerifCond with data set "Test that verifConf("0") returns false" ('0', false)
Test code or tested code did not (only) close its own output buffers

OK, but incomplete, skipped, or risky tests\!

Also refactored a test case to use a data provider which helped identify that it was related
to dol_eval.
---
 htdocs/core/lib/functions.lib.php |  6 ++--
 test/phpunit/FunctionsLibTest.php | 52 ++++++++++++++++++-------------
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php
index ffbe1b1afcf..1ec0132756d 100644
--- a/htdocs/core/lib/functions.lib.php
+++ b/htdocs/core/lib/functions.lib.php
@@ -9957,8 +9957,7 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1'
 			if ($hideerrors) {
 				ob_start();	// An evaluation has no reason to output data
 				$tmps = @eval('return '.$s.';');
-				$tmpo = ob_get_contents();
-				ob_clean(); // End of interception of data
+				$tmpo = ob_get_clean();
 				if ($tmpo) {
 					print 'Bad string syntax to evaluate. Some data were output when it should not when evaluating: '.$s;
 				}
@@ -9966,8 +9965,7 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1'
 			} else {
 				ob_start();	// An evaluation has no reason to output data
 				$tmps = eval('return '.$s.';');
-				$tmpo = ob_get_contents();
-				ob_clean(); // End of interception of data
+				$tmpo = ob_get_clean();
 				if ($tmpo) {
 					print 'Bad string syntax to evaluate. Some data were output when it should not when evaluating: '.$s;
 				}
diff --git a/test/phpunit/FunctionsLibTest.php b/test/phpunit/FunctionsLibTest.php
index f621e6dfe27..bd0f56b57cf 100644
--- a/test/phpunit/FunctionsLibTest.php
+++ b/test/phpunit/FunctionsLibTest.php
@@ -1227,33 +1227,43 @@ class FunctionsLibTest extends CommonClassTest
 		$this->assertEquals(getServerTimeZoneInt('now') * 3600, ($nowtzserver - $now));
 	}
 
+
+
+	/**
+	 * Data provider for testVerifCond
+	 *
+	 * @return array<string,array{0:string,1:bool}>
+	 */
+	public function verifCondDataProvider(): array
+	{
+		return [
+			'Test a true comparison' => ['1==1', true,],
+			'Test a false comparison' => ['1==2', false,],
+			'Test that the conf property of a module reports true when enabled' => ['isModEnabled("facture")', true,],
+			'Test that the conf property of a module reports false when disabled' => ['isModEnabled("moduledummy")', false,],
+			'Test that verifConf(0) returns false' => [0, false,],
+			'Test that verifConf("0") returns false' => ["0", false,],
+			'Test that verifConf("") returns false (special case)' => ['', true,],
+		];
+	}
+
 	/**
 	 * testVerifCond
 	 *
+	 * @dataProvider verifCondDataProvider
+	 *
+	 * @param string $cond     Condition to test using verifCond
+	 * @param string $expected Expected outcome of verifCond
+	 *
 	 * @return	void
 	 */
-	public function testVerifCond()
+	public function testVerifCond($cond, $expected)
 	{
-		$verifcond = verifCond('1==1');
-		$this->assertTrue($verifcond, 'Test a true comparison');
-
-		$verifcond = verifCond('1==2');
-		$this->assertFalse($verifcond, 'Test a false comparison');
-
-		$verifcond = verifCond('isModEnabled("facture")');
-		$this->assertTrue($verifcond, 'Test that the conf property of a module reports true when enabled');
-
-		$verifcond = verifCond('isModEnabled("moduledummy")');
-		$this->assertFalse($verifcond, 'Test that the conf property of a module reports false when disabled');
-
-		$verifcond = verifCond(0);
-		$this->assertFalse($verifcond, 'Test that verifConf(0) return False');
-
-		$verifcond = verifCond("0");
-		$this->assertFalse($verifcond, 'Test that verifConf("0") return False');
-
-		$verifcond = verifCond('');
-		$this->assertTrue($verifcond, 'Test that verifConf("") return False (special case)');
+		if ($expected) {
+			$this->assertTrue(verifCond($cond));
+		} else {
+			$this->assertFalse(verifCond($cond));
+		}
 	}
 
 	/**