Page MenuHomePhorge
Differential D25267

Fix PHP 8.2 "trim(null)" exception which causes Conduit's user.whoami to fail
AbandonedPublic
Actions

Authored by aklapper on Jun 1 2023, 19:05.

Details

Reviewers
speck
valerio.bozzolan
mturdus
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15469: PHP 8.1 "strlen(null)" exception calling Conduit's user.whoami
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.

Stack trace
[Thu Jun 01 19:47:05.238882 2023] [php:notice] [::1:39392] [2023-06-01 17:47:05] EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
[Thu Jun 01 19:47:05.239220 2023] [php:notice] [::1:39392] arcanist(head=master, ref.master=18554ea76ceb), phorge(head=merula, ref.master=e11c5486c92b, ref.merula=9c37f16f7a63, custom=1)
[Thu Jun 01 19:47:05.239234 2023] [php:notice] [::1:39392]   #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/conduit/controller/PhabricatorConduitAPIController.php:686]
[Thu Jun 01 19:47:05.239241 2023] [php:notice] [::1:39392]   #1 <#2> PhabricatorConduitAPIController::decodeConduitParams(AphrontRequest, string) called at [<phorge>/src/applications/conduit/controller/PhabricatorConduitAPIController.php:28]
[Thu Jun 01 19:47:05.239260 2023] [php:notice] [::1:39392]   #2 phlog(RuntimeException) called at [<phorge>/src/applications/conduit/controller/PhabricatorConduitAPIController.php:111]
[Thu Jun 01 19:47:05.239266 2023] [php:notice] [::1:39392]   #3 PhabricatorConduitAPIController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
[Thu Jun 01 19:47:05.239270 2023] [php:notice] [::1:39392]   #4 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
[Thu Jun 01 19:47:05.239275 2023] [php:notice] [::1:39392]   #5 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]
Test Plan

The following python script executes the user.whoami conduit call

user.whoami.py
import json
import requests
import sys

phabricator_url = sys.argv[1];
api_token = sys.argv[2];

def call_conduit(method, params):
    params['api.token'] = api_token
    response = requests.post(f'{phabricator_url}/api/{method}', data=params)
    return json.loads(response.text)

result = call_conduit('user.whoami', {})

print(result)
Execution in bash
export phabricator_url=http://phorge.blackbird.turdus.localphabricator_url
export conduit_api=api-abcdef1g2hijk45lmnop6qrs7tu8
python3 user.whoami.py $phabricator_url $conduit_api

Diff Detail

Repository
rP Phorge
Branch
arcpatch-D25267
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 565
Build 565: arc lint + arc unit

Event Timeline

mturdus created this revision.Jun 1 2023, 19:05
Owners added a reviewer: O1: Blessed Committers.Jun 1 2023, 19:05
Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJun 1 2023, 19:05
mturdus requested review of this revision.Jun 1 2023, 19:05
Harbormaster completed remote builds in B495: Diff 863.Jun 1 2023, 19:05
speck accepted this revision.Jun 8 2023, 00:32
This revision is now accepted and ready to land.Jun 8 2023, 00:32
valerio.bozzolan accepted this revision.Jun 15 2023, 10:30

Thanks again for this patch

If you can, please try to "land" this :)

In short, using Arcanist, you can download the patch, and then "land" that into the master branch:

arc patch D25267
arc land
mturdus added a comment.Jun 15 2023, 18:19

I'm sorry I didn't know I had to more.

I'm totally new with this arc tool.
The arc patch command did work, but the arc land command returned this:

 STRATEGY  Merging with "squash" strategy, the default strategy.
 SOURCE  Landing the current branch, "arcpatch-D25267".
 ONTO REMOTE  Landing onto remote "origin", the default remote under Git.
 ONTO TARGET  Landing onto target "master", the default target under Git.
 INTO REMOTE  Will merge into remote "origin" by default, because this is the remote the change is landing onto.
 INTO TARGET  Will merge into target "master" by default, because this is the "onto" target.
 FETCH  Fetching "master" from remote "origin"...

  $   git fetch --no-tags --quiet -- origin master


 INTO COMMIT  Preparing merge into "master" from remote "origin", at commit "c300f0e87416".

 <!> UNKNOWN REVISION 
