Fix PHP 8.1 "file_exists(null)" exception rendering AphrontStackTraceView
ClosedPublic
Actions

Authored by aklapper on Jul 9 2024, 22:23.

Details

Reviewers

speck

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15881: PHP 8.1 "strlen(null)" exception rendering AphrontStackTraceView

Commits

rP59428f64ca2c: Fix PHP 8.1 "file_exists(null)" exception rendering AphrontStackTraceView

Summary

Passing null to file_exists() is deprecated behavior since PHP 8.1.
The already existing if ($file) check in AphrontStackTraceView implies that $file can indeed be empty.
Thus add another such check higher up in that class to avoid deprecation warnings when rendering stacktraces.

ERROR 8192: file_exists(): Passing null to parameter #1 ($filename) of type string is deprecated at [/var/www/html/phorge/arcanist/src/filesystem/Filesystem.php:1068];
  #0 file_exists(NULL) called at [<arcanist>/src/filesystem/Filesystem.php:1068];
  #1 Filesystem::pathExists(NULL) called at [<arcanist>/src/filesystem/Filesystem.php:1180];
  #2 Filesystem::assertExists(NULL) called at [<arcanist>/src/filesystem/Filesystem.php:1020];
  #3 Filesystem::isDescendant(NULL, string) called at [<phorge>/src/view/widget/AphrontStackTraceView.php:33];

Closes T15881

Test Plan

Intentionally inject an Array to string conversion bug in PhabricatorEmailPreferencesSettingsPanel (see T15881). Then visit /settings/panel/emailpreferences/. See the same stacktrace before and after applying this change, but see only a single line of output in DarkConsole / Error Log after applying this patch and not anymore numerous Passing null to parameter #1 ($filename) of type string is deprecated errors in DarkConsole / Error Log.

Diff Detail

Repository

rP Phorge

Branch

fileNoLikeNull

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1431
Build 1431: arc lint + arc unit

Event Timeline

aklapper created this revision.Jul 9 2024, 22:23

Owners added a reviewer: O1: Blessed Committers.Jul 9 2024, 22:23

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptJul 9 2024, 22:23

aklapper requested review of this revision.Jul 9 2024, 22:23

Harbormaster completed remote builds in B1431: Diff 2227.Jul 9 2024, 22:23

aklapper edited the summary of this revision. (Show Details)Jul 9 2024, 22:29

This might regress the case where file is “0”

Eh, the PHP pitfalls that I'll never learn by heart... Thanks! :)
Let's try if ($file !== null && $file !== '') instead of if ($file) then?

Harbormaster completed remote builds in B1435: Diff 2239.Jul 10 2024, 19:53

😆

This revision is now accepted and ready to land.Jul 10 2024, 21:54

aklapper retitled this revision from Fix PHP 8.1 "strlen(null)" exception rendering AphrontStackTraceView to Fix PHP 8.1 "file_exists(null)" exception rendering AphrontStackTraceView.Jul 11 2024, 05:21

aklapper edited the summary of this revision. (Show Details)

Closed by commit rP59428f64ca2c: Fix PHP 8.1 "file_exists(null)" exception rendering AphrontStackTraceView. · Explain WhyJul 11 2024, 05:25

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP59428f64ca2c: Fix PHP 8.1 "file_exists(null)" exception rendering AphrontStackTraceView.