From a5c93dea568bcdf59122e52bd8bd5e8c849bcfd7 Mon Sep 17 00:00:00 2001
From: Valerio Bozzolan <nsa+gitspam@succhia.cz>
Date: Fri, 14 Apr 2023 22:08:15 +0200
Subject: [PATCH] Fix InvalidArgumentException on commit hook

Summary:
Fix a regression introduced here:

96ae4ba13acbf0e2f8932e950a92af0495f034d7

I reproduced this exception executing "svn commit" on a hosted repository.

That crash happened because the PHP getenv() function can return false.
But, that is a very terrible value that blasts the non-string-empty check.

So, now the default getenv() value is skipped, without causing problems.

Closes T15253
Ref T15190

Test Plan: - I've run `svn commit` and I have not encountered any issue now

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15253, T15190

Differential Revision: https://we.phorge.it/D25122
---
 scripts/repository/commit_hook.php | 32 +++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/scripts/repository/commit_hook.php b/scripts/repository/commit_hook.php
index 28829edeb2..08f4600b9b 100755
--- a/scripts/repository/commit_hook.php
+++ b/scripts/repository/commit_hook.php
@@ -119,12 +119,14 @@ if ($is_svnrevprop) {
   exit($err);
 } else if ($repository->isGit() || $repository->isHg()) {
   $username = getenv(DiffusionCommitHookEngine::ENV_USER);
-  if (!phutil_nonempty_string($username)) {
-    throw new Exception(
-      pht(
-        'No Direct Pushes: You are pushing directly to a hosted repository. '.
-        'This will not work. See "No Direct Pushes" in the documentation '.
-        'for more information.'));
+  if ($username !== false) {
+    if (!phutil_nonempty_string($username)) {
+      throw new Exception(
+        pht(
+          'No Direct Pushes: You are pushing directly to a hosted repository. '.
+          'This will not work. See "No Direct Pushes" in the documentation '.
+          'for more information.'));
+    }
   }
 
   if ($repository->isHg()) {
@@ -181,18 +183,24 @@ $engine->setStdin($stdin);
 $engine->setOriginalArgv(array_slice($argv, 2));
 
 $remote_address = getenv(DiffusionCommitHookEngine::ENV_REMOTE_ADDRESS);
-if (phutil_nonempty_string($remote_address)) {
-  $engine->setRemoteAddress($remote_address);
+if ($remote_address !== false) {
+  if (phutil_nonempty_string($remote_address)) {
+    $engine->setRemoteAddress($remote_address);
+  }
 }
 
 $remote_protocol = getenv(DiffusionCommitHookEngine::ENV_REMOTE_PROTOCOL);
-if (phutil_nonempty_string($remote_protocol)) {
-  $engine->setRemoteProtocol($remote_protocol);
+if ($remote_protocol !== false) {
+  if (phutil_nonempty_string($remote_protocol)) {
+    $engine->setRemoteProtocol($remote_protocol);
+  }
 }
 
 $request_identifier = getenv(DiffusionCommitHookEngine::ENV_REQUEST);
-if (phutil_nonempty_string($request_identifier)) {
-  $engine->setRequestIdentifier($request_identifier);
+if ($request_identifier !== false) {
+  if (phutil_nonempty_string($request_identifier)) {
+    $engine->setRequestIdentifier($request_identifier);
+  }
 }
 
 try {