search

joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.73k stars 3.64k forks source link

Wrong JPATH_PUBLIC for cli app #42120

Open rabidgrowth opened 11 months ago

rabidgrowth commented 11 months ago

Steps to reproduce the issue

Expected result

JPATH_PUBLIC is public directory. Example: /var/www/jbeta

Actual result

JPATH_PUBLIC is directory I putted joomla5 files inside before install. Example: /opt/joomla5

System information (as much as possible)

Ubuntu Server 22.04, Apache 2.4, MySQL 8.0, PHP 8.2

Additional comments

I think problem is same file as my issue 42114, libraries/src/Helper/PublicFolderGeneratorHelper.php. I see he makes new file defines.php inside new public directory. This work for site and administrator and api app, okay. He does not work for cli app. cli app load only JPATH_ROOT/includes/defines.php, he does not load JPATH_PUBLIC/defines.php. He does not know JPATH_PUBLIC!

I try fix, edit includes/defines.php. I am unhappy. After I upgrade joomla, poof, file reset, problem again. Not a fix.

I like public folder feature. But he is not "release candidate" feature still. I think he needs public folder in configuration.php. After load configuration.php, define JPATH_PUBLIC. Not define him inside the defines.php who load before configuration.php.

dgrammatiko commented 11 months ago

I'll have a look but fwiw the constant JPATH_PUBLIC needs to be declared way before loading the configuration iirc. Thanks for the feedback

@HLeithner how do you feel about introducing a CLI/joomla.php in the public folder (just define the JPATH_PUBLIC, load the defines and require the original joomla.php)?

@rabidgrowth I'm not so sure if the CLI code should be mixed with the served (public folder). I mean you can still execute the php file from the non public folder but anyways I think the maintainers should decide what's more convenient/hepful/expected behaviour...

rabidgrowth commented 11 months ago

I agree cli not in public. I fix by put cli/joomla.php in JPATH_PUBLIC yesterday. But bad for security! No do that! I said not this because bad. You and I agree.

But CLI app is not for only manage site database, no files. I don't agree. CLI app is for manage all site, database, files, what ever.

Example. I have console plugin who update .htaccess. Joomla 5, .htaccess now is inside JPATH_PUBLIC, no inside JPATH_ROOT. I don't know JPATH_PUBLIC inside CLI. I can not update .htaccess. Bad.

I looked again. I have new idea! May be, put new defines.php inside JPATH_ROOT/cli. This stuff:

\defined('_JEXEC') or die;
\defined('JPATH_PUBLIC') || \define('JPATH_PUBLIC', '/var/www/jbeta');
require __DIR__ . '/../includes/defines.php';

This solves problem. No bad things happen, yes? Need only small change inside your libraries/src/Helper/PublicFolderGeneratorHelper.php file. Can you think about this? Please? Thanks!

dgrammatiko commented 11 months ago

@rabidgrowth how about \defined('JPATH_PUBLIC') || \define('JPATH_PUBLIC', JPATH_ROOT);?

FWIW for the CLI instances JPATH_PUBLIC === JPATH_ROOT (unless you want to actually modify things on the public folder, which probably is not recommended [supported?])

rabidgrowth commented 11 months ago

I'm writing it with Google Translate because I don't think we understand each other.

In joomla 3 I had made a CLI script that extended JApplicationCli. Every hour I ran it with a CRON job. It downloads a list of blocked IPs from a central server and passes them into the .htaccess of each of my client's websites.

Because I read you will remove JApplicationCli in joomla5, I made it a console plugin in joomla4. I just changed the CRON jobs to go via cli/joomla.php.

This solution has been working for over 10 years. Therefore, it is supported.

The fact that a CLI script that only runs if you have SSH access or a CRON job is obviously more secure than a web script that is run by whoever has the URL is self-evident. Therefore, it is recommended.

Do you agree?

dgrammatiko commented 11 months ago

Do you agree?

Yes, the use case is totally legit

rabidgrowth commented 11 months ago

Thank you very much! If I can help further, let me know.

HLeithner commented 11 months ago

Finding a solution here would be great, writing it into the configuration.php might not be the right approach, we should keep this file as tidy as possible. Adding a variable which might only effect cli because the web variant doesn't use it can lead to confusion.

An alternative could be to add a defines.php for the cli, which is also not the nicest why. allowing to set an environment variable could be a solution. or a parameter like live_url. I'm not so sure about the best way currently.

dgrammatiko commented 11 months ago

Ok couple notes here: