Paths

Table of Contentst

Differential D25227

Fix PHP 8.1 "strlen(null)" exception which blocks rendering "Browse Dashboards" dialog
ClosedPublic
Actions

Authored by aklapper on May 13 2023, 11:24.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15396: PHP 8.1 "strlen(null)" exception blocks rendering "Browse Dashboards" dialog

Commits

rP402a63c5de96: Fix PHP 8.1 "strlen(null)" exception which blocks rendering "Browse Dashboards"…

Summary

strlen() was used in Phabricator to check if a generic value is a non-empty string.

But, that function does not accept null anymore since PHP 8.1.

Closes T15396

Test Plan

Applied this change (on top of D25179 and D25226) and the "Browse Dashboards" dialog got rendered in web browser.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.May 13 2023, 11:24

Owners added a reviewer: O1: Blessed Committers.May 13 2023, 11:24

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

aklapper requested review of this revision.May 13 2023, 11:24

Harbormaster completed remote builds in B410: Diff 734.May 13 2023, 11:25

valerio.bozzolan requested changes to this revision.May 13 2023, 22:54

valerio.bozzolan added inline comments.

src/applications/search/compiler/PhutilSearchQueryCompiler.php
107	Wow, finally a place where `strlen()` was really needed! Please just do something like this before the strlen: if($query === null) { $query = ''; }

This revision now requires changes to proceed.May 13 2023, 22:54

Uh, that unit test was useful indeed, and I know understand the rest of the code.

Harbormaster completed remote builds in B414: Diff 738.May 13 2023, 23:59

valerio.bozzolan mentioned this in Wikimedia Hackaton 2023 Athens Code Sprint.May 18 2023, 10:16

valerio.bozzolan edited the summary of this revision. (Show Details)May 19 2023, 08:35

Thanks for your update

I tested this locally without any implosion even if I was honestly not able to reproduce a null case while visiting the /dashboard/ page. Anyway, this is 100% legit to me.

lgtm

(#WikimediaHackaton)

This revision is now accepted and ready to land.May 19 2023, 08:54

Closed by commit rP402a63c5de96: Fix PHP 8.1 "strlen(null)" exception which blocks rendering "Browse Dashboards"…. · Explain WhyMay 19 2023, 10:00

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP402a63c5de96: Fix PHP 8.1 "strlen(null)" exception which blocks rendering "Browse Dashboards"….

Revision Contents
Changeset List

Path

Size

Packages

src/

applications/

search/

compiler/

PhutilSearchQueryCompiler.php

4 lines

O1: Blessed Committers

Diff 741

src/applications/search/compiler/PhutilSearchQueryCompiler.php

Loading...