Page MenuHomePhorge

Fix PHP 8.1 "urlencode(null)" exception blocking account registration redirect for custom OAuth provider
Needs ReviewPublic

Authored by aklapper on Aug 5 2023, 20:28.
Referenced Files
Unknown Object (File)
Thu, Sep 21, 08:03
Unknown Object (File)
Mon, Sep 4, 12:41
Unknown Object (File)
Mon, Sep 4, 10:59
Unknown Object (File)
Sun, Sep 3, 11:30
Unknown Object (File)
Sun, Sep 3, 06:41
Unknown Object (File)
Sat, Sep 2, 02:08
Unknown Object (File)
Wed, Aug 30, 02:16
Unknown Object (File)
Wed, Aug 30, 00:35



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 [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> urlencode(NULL) called at [<arcanist>/src/future/oauth/PhutilOAuth1Future.php:232]

Closes T15589

Test Plan

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.)

Diff Detail

rARC Arcanist
Lint Passed
Tests Passed
Build Status
Buildable 807
Build 807: arc lint + arc unit

Event Timeline

aklapper requested review of this revision.Aug 5 2023, 20:28
Matthew requested changes to this revision.Aug 7 2023, 03:12

Please fix unit errors

This revision now requires changes to proceed.Aug 7 2023, 03:12

Please fix unit errors

I've looked at this for five minutes and unfortunately I'm afraid that I do not know.


Thanks. Does this work as well? (Also reported below to make it more visible)

$key = urlencode($consumer_secret);
if ($this->tokenSecret !== null) {
  $key .= '&' . urlencode($this->tokenSecret);

No, this does not work as well. In that case I get an Expected 'oauth_callback_confirmed' to be 'true'! error. But your format is more readable, so I should adjust my proposal.

aklapper marked an inline comment as done and an inline comment as not done.Aug 20 2023, 11:22


This seems the exact backward-compatible thing to me


Please fix unit errors

Yeah! it seems OK now