Skip to content

Commit 98dfac5

Browse files
committed
Fix PHP 8.1 "strlen(null)" exceptions blocking account registration with custom OAuth provider after redirect
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. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(), ava(), phorge(), wmf-ext-misc() #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/provider/PhabricatorOAuth1AuthProvider.php:70] ``` ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(), ava(), phorge(), wmf-ext-misc() #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/view/PhabricatorAuthAccountView.php:32] ``` Closes T15590 Test Plan: * As an admin, set up custom "MediaWiki" OAuth provider from from https://gitlab.wikimedia.org/-/ide/project/repos/phabricator/extensions/edit/wmf/stable/-/src/oauth/ * As an admin, apply D25373 * As a user, go to `/auth/login/mediawiki:whatever/` * Select login button * Allow authentication on third-party site * Get redirected to Phorge instance Phorge user account registration page "Create a New Account" at `/auth/register/abcdefghijklmnopqrstuvwxyz0123456/` now renders as expected, instead of displaying errors only. Reviewers: O1 Blessed Committers, Matthew Reviewed By: O1 Blessed Committers, Matthew Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15590 Differential Revision: https://we.phorge.it/D25375
1 parent a2e8ab3 commit 98dfac5

File tree

2 files changed

+6
-5
lines changed

2 files changed

+6
-5
lines changed

src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function processLoginRequest(
6767
}
6868

6969
$denied = $request->getStr('denied');
70-
if (strlen($denied)) {
70+
if ($denied) {
7171
// Twitter indicates that the user cancelled the login attempt by
7272
// returning "denied" as a parameter.
7373
throw new PhutilAuthUserAbortedException();

src/applications/auth/view/PhabricatorAuthAccountView.php

+5-4
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@ public function render() {
2929
$realname = $account->getRealName();
3030

3131
$use_name = null;
32-
if (strlen($dispname)) {
32+
if (phutil_nonempty_string($dispname)) {
3333
$use_name = $dispname;
34-
} else if (strlen($username) && strlen($realname)) {
34+
} else if (phutil_nonempty_string($username) &&
35+
phutil_nonempty_string($realname)) {
3536
$use_name = $username.' ('.$realname.')';
36-
} else if (strlen($username)) {
37+
} else if (phutil_nonempty_string($username)) {
3738
$use_name = $username;
38-
} else if (strlen($realname)) {
39+
} else if (phutil_nonempty_string($realname)) {
3940
$use_name = $realname;
4041
}
4142

0 commit comments

Comments
 (0)