From 2df7ea13a3877250354556f08f40e26ccc727144 Mon Sep 17 00:00:00 2001
From: Valerio Bozzolan <nsa+gitspam@succhia.cz>
Date: Fri, 12 May 2023 21:36:35 +0200
Subject: [PATCH] PhabricatorModularTransactionType: fix regression

Summary:
Fix a regression in this specific point:

    phutil_nonempty_string(integer) called at [<phorge>/src/applications/transactions/storage/PhabricatorModularTransactionType.php:342]

This regression was causing a broken Almanac page and maybe others.

Note: The function phutil_nonempty_string() is well-known to be very angry and
throws for any alien value. This is by design, and in many cases
this is appropriate. But not here.

The business logic here handles very generic types like integers
and really probably whatever scalar value coming from an user input
and then normalized to something else, not necessarily a string, but definitely
something that can be cast to string.

If you have better ideas about how to handle these cases, please join T15190.

Closes T15385

Test Plan:
To test Almanac:

1. go to `/almanac/network/` and create at least one network (example: "foo")
2. go to `/almanac/device/` and create at least one device (example: "bar")
3. visit that Device
4. Add Interface
    - test the creation of an empty Interface
    - test the creation of a right Interface (example: Network "foo", Address 127.0.0.1, Port 80)
    - nothing esplodes anymore

Reviewers: arnold, O1 Blessed Committers, avivey

Reviewed By: arnold, O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15385

Differential Revision: https://we.phorge.it/D25220
---
 .../PhabricatorModularTransactionType.php      | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
index 3840bcdf9c..e2f0a02239 100644
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -334,12 +334,28 @@ abstract class PhabricatorModularTransactionType
     return $this->getEditor()->getIsNewObject();
   }
 
+  /**
+   * Check whenever a new transaction's value is considered an "empty text"
+   * @param mixed $value    A string, null, an integer...
+   * @param array $xactions Transactions
+   */
   final protected function isEmptyTextTransaction($value, array $xactions) {
     foreach ($xactions as $xaction) {
       $value = $xaction->getNewValue();
     }
 
-    return !phutil_nonempty_string($value);
+    // The $value can be a lot of stuff, null, string, integer and maybe more.
+    // We cast to string to answer the question "Is this string empty?".
+    // Note: Don't use phutil_nonempty_stringlike() since it was not designed
+    //       for integers.
+    // Note: Don't use phutil_nonempty_scalar() since very probably we could
+    //       receive a boolean, causing exceptions.
+    //       https://we.phorge.it/T15239
+    $value_clean = phutil_string_cast($value);
+
+    // We made our lives easier and we don't need strlen(something)
+    // and we should not.
+    return $value_clean === '';
   }
 
   /**