Skip to content

Fix PHP notices in CLI coming from the WebAuthn plugin#38340

Merged
wilsonge merged 4 commits intojoomla:4.2-devfrom
nikosdion:fix/webauthn-notices-cli
Sep 5, 2022
Merged

Fix PHP notices in CLI coming from the WebAuthn plugin#38340
wilsonge merged 4 commits intojoomla:4.2-devfrom
nikosdion:fix/webauthn-notices-cli

Conversation

@tampe125
Copy link
Copy Markdown
Contributor

Summary of Changes

This PR aims to fix some PHP notices when running under CLI. In that case, if a form is loaded (for example to gather the default data), the onContentPrepareForm event is fired. The WebAuthn plugin kicks in, tries to read the current URL and a notice is raised.
This PR will double check that we are in a web environment before continuing the execution

Testing Instructions

The best way to test it is to have a small script that tries to load an XML form. Sadly at the moment I don't have any, I came across this bug by developing CLI commands for the next version of Admin Tools.

Actual result BEFORE applying this Pull Request

PHP notice:

Notice: Undefined index: HTTP_HOST in /var/www/html/temp/libraries/src/Uri/Uri.php on line 103

Expected result AFTER applying this Pull Request

No PHP notice

Documentation Changes Required

None

@alikon
Copy link
Copy Markdown
Contributor

alikon commented Jul 29, 2022

I have tested this item ✅ successfully on 25c73da


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38340.

@RickR2H
Copy link
Copy Markdown
Member

RickR2H commented Aug 23, 2022

I have tested this item ✅ successfully on 25c73da

Tested by code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38340.

@RickR2H
Copy link
Copy Markdown
Member

RickR2H commented Aug 23, 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38340.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 23, 2022
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Sep 3, 2022

Is this one still valid with #38524 being merged?

@laoneo laoneo added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Sep 4, 2022
@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Sep 5, 2022

It will be in old style CLI apps. Plus it's better that this doesn't apply in API Apps anyhow - which even though it doesn't error - currently web auth will still run without this patch

@wilsonge wilsonge merged commit 7467cea into joomla:4.2-dev Sep 5, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 5, 2022
@wilsonge wilsonge added this to the Joomla! 4.2.3 milestone Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Updates Requested Indicates that this pull request needs an update from the author and should not be tested.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants