It seems we have a regression when an User has an extra field that is non-string and non-empty.
Steps to reproduce:
Set the config user.custom-field-definitions to this value:
{
"mycompany:life": {
"name": "Estimated Hours of Life",
"type": "int",
"instructions": "Estimated number of hours this human will take.",
}
}Then, visit your profile and set a value.
What happens:
EXCEPTION: (InvalidArgumentException) Call to phutil_nonempty_string() expected null or a string, got: int. at [<arcanist>/src/utils/utils.php:2127] arcanist(head=arcpatch-D25304, ref.master=97e163187418, ref.arcpatch-D25304=b1c4cb85e0d1), phorge(head=arcpatch-D25322, ref.master=d725ffaa7728, ref.arcpatch-D25322=c076e9ab360f) #0 <#2> phutil_nonempty_string(integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php:304] #1 <#2> PhabricatorStandardCustomField::renderPropertyViewValue(array) called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomField.php:1301] #2 <#2> PhabricatorCustomField::renderPropertyViewValue(array) called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php:159] #3 <#2> PhabricatorCustomFieldList::appendFieldsToPropertyList(PhabricatorUser, PhabricatorUser, PHUIPropertyListView) called at [<phorge>/src/applications/people/controller/PhabricatorPeopleProfileManageController.php:74] #4 <#2> PhabricatorPeopleProfileManageController::buildPropertyView(PhabricatorUser) called at [<phorge>/src/applications/people/controller/PhabricatorPeopleProfileManageController.php:29] #5 <#2> PhabricatorPeopleProfileManageController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284] #6 phlog(InvalidArgumentException) called at [<phorge>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41] #7 PhabricatorDefaultRequestExceptionHandler::handleRequestThrowable(AphrontRequest, InvalidArgumentException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:751] #8 AphrontApplicationConfiguration::handleThrowable(InvalidArgumentException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:296] #9 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:204] #10 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]
Proposed solution
Maybe this is a good minimal fix:
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php index 37f9e808ab..f7f1fb216e 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php @@ -301,7 +301,8 @@ abstract class PhabricatorStandardCustomField } public function renderPropertyViewValue(array $handles) { - if (!phutil_nonempty_string($this->getFieldValue())) { + $v_str = phutil_string_cast($this->getFieldValue()); + if ($v_str === '') { return null; } return $this->getFieldValue();
Note that this does not work since phutil_nonempty_stringlike() does not allow a string:
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php index 37f9e808ab..f7f1fb216e 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php @@ -301,7 +301,8 @@ abstract class PhabricatorStandardCustomField } public function renderPropertyViewValue(array $handles) { - if (!phutil_nonempty_string($this->getFieldValue())) { + if (!phutil_nonempty_stringlike($this->getFieldValue())) { return null; } return $this->getFieldValue();
Also note that this probably is not OK since phutil_nonempty_scalar() can explode when receiving a boolean T15239, and AFAIK an extra field can be boolean.
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php index 37f9e808ab..f7f1fb216e 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php @@ -301,7 +301,8 @@ abstract class PhabricatorStandardCustomField } public function renderPropertyViewValue(array $handles) { - if (!phutil_nonempty_string($this->getFieldValue())) { + if (!phutil_nonempty_scalar($this->getFieldValue())) { return null; } return $this->getFieldValue();