Fix PHP 8.1 "strlen(null)" exceptions adding an OAuth provider
ClosedPublic
Actions

Authored by aklapper on Apr 15 2024, 15:00.

Details

Reviewers

speck

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15786: PHP 8.1 "strlen(null)" exceptions adding an OAuth provider

Commits

rP81653abb5440: Fix PHP 8.1 "strlen(null)" exceptions adding an OAuth provider

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.

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.

ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php:140]

ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php:155]

ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php:165]

Closes T15786

Test Plan

Set up any Auth provider which uses OAuth2 and check the error console.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Apr 15 2024, 15:00

Owners added a reviewer: O1: Blessed Committers.Apr 15 2024, 15:00

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptApr 15 2024, 15:00

aklapper requested review of this revision.Apr 15 2024, 15:00

Harbormaster completed remote builds in B1170: Diff 1848.Apr 15 2024, 15:00

aklapper edited the test plan for this revision. (Show Details)Apr 15 2024, 15:02

If we find 10 minutes, let's test this on every provider, as last time it gave us a little surprise on one of them

If we find 10 minutes, let's test this on every provider, as last time it gave us a little surprise on one of them

I checked this with two or three providers - it's used in a common string for all OAuth providers and refers to an ID, Secret, or Property note.

But if we ever ran into some surprise here (e.g. getting an integer instead of a string) then we'd know to change phutil_nonempty_string into phutil_nonempty_scalar? :)

Thank you for testing. I think this looks good. In the one case that the old var is used it’s rendered as a string in pht so string assumption seems okay here.

I’m good with this if @valerio.bozzolan is

This revision is now accepted and ready to land.Wed, May 1, 23:12

Closed by commit rP81653abb5440: Fix PHP 8.1 "strlen(null)" exceptions adding an OAuth provider. · Explain WhyFri, May 3, 13:10

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP81653abb5440: Fix PHP 8.1 "strlen(null)" exceptions adding an OAuth provider.