Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#15011] - Remove check as JApplicationCli constructor has a check for this

? ? Pending

User tests: Successful: Unsuccessful:

avatar photodude
photodude
30 Mar 2017

Pull Request for Issue # Check is unnecessary as JApplicationCli constructor has a check for this.

Summary of Changes

Removes duplicated code

Testing Instructions

CLI scripts are still work in CLI and not accessible via things like web interface.

(code review might be sufficent)

Expected result

CLI scripts are still work in CLI and not accessible via things like web interface.

Actual result

CLI scripts are still work in CLI and not accessible via things like web interface.

Documentation Changes Required

none

avatar photodude photodude - open - 30 Mar 2017
avatar photodude photodude - change - 30 Mar 2017
Status New ⇒ Pending
avatar photodude photodude - change - 30 Mar 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2017
Category ⇒ CLI
avatar photodude
photodude - comment - 30 Mar 2017

Can someone rerun the appveyor 5.6 test? the Cachelite failure is unrelated to this PR

1) JCacheStorageCacheliteTest::testCacheTimeout
Some data should be available in lifetime.
Failed asserting that false matches expected 'testData'.
\tests\unit\core\case\cache.php:121
\tests\unit\suites\libraries\joomla\cache\storage\JCacheStorageCacheliteTest.php:66
avatar wilsonge
wilsonge - comment - 30 Mar 2017

You can't rerun appveyor tests :(

avatar mbabker
mbabker - comment - 30 Mar 2017

I have to do it from the bot account. I set it to rebuild.

avatar photodude
photodude - comment - 30 Mar 2017

Thanks @mbabker

avatar euismod2336 euismod2336 - test_item - 31 Mar 2017 - Tested successfully
avatar euismod2336
euismod2336 - comment - 31 Mar 2017

I have tested this item ✅ successfully on 3300a46

scripts remain inaccessible for the web as indeed the JApplicationCli does the same check.

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

avatar alikon alikon - test_item - 31 Mar 2017 - Tested successfully
avatar alikon
alikon - comment - 31 Mar 2017

I have tested this item ✅ successfully on 3300a46

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

avatar franz-wohlkoenig franz-wohlkoenig - change - 31 Mar 2017
Status Pending ⇒ Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 31 Mar 2017

RTC after two successful testes.

avatar wilsonge wilsonge - change - 31 Mar 2017
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2017-03-31 22:36:04
Closed_By ⇒ wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 31 Mar 2017
avatar wilsonge wilsonge - merge - 31 Mar 2017
avatar wilsonge
wilsonge - comment - 31 Mar 2017

It's not exactly the same check. But seems to be good enough for web accessibility

avatar photodude
photodude - comment - 1 Apr 2017

I believe the JApplicationCli constructor check is better than the PHP_SAPI check. As @nikosdion pointed out in another pr

You MUST NOT explicitly check for the CLI SAPI. Some hosts only provide access to the CGI SAPI but allow you to set the maximum execution time to 0, i.e. work almost identical to the CLI SAPI. In these cases you need to tweak the way input handling works.

I think the JApplicationCli constructor check fixes that issue too

Although his constructor check is probably a lot better

avatar nikosdion
nikosdion - comment - 1 Apr 2017

Please do not mention me on Joomla repositories.

Add a Comment

Login with GitHub to post a comment