From e46025f7a9146f9918bab9d6fbdf6ed1816db5b5 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Mon, 4 Dec 2023 19:27:47 -0800 Subject: [PATCH] Fix PHP 8.1 "urlencode(null)" exception blocking account registration redirect for custom OAuth provider Summary: It seems that a `tokenSecret` is not always passed at this stage, and that PHP's `urlencode()` does not accept passing a `null` string since PHP 8.1 (I could not find any upstream note about this but bug reports across the web seem to confirm this). Thus do not try to `urlencode($this->tokenSecret)` if it is `null`. ``` EXCEPTION: (RuntimeException) urlencode(): Passing null to parameter #1 ($string) of type string is deprecated at [/src/error/PhutilErrorHandler.php:261] arcanist(), ava(), phorge(), wmf-ext-misc() #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [/src/error/PhutilErrorHandler.php:261] #1 <#2> urlencode(NULL) called at [/src/future/oauth/PhutilOAuth1Future.php:232] ``` Closes T15589 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 Redirect now works as expected: The URL redirect to allow access on http://mediawiki.localhost/index.php?title=Special%3AOAuth%2Fauthorize&oauth_token=1234567890abcdef1234567890abcdef&oauth_consumer_key=1234567890abcdef1234567890abcdef works as expected, instead of showing a raw error page about `urlencode()` not accepting passing `null`. (After allowing authorization there are more issues in Phorge code but they are out of scope for this Arcanist patch.) Reviewers: O1 Blessed Committers, valerio.bozzolan, speck Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15589 Differential Revision: https://we.phorge.it/D25374 --- src/future/oauth/PhutilOAuth1Future.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/future/oauth/PhutilOAuth1Future.php b/src/future/oauth/PhutilOAuth1Future.php index 8edd6c26..f73b9db0 100644 --- a/src/future/oauth/PhutilOAuth1Future.php +++ b/src/future/oauth/PhutilOAuth1Future.php @@ -229,7 +229,10 @@ final class PhutilOAuth1Future extends FutureProxy { $consumer_secret = $this->consumerSecret->openEnvelope(); } - $key = urlencode($consumer_secret).'&'.urlencode($this->tokenSecret); + $key = urlencode($consumer_secret).'&'; + if ($this->tokenSecret !== null) { + $key .= urlencode($this->tokenSecret); + } switch ($this->signatureMethod) { case 'HMAC-SHA1':