Unable to determine which revision is associated with commit "9c37f16f7a63".
Use "arc diff" to create or update a revision with this commit, or
"--revision" to force selection of a particular revision.

 USAGE EXCEPTION  Unable to determine revision for commit "9c37f16f7a63".

I guess it didn't work...

git status returns this:

On branch arcpatch-D25267
nothing to commit, working tree clean

and git log shows the D25267 commit at HEAD

I don't know what to do here.

valerio.bozzolan added a comment.Jun 15 2023, 21:36

OK don't worry, you are almost there!

In that branch, just try updating this page first:

arc diff --update D25267

And then retry:

arc land
mturdus updated this revision to Diff 973.Jun 16 2023, 04:26

Updating D25267: Fix PHP 8.2 "trim(null)" exception which causes Conduit's user.whoami to fail

Harbormaster completed remote builds in B565: Diff 973.Jun 16 2023, 04:26
mturdus added a comment.Jun 16 2023, 04:31

I uncommented a line in the editor executed by arc diff and it seemed to work

The arc land however returned this:

 STRATEGY  Merging with "squash" strategy, the default strategy.
 SOURCE  Landing the current branch, "arcpatch-D25267".
 ONTO REMOTE  Landing onto remote "origin", the default remote under Git.
 ONTO TARGET  Landing onto target "master", the default target under Git.
 INTO REMOTE  Will merge into remote "origin" by default, because this is the remote the change is landing onto.
 INTO TARGET  Will merge into target "master" by default, because this is the "onto" target.
 FETCH  Fetching "master" from remote "origin"...

  $   git fetch --no-tags --quiet -- origin master


 INTO COMMIT  Preparing merge into "master" from remote "origin", at commit "c300f0e87416".

 <!> UNKNOWN REVISION 
Unable to determine which revision is associated with commit "9c37f16f7a63".
Use "arc diff" to create or update a revision with this commit, or
"--revision" to force selection of a particular revision.

 USAGE EXCEPTION  Unable to determine revision for commit "9c37f16f7a63".
valerio.bozzolan added a task: T15469: PHP 8.1 "strlen(null)" exception calling Conduit's user.whoami.Jun 16 2023, 08:00
valerio.bozzolan added a subscriber: aklapper.Jun 16 2023, 08:03
__________________________________ < Overlap detected! Already fixed! > ---------------------------------- \ ^__^ \ (oo)\_______ (__)\ )\/\ ||----w | || ||

Mistery resolved! Git was right. The patch was not successfully land-able since the kind user @aklapper already landed a fix for this 3 days ago

rP1c098c273d06: Fix PHP 8.1 "strlen(null)" exception calling Conduit's user.whoami

Ouch :D

Thank you very much anyway @mturdus - For consolation, you unlocked this command:

arc anoid
mturdus added a comment.Jun 16 2023, 16:33
In D25267#8682, @valerio.bozzolan wrote:
__________________________________ < Overlap detected! Already fixed! > ---------------------------------- \ ^__^ \ (oo)\_______ (__)\ )\/\ ||----w | || ||

Mistery resolved! Git was right. The patch was not successfully land-able since the kind user @aklapper already landed a fix for this 3 days ago

rP1c098c273d06: Fix PHP 8.1 "strlen(null)" exception calling Conduit's user.whoami

Ouch :D

Thank you very much anyway @mturdus - For consolation, you unlocked this command:

arc anoid

😄

valerio.bozzolan added a comment.Jul 24 2023, 09:17

Do you want an help to land this? (No problem, in case)

mturdus added a comment.Jul 24 2023, 14:30

I'm not sure if I understand your question.
I thought the fix was already implemented by aklapper?
What do you want me to do?

aklapper commandeered this revision.Dec 17 2023, 10:54
aklapper added a reviewer: mturdus.

Abandoning as superseded by rP1c098c273d06ea3cf8215245ea109f41c59593d9 (which first requires commandeering)

aklapper abandoned this revision.Dec 17 2023, 10:54

Revision Contents
Changeset List

PathSizePackages
src/
applications/
conduit/
controller/
2 linesO1: Blessed Committers

Diff 973

View Options

src/applications/conduit/controller/PhabricatorConduitAPIController.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