? ? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
18 May 2021

Summary of Changes

StackOverflow emailed me this https://joomla.stackexchange.com/questions/29123/cli-directory-is-publicly-accessible-is-it-supposed-to-be-is-that-safe/29181

They were correct, that Joomla 4 did not have any test for CLI use when the scripts were called from the web.

The code proposed is back ported from the Joomla-framework Joomla\Console\Application constructor and so should be "good enough"

Testing Instructions

Go to http://example.com/cli/joomla.php in a web browser.

Actual result BEFORE applying this Pull Request

With debug on/error reporting = maximum
Screenshot 2021-05-18 at 18 17 17

With debug off/error_reporting none

Screenshot 2021-05-18 at 18 18 00

Expected result AFTER applying this Pull Request

White screen of death - which is a good thing.

Documentation Changes Required

// @joomla/security @wilsonge

avatar PhilETaylor PhilETaylor - open - 18 May 2021
avatar PhilETaylor PhilETaylor - change - 18 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 May 2021
Category Libraries
avatar HLeithner
HLeithner - comment - 18 May 2021

is there a reason why you not simple do if (\php_sapi_name() === 'cli') { ?

avatar PhilETaylor
PhilETaylor - comment - 18 May 2021

Just like I stated:

The code proposed is back ported from the Joomla-framework Joomla\Console\Application constructor and so should be "good enough"

avatar PhilETaylor
PhilETaylor - comment - 18 May 2021

It should also be noted that its the same code that is currently used in Joomla 3

// Close the application if we are not executed from the command line.

avatar PhilETaylor PhilETaylor - change - 18 May 2021
The description was changed
avatar PhilETaylor PhilETaylor - edited - 18 May 2021
avatar richard67 richard67 - test_item - 18 May 2021 - Tested successfully
avatar richard67
richard67 - comment - 18 May 2021

I have tested this item successfully on 9926dbc


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

avatar HLeithner
HLeithner - comment - 18 May 2021

Just like I stated:

The code proposed is back ported from the Joomla-framework Joomla\Console\Application constructor and so should be "good enough"

^^ ok sorry missed this part, looks strange anyway but ok

avatar PhilETaylor
PhilETaylor - comment - 18 May 2021

Im not saying its perfect, but at least its consistent :)

There are a few uses of php_sapi_name in Joomla 3 but the only use of php_sapi_name in Joomla 4 appears to be in vendor libs.

avatar alikon alikon - test_item - 18 May 2021 - Tested successfully
avatar alikon
alikon - comment - 18 May 2021

I have tested this item successfully on 9926dbc


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

avatar alikon
alikon - comment - 18 May 2021

perfectible but better than before

avatar alikon alikon - change - 18 May 2021
The description was changed
Status Pending Ready to Commit
avatar alikon
alikon - comment - 18 May 2021

RTC


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

avatar joomla-cms-bot joomla-cms-bot - edited - 18 May 2021
avatar PhilETaylor PhilETaylor - change - 18 May 2021
Labels Added: ? ?
avatar richard67 richard67 - alter_testresult - 18 May 2021 - richard67: Tested successfully
avatar richard67 richard67 - alter_testresult - 18 May 2021 - alikon: Tested successfully
avatar richard67
richard67 - comment - 18 May 2021

Previous tests and RTC are still valid since last commit was just a change in a code comment.

avatar wilsonge wilsonge - close - 19 May 2021
avatar wilsonge wilsonge - merge - 19 May 2021
avatar wilsonge wilsonge - change - 19 May 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-05-19 22:28:23
Closed_By wilsonge
Labels Added: ?
Removed: ?
avatar wilsonge
wilsonge - comment - 19 May 2021

Somewhat unconvinced this belongs here vs in the entry file itself for the cli. But better than nothing for sure. easy to move around in the future. Thanks!

avatar PhilETaylor
PhilETaylor - comment - 19 May 2021

its a direct copy of the same code in a constructor in the CliApplication.php

// Close the application if we are not executed from the command line.

avatar wilsonge
wilsonge - comment - 19 May 2021

Sure but in that place every CLI app had a different entry point. In J4 the one true way (xD) is that we'll have a single entry point like site/admin is - so should be a bit simpler to manage and less confusing for 3rd party devs who don't need to look at the file. But anyhow it's fine for now

Add a Comment

Login with GitHub to post a comment