Page MenuHomePhorge

Fix PHP 8.1 "strlen(null)" exception trying to create an empty Menu Item

Authored by aklapper on Sun, May 14, 10:17.
Referenced Files
Unknown Object (File)
Sat, Jun 3, 22:44
Unknown Object (File)
Wed, May 31, 06:46
Unknown Object (File)
Tue, May 30, 00:05
Unknown Object (File)
Mon, May 29, 19:54
Unknown Object (File)
Sat, May 27, 17:43
F293337: Phorge home menu test.png
Sun, May 21, 07:41
F293335: Phorge menu items tests.png
Sun, May 21, 07:41
Unknown Object (File)
Sat, May 20, 12:43



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.

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.

Closes T15398

Test Plan

Applied this change and "Edit Menu Item" overlay dialog correctly shows "You must choose a project" resp. "You must choose a room." error message, now without an exception.

Go to /home/menu/configure/custom/ and also to /home/menu/configure/global/ and try:

  • Add Label with name 0 / 1 / 1.123 etc.
  • Add Label, Save again
  • Add Link, Save again
  • Add Application, Save again
  • Add Divider, Save again
  • Add Conpherence, Save again
  • Add Link, Save Again
  • Add Motivator (Cat Facts), Save again
  • Add Form, Save again
  • Order Items
  • Activate/Disable Built-in Home (unrelated)
  • Remove all the above

Also, tried on a Project menu.

Diff Detail

rP Phorge
Lint Not Applicable
Tests Not Applicable

Event Timeline

TBH, I'm confused about $xaction being treated as an array in line 126.

I was able to track it down to PhabricatorProfileMenuEditor::validateTransaction(), which is using "regular" transactions and converts them to an array in case the xaction is of type PhabricatorProfileMenuItemConfigurationTransaction::TYPE_PROPERTY.

At this point, I think the content might be generated in javascript, which means all bets are off.

To complete the testing, please try creating/moving/hiding/showing these kinds of menu items:

  • "Label" menu item with a label 0 and something else that might be converted to an int
  • a "Link" item
  • a "Divider" item

That should cover all complex/surprise possible values of $xaction['new'].


Thanks avivey, I share the same concerns. Hoping to be useful I've followed these tips to expand the test plan a bit.

I tested this locally. Kind of re-ordering stuff in Personal / Global menus and do fuzzy tests. Limited examples:

Phorge menu items tests.png (885×717 px, 54 KB)

Phorge home menu test.png (440×222 px, 11 KB)

I'm also available to hot-patch this immediately if somebody will report any future issue with any unknown alien corner case.



This revision is now accepted and ready to land.Sun, May 21, 07:41