revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-29 10:12:41 +01:00
Code Issues Releases Wiki Activity

Fix PHP 8.1 "strlen(null)" exceptions blocking account registration with custom OAuth provider after redirect

Browse source 
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 #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/provider/PhabricatorOAuth1AuthProvider.php:70]
```

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
  #0 <#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
This commit is contained in:
Andre Klapper 2023-08-11 21:02:44 +02:00
parent a2e8ab3180
commit 98dfac53ba
2 changed files with 6 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php
View file

@ -67,7 +67,7 @@ abstract class PhabricatorOAuth1AuthProvider
} }
$denied = $request->getStr('denied'); $denied = $request->getStr('denied');
if (strlen($denied)) { if ($denied) {
// Twitter indicates that the user cancelled the login attempt by // Twitter indicates that the user cancelled the login attempt by
// returning "denied" as a parameter. // returning "denied" as a parameter.
throw new PhutilAuthUserAbortedException(); throw new PhutilAuthUserAbortedException();

9
src/applications/auth/view/PhabricatorAuthAccountView.php
View file

@ -29,13 +29,14 @@ final class PhabricatorAuthAccountView extends AphrontView {
$realname = $account->getRealName(); $realname = $account->getRealName();
$use_name = null; $use_name = null;
if (strlen($dispname)) { if (phutil_nonempty_string($dispname)) {
$use_name = $dispname; $use_name = $dispname;
} else if (strlen($username) && strlen($realname)) { } else if (phutil_nonempty_string($username) &&
phutil_nonempty_string($realname)) {
$use_name = $username.' ('.$realname.')'; $use_name = $username.' ('.$realname.')';
} else if (strlen($username)) { } else if (phutil_nonempty_string($username)) {
$use_name = $username; $use_name = $username;
} else if (strlen($realname)) { } else if (phutil_nonempty_string($realname)) {
$use_name = $realname; $use_name = $realname;
} }