Fix PHP 8.1 null exceptions which block rendering tab panels on dashboards
ClosedPublic
Actions

Authored by aklapper on May 28 2023, 13:02.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15346: PHP 8.1 exceptions in JsonLint block rendering Tab Panels in Dashboards

Commits

rARCb325304b6e52: Fix PHP 8.1 null exceptions which block rendering tab panels on dashboards

Summary

Passing null as input strings to substr() and preg_match() is deprecated in PHP 8.
Thus do not call substr() when input is null and pass an empty string instead of null to preg_match(). (Not calling preg_match() at all here would lead to Exception: Lexical error on line 1. Unrecognized text. ^).

Closes T15346

Test Plan

After applying these three changes and following the steps in T15346, tab on the dashboard displays "This tab panel does not have any tabs yet." as expected instead of the previous RuntimeException.

Diff Detail

Repository

rARC Arcanist

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.May 28 2023, 13:02

Owners added a reviewer: O1: Blessed Committers.May 28 2023, 13:02

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 28 2023, 13:02

aklapper requested review of this revision.May 28 2023, 13:02

Harbormaster completed remote builds in B468: Diff 824.May 28 2023, 13:02

valerio.bozzolan added inline comments.May 29 2023, 19:26

externals/jsonlint/src/Seld/JsonLint/JsonParser.php
484	✅ Verified. This involves a PHP pitfall but it's OK: the string `"0"` cannot never match the Byte Order Mark. Nice short-circuit.
externals/jsonlint/src/Seld/JsonLint/Lexer.php
97	🔶 Maybe problematic since the string `"0"` will be skipped and maybe it should not. Proposed minimal change: if ($this->input !== null && strlen($next) < 20) {
131–133	✅ Verified. This is the minimal change to have an usable non-deprecated input to be used against whatever regex rule.

aklapper mentioned this in T15432: PHP 8.1 "strlen(null)" exceptions block rendering Differential Revision page.May 30 2023, 10:59

Make check in getUpcomingInput() also cover string "0" (as requested by valerio.bozzolan)

Harbormaster completed remote builds in B490: Diff 855.May 30 2023, 11:27

This now partially overlaps with D25269

valerio.bozzolan mentioned this in D25269: Fix PHP 8.2 "strlen(null)" exceptions block rendering Differential Revision page (T15432 - 2/2).Jun 2 2023, 14:19

Thanks again for this patch

Hoping to be useful I will just keep the last newline as-is, but everything else seems good to me to avoid NULL/undefined. Also better than other similar patches on the same files.

lgtm