Page MenuHomePhorge
Differential D25216

Fix PHP 8.1 "strlen(null)" exceptions setting up Username/Password Authentication Provider
ClosedPublic
Actions

Authored by aklapper on May 10 2023, 18:09.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 06:13
Unknown Object (File)
Fri, Apr 12, 04:54
Unknown Object (File)
Thu, Apr 11, 10:25
Unknown Object (File)
Thu, Apr 11, 07:33
Unknown Object (File)
Sun, Apr 7, 06:17
Unknown Object (File)
Sun, Apr 7, 00:31
Unknown Object (File)
Mon, Apr 1, 01:38
Unknown Object (File)
Mon, Apr 1, 01:38
View All 81 Files
Subscribers
Cigaryno
Matthew
speck
tobiaswiese
valerio.bozzolan

Details

Reviewers
valerio.bozzolan
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15383: PHP 8.1 "strlen(null)" exceptions setting up Username/Password Authentication Provider
Commits
rP678c7a78be43: Fix PHP 8.1 "strlen(null)" exceptions setting up Username/Password…
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.

Closes T15383

Test Plan

Applied these two changes; afterwards "Username/Password" page at /auth/config/view/1/ gets rendered in web browser.

Additional tests (you need D25231 to complete this test plan):

  • Create WordPress Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Twitter Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Amazon Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Asana Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Bitbucket Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Disqus Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Disqus Auth; Edit; Disable Enable (and Disable again to cleanup)
  • Create Facebook Auth; Edit; Disable Enable (and Disable again to cleanup)
  • Create GitHub Auth; Edit; Disable Enable (and Disable again to cleanup)
  • Create JIRA Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Google Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create LDAP Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Phorge Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Slack Auth; Edit; Disable; Enable (and Disable again to cleanup)
  • Create Twitch.tv Auth; Edit; Disable; Enable (and Disable again to cleanup)

Diff Detail

Repository
rP Phorge
Branch
setupAuthProv (branched from master)
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 393
Build 393: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
67 msPhabricatorAuthSSHKeyTestCase::testRevokeSSHKey
EXCEPTION (RuntimeException): Implicit conversion from float 76.50000000000001 to int loses precision #0 /var/www/html/phorge/phorge/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php(192): PhutilErrorHandler::handleError(8192, '...', '...', 192) #1 /var/www/html/phorge/phorge/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php(148): PhabricatorFilesComposeAvatarBuiltinFile::rgba2gd(Array)
371 msPhabricatorAuthPasswordTestCase::testCompare
2 assertions passed.
21 msPhabricatorAuthPasswordTestCase::testPasswordBlocklisting
15 assertions passed.
1,718 msPhabricatorAuthPasswordTestCase::testPasswordEngine
20 assertions passed.
323 msPhabricatorAuthPasswordTestCase::testPasswordUpgrade
9 assertions passed.
View Full Test Results (1 Failed · 11 Passed)

Event Timeline

aklapper created this revision.May 10 2023, 18:09
Owners added a reviewer: O1: Blessed Committers.May 10 2023, 18:09
Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptMay 10 2023, 18:09
aklapper requested review of this revision.May 10 2023, 18:09
Harbormaster completed remote builds in B393: Diff 712.May 10 2023, 18:09
valerio.bozzolan mentioned this in Wikimedia Hackaton 2023 Athens Code Sprint.May 18 2023, 10:16
valerio.bozzolan mentioned this in T15402: Auth Messages: allow to reset custom message to default.May 19 2023, 11:20
valerio.bozzolan updated this revision to Diff 744.May 19 2023, 14:48

run arc lint again

Harbormaster completed remote builds in B418: Diff 744.May 19 2023, 14:49
valerio.bozzolan edited the test plan for this revision. (Show Details)May 19 2023, 15:27
valerio.bozzolan accepted this revision.May 19 2023, 15:30

Thanks for this patch (again)!

Believe me or not but I tested this intensively, following the test plan. If you don't believe me:

Phorge Auth providers used.png (1×869 px, 172 KB)

I hope this will not explode in the future like a nuclear bomb as we usually see then.

sgtm

This revision is now accepted and ready to land.May 19 2023, 15:30
Closed by commit rP678c7a78be43: Fix PHP 8.1 "strlen(null)" exceptions setting up Username/Password…. · Explain WhyMay 19 2023, 18:16
This revision was automatically updated to reflect the committed changes.
aklapper added a commit: rP678c7a78be43: Fix PHP 8.1 "strlen(null)" exceptions setting up Username/Password….

Revision Contents
Changeset List

PathSizePackages
src/
applications/
auth/
controller/
config/
4 linesO1: Blessed Committers

Diff 712

View Options

src/applications/auth/controller/config/PhabricatorAuthEditController.php

Loading...
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0