From 9bf5e1735287f6f4113040fa5eb763e20d94906d Mon Sep 17 00:00:00 2001 From: Steve Campbell Date: Tue, 4 Jul 2023 12:01:51 +0100 Subject: [PATCH] PHP8.1 fix for DiffusionServeController serveRequest() Summary: When a 'git pull' is done to an https git URL, the $_SERVER variables PHP_AUTH_USER and PHP_AUTH_PW will be unset, causing PHP 8.1 to throw strlen(null) errors. This update fixes the issue by defaulting the values to '', which results in $have_user and $have_pass having false values as desired. Fixes T15520 Test Plan: arc unit Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15520 Differential Revision: https://we.phorge.it/D25327 --- src/__phutil_library_map__.php | 2 ++ .../controller/DiffusionServeController.php | 4 ++-- .../DiffusionServeControllerTestCase.php | 21 +++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 src/applications/diffusion/controller/__tests__/DiffusionServeControllerTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 09b8b86da4..3de392e025 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1052,6 +1052,7 @@ phutil_register_library_map(array( 'DiffusionSSHWorkflow' => 'applications/diffusion/ssh/DiffusionSSHWorkflow.php', 'DiffusionSearchQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php', 'DiffusionServeController' => 'applications/diffusion/controller/DiffusionServeController.php', + 'DiffusionServeControllerTestCase' => 'applications/diffusion/controller/__tests__/DiffusionServeControllerTestCase.php', 'DiffusionServiceRef' => 'applications/diffusion/ref/DiffusionServiceRef.php', 'DiffusionSetPasswordSettingsPanel' => 'applications/diffusion/panel/DiffusionSetPasswordSettingsPanel.php', 'DiffusionSetupException' => 'applications/diffusion/exception/DiffusionSetupException.php', @@ -7103,6 +7104,7 @@ phutil_register_library_map(array( 'DiffusionSSHWorkflow' => 'PhabricatorSSHWorkflow', 'DiffusionSearchQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionServeController' => 'DiffusionController', + 'DiffusionServeControllerTestCase' => 'PhabricatorTestCase', 'DiffusionServiceRef' => 'Phobject', 'DiffusionSetPasswordSettingsPanel' => 'PhabricatorSettingsPanel', 'DiffusionSetupException' => 'Exception', diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index 89430eef37..93656ea607 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -183,8 +183,8 @@ final class DiffusionServeController extends DiffusionController { // won't prompt users who provide a username but no password otherwise. // See T10797 for discussion. - $have_user = strlen(idx($_SERVER, 'PHP_AUTH_USER')); - $have_pass = strlen(idx($_SERVER, 'PHP_AUTH_PW')); + $have_user = strlen(idx($_SERVER, 'PHP_AUTH_USER', '')); + $have_pass = strlen(idx($_SERVER, 'PHP_AUTH_PW', '')); if ($have_user && $have_pass) { $username = $_SERVER['PHP_AUTH_USER']; $password = new PhutilOpaqueEnvelope($_SERVER['PHP_AUTH_PW']); diff --git a/src/applications/diffusion/controller/__tests__/DiffusionServeControllerTestCase.php b/src/applications/diffusion/controller/__tests__/DiffusionServeControllerTestCase.php new file mode 100644 index 0000000000..67c5e63a57 --- /dev/null +++ b/src/applications/diffusion/controller/__tests__/DiffusionServeControllerTestCase.php @@ -0,0 +1,21 @@ + true, + ); + } + + public function testHandleRequest() { + $aphront_request = new AphrontRequest('example.com', '/'); + $diffusion_serve_controller = new DiffusionServeController(); + + $diffusion_serve_controller->setRequest($aphront_request); + $result = $diffusion_serve_controller->handleRequest($aphront_request); + $this->assertTrue(true, 'handleRequest did not throw an error'); + $this->assertTrue($result instanceof PhabricatorVCSResponse, + 'handleRequest() returns PhabricatorVCSResponse object'); + } + +}