Paths

Table of Contentst

Differential D25147

Fix PHP 8.1 "strlen(null)" exceptions which block rendering the People page
ClosedPublic
Actions

Authored by aklapper on Apr 29 2023, 20:59.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15297: PHP 8.1 "strlen(null)" exceptions block rendering the People page

Commits

rP8f669ea08289: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the People page

Summary

strlen() was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts phutil_nonempty_string() as
a general replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Use phutil_nonempty_scalar() instead in PhabricatorSearchDateField.php as
input could only be integer instead of string.

Closes T15297

Test Plan

Applied these four changes (on top of D25144, D25145, D25146) and /people/ finally rendered in web browser.

Diff Detail

Repository

rP Phorge

Branch

D25147 (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 292
Build 292: arc lint + arc unit

Event Timeline

aklapper created this revision.Apr 29 2023, 20:59

Owners added a reviewer: O1: Blessed Committers.Apr 29 2023, 20:59

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptApr 29 2023, 20:59

Harbormaster completed remote builds in B292: Diff 558.Apr 29 2023, 20:59

aklapper requested review of this revision.Apr 29 2023, 20:59

aklapper mentioned this in D25150: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Diffusion page.Apr 30 2023, 08:08

aklapper mentioned this in rP1b8294341916: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Diffusion page.Apr 30 2023, 13:13

valerio.bozzolan mentioned this in D25161: Fix PHP 8.1 "strlen(null)" exception which blocks rendering Upload SSH Public key settings dialog.Apr 30 2023, 22:52

aklapper mentioned this in rPe3ad37f79259: Fix PHP 8.1 "strlen(null)" exception which blocks rendering Upload SSH Public….Apr 30 2023, 23:08

valerio.bozzolan mentioned this in D25152: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Maniphest task creation page.May 1 2023, 08:16

valerio.bozzolan mentioned this in D25162: Fix PHP 8.1 "strlen(null)" exception which blocks rendering list of Active/All projects.May 1 2023, 08:21

valerio.bozzolan mentioned this in D25153: Fix PHP 8.1 "strlen(null)" exception which blocks rendering the Projects page (and log alien values).May 1 2023, 08:37

valerio.bozzolan edited the test plan for this revision. (Show Details)May 1 2023, 09:06

hoping to help future people, added small PHPDoc on a private method to clarify the $icon that is a string (name in Font Awesome)

Harbormaster completed remote builds in B315: Diff 588.May 1 2023, 12:09

valerio.bozzolan requested changes to this revision.May 1 2023, 12:49

valerio.bozzolan added inline comments.

src/applications/files/controller/PhabricatorFileDataController.php

✅ It seems $alt_uri->getDomain() is null as default and usages seems to just use setDomain(string).

The phutil_nonempty_string() will report alien values and this is OK.

src/applications/search/field/PhabricatorSearchDateField.php

WARNING: The check phutil_nonempty_string() is probably too much strict here, since this function also should accept integers. This is probably a problem for Conduit API consumers sending integers. Note that parseDateTime() accepts integers by design.

In short:

Value	`phutil_nonempty_scalar($v)`
`"foo"`	true
`""`	false
`null`	false
`"0"`	true
`"1"`	true
`0`	true
`1`	true
`0.5`	true
`obj` with tostring	true
`obj` withno tostr.	Exception
`array()`	Exception

For reference:

Value	`phutil_nonempty_string($v)`
`"foo"`	true
`""`	false
`null`	false
`"0"`	true
`"1"`	true
`0`	Exception
`1`	Exception
`0.5`	Exception
`obj` with tostring	Exception
`obj` withno tostr.	Exception
`array()`	Exception

WARNING: Same as above.

src/view/layout/AphrontSideNavFilterView.php

100

✅ I tested with phlog() and it seems $item is just the string name of Font Awesome icon, or null.

The phutil_nonempty_string() will report alien values and that is OK.

This revision now requires changes to proceed.May 1 2023, 12:49

valerio.bozzolan mentioned this in D25164: Fix PHP 8.1 "rawurlencode(null)" exception which blocks rendering a project page.May 1 2023, 13:36

aklapper mentioned this in rP313d3b7bf2ea: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the Maniphest task….May 1 2023, 13:52

aklapper mentioned this in rP6e8852837004: Fix PHP 8.1 "strlen(null)" exception which blocks rendering list of Active/All….May 1 2023, 13:55

aklapper mentioned this in T15318: PHP 8.1 "strlen(null)" exceptions block rendering the People page of a user.May 1 2023, 14:56

aklapper mentioned this in D25167: Fix PHP 8.1 "strlen(null)" exceptions which block rendering the People page of a user.May 1 2023, 14:57

aklapper mentioned this in T15319: PHP 8.1 "strlen(null)" exception blocks rendering user's Edit Profile page.May 1 2023, 15:08

aklapper mentioned this in D25168: Fix PHP 8.1 "strlen(null)" exception which blocks rendering user's Edit Profile page.May 1 2023, 15:09

aklapper mentioned this in T15321: PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions block typeahead completion proposals.May 1 2023, 17:44

aklapper mentioned this in D25170: Fix PHP 8.1 "strlen(null)" and "array_slice(null)" exceptions which block typeahead completion proposals.May 1 2023, 18:09

Fix PHP 8.1 "strlen(null)" exceptions which block rendering the People page

Summary:
strlen() was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts phutil_nonempty_string() as a replacement.

Use phutil_nonempty_scalar() instead in PhabricatorSearchDateField.php as
input could only be integer instead of string.

Closes T15297

Test Plan:
Applied these four changes (on top of D25144, D25145, D25146) and /people/ finally rendered in web browser.