From 65e57fe23dd30d4d3a7f31b761de3e73b08911cc Mon Sep 17 00:00:00 2001 From: Steve Campbell Date: Mon, 3 Jul 2023 17:39:52 +0100 Subject: [PATCH] Fix PHP 8.1 PhabricatorEditorURIEngine::newForViewer() trim(NULL) error Summary: Under PHP 8.1, PhabricatorEditorURIEngine::newForViewer() is throwing a trim(NULL) error when trying to view a diff. This is because it tries to apply string operations to a user setting which will be null by default. Fixes T15518 Test Plan: Unit test added - arc unit Or just view a diff. Eg: https://my.phorge.site/D1234 Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15518 Differential Revision: https://we.phorge.it/D25324 --- .../editor/PhabricatorEditorURIEngine.php | 2 +- .../PhabricatorEditorURIEngineTestCase.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/infrastructure/editor/PhabricatorEditorURIEngine.php b/src/infrastructure/editor/PhabricatorEditorURIEngine.php index d4821c4ffd..76c95dead2 100644 --- a/src/infrastructure/editor/PhabricatorEditorURIEngine.php +++ b/src/infrastructure/editor/PhabricatorEditorURIEngine.php @@ -16,7 +16,7 @@ final class PhabricatorEditorURIEngine $pattern = $viewer->getUserSetting(PhabricatorEditorSetting::SETTINGKEY); - if (!strlen(trim($pattern))) { + if ($pattern === null || trim($pattern) === '') { return null; } diff --git a/src/infrastructure/editor/__tests__/PhabricatorEditorURIEngineTestCase.php b/src/infrastructure/editor/__tests__/PhabricatorEditorURIEngineTestCase.php index 43cebff2e6..b6a2b4d8db 100644 --- a/src/infrastructure/editor/__tests__/PhabricatorEditorURIEngineTestCase.php +++ b/src/infrastructure/editor/__tests__/PhabricatorEditorURIEngineTestCase.php @@ -3,6 +3,12 @@ final class PhabricatorEditorURIEngineTestCase extends PhabricatorTestCase { + protected function getPhabricatorTestCaseConfiguration() { + return array( + self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true, + ); + } + public function testPatternParsing() { $map = array( '' => array(), @@ -129,4 +135,15 @@ final class PhabricatorEditorURIEngineTestCase } } + public function testNewForViewer() { + $phabricator_user = $this->generateNewTestUser(); + try { + $engine = PhabricatorEditorURIEngine::newForViewer($phabricator_user); + $this->assertTrue(true, 'newForViewer did not throw an error'); + } catch (Throwable $ex) { + $this->assertTrue(false, + 'newForViewer threw an exception:'.$ex->getMessage()); + } + } + } -- 2.51.2