User tests: Successful: Unsuccessful:
Pull Request for Issue #31982 and #33019 and #31857 (comment)
On page load, and on demand, Joomla 4 will attempt to write autoload_psr4.php to the JPATH_CACHE folder.
If that folder is unwritable for any reason (many reasons, incorrect permissions, doesnt exist, or needs a FTP Layer to be writable) then Joomla will crash and cannot be used, doesnt display any helpful error, shows a 500 Internal Server Error page. Its a dead end.
This PR, catches the failure of the creation of a physical file, and caches the mapping of the namespace to the path in memory and logs (if enabled) a note to the Joomla log to report the failure of file creation.
Of course it would be more performant to not then have to regenerate the map each page load, but equally, we dont want Joomla to "crash" just because it cannot write an important file.
If the JPATH_CACHE folder exists and is writable, then the code in this PR is never used and full performance is kept, thus making this backward compatible.
Many different ways, but quickest is to install
totally unusable Joomla if JPATH_CACHE path is unwritable.
totally stable Joomla if JPATH_CACHE path is unwritable.
none.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
?
|
I have tested this item
After chmod 000 /administrator/cache and applying the path, Joomla is still unusable.
I see in frontend Casiopeia header but a message "The requested page can't be found".
Backend is unusable too: error 500.
Maybe Am I doing something wrong?
@pabloarias Thanks for testing.
You need to test with Debug = Off and Error Reporting = None in Joomla Global Config. This is the "normal user live site" configuration.
If you have debug on, or error reporting set high (development) then you will correctly get the 500 Internal Server Error message and the (new) red screen of death.
I'm testing now with Debug = Off and Error Reporting = None.
I obtain in frontend "The requested page can't be found".
Backend is still unusable with 500 error.
Sorry, something else wrong? Thank you very much.
ok that is because you HAVE a /administrator/cache/autoload_psr4.php file but it cannot be read. That is out of scope of this PR which seeks to prevent if Joomla cannot write to the folder, not read from it (and this PR is part of the FTP Layer fixes later on)
So do this:
chmod 777 /administrator/cache/
rm /administrator/cache/autoload_psr4.php
chmod 000 /administrator/cache/
and then you will be able to test this PR and the admin console will work
As you mentioned, it would cause performance issue because each page load, the system will have to read, parse xml files from all our extensions to create namespace map and users won't know about this.
To me, I would let the error happens so that users will know and fix this error instead. Just my personal option.
Yes it really is suboptimal, however without it Joomla! cannot be installed using FTP layer because it cannot write files until after it has asked for the credentials for FTP, and in order to do that it needs to boot which attempts to create this file.
I don't think it is optimal that Joomla! should crash just because it cannot write one file to the hard disk - this PR provides a workaround
99.999% of sites will never use this code, will never use in memory version of this file, because modern professional web hosting will allow the file to be written - but this PR will handle the edge case when it cannot be written
Well if the decision to keep the FTP Layer doesn't change - then this is the only way, because without this file Joomla cannot boot, and without booting Joomla cannot install over FTP because it cannot ask for the FTP Credentials.
Their sites would ONLY be slow if they had a READ ONLY file system that did not allow the file writes - for example if they did not store their FTP Credentials in Joomla Global Configuration and then used (other currently broken) features of Joomla (Like extension install over FTP) that made a change that needed the namespacemap to change, and Joomla could not write the file.
Its an edge case. No one in the real world would ever experience a slow site due to this PR handling the edge case.
Ok. So this is a fallback to cache not being writeable. So I'm ok with this PR from that perspective.
To address your concerns @joomdonation :
I think we need to catch the exception from adding the log entry (if the cache isn't writeable there's a decent chance the logs aren't writeable either!)
I think we need to get better at having logs for admins for the site maintainers - not adding the messages into the UI. If an end user sees this it's useless to them... even if that comes with us needing to add a way to view site logs in the admin ui
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-25 21:01:43 |
Closed_By | ⇒ | wilsonge |
I didn't mean to merge - was on complete autopilot doh. @PhilETaylor can you do an additional PR catching a potential exception from Log::add please
As logging should be viewed as a nice to have I would say that inability to write a log file should not hinder a user with an exception (unless debug mode is on)
The reason for this PR, in the FTP layer section of fixes, would probably mean that logs could not be written either, and also we do not yet have a configuration so we could not have debug mode on, so we want to gracefully allow the loading of the installation page so that we can finally ask for the credentials for the FTP layer.
If we go about throwing an exception because we can't write the log because we can't write the name space file then we are no further along in our quest to install joomla
Sorry, that was a long way to say yes I will look at it further tomorrow
Following the projects decision to remove the FTP Layer from Joomla 4 - this could possibly be reverted.
Following the projects decision to remove the FTP Layer from Joomla 4 - this could possibly be reverted.
@wilsonge What do you think?
This seems like a fairly safe fallback. In this case we would allow things like the error pages to render I suspect - whereas without this we'll spectacularly crash and burn. So I'd leave this one as it is only because it's so early in the execution stack
This does not work in its current state.
This does not work in its current state.
Care to actually elaborate? or just trolling?
Try on a real hosting environment. And stop with the personal attacks! I don't troll, ever.
You are a troll. Period.
I have tested this PR (Actually the Nightly Zip from https://developer.joomla.org/nightly-builds.html which has this PR already merged into it on the two largest hosting platforms, Plesk and cPanel and it works perfectly with the /administrator/cache folder set to 000
And to prove I actually did this - here is the live site, on cPanel hosting, http://sharkykz.phil-taylor.com/ running with /administrator/cache with 000 permissions and unwritable.
So you state:
This does not work in its current state.
And I ask you again:
Care to actually elaborate? or just trolling?
Would you care to actually tell me why you believe this PR doesn't work in hosting environments, when all the testing proves it does?
WHAT EXACTLY are you trying to report is wrong with it? TELL ME what is actually wrong with it instead of just trolling and saying "does not work" - that is not acceptable feedback. If you can explain what your TESTING has shown, then I will FIX IT.
Simply stating it "does not work" is trolling.
Going by code review, my guess this is the culprit https://github.com/joomla/joomla-cms/pull/33263/files#diff-34e6fec5937ad1928863d81b93501ed9d0e5ddb3a607da8bc0d91f3611ca18d7R159. Why strip out random characters? Since when are these not valid in paths?
Why strip out random characters? Since when are these not valid in paths?
$path
incoming to my changed code code is a string of
"JPATH_ADMINISTRATOR . '/components/com_actionlogs/src'"
We were then replacing the constant with the correct folder structure:
This would give a string of something like
"/application/public_html/administrator . '/com_actionlogs/src'"
The reason for the str_replace
to remove spaces, the concated period, and a single quote at the end of the string to give
/application/public_html/administrator/com_actionlogs/src
This obviously works, and so simply saying "This does not work in its current state" is simply untrue and unhelpful.
Since when are these not valid in paths?
Of course [spaces, periods, single quotes]
are valid (albeit stupid) in folder structures.
My mistake was to remove the [spaces, periods, single quotes]
from the WHOLE absolute path, and not just the part that was not the constant.
Try on a real hosting environment
I did. Unlike you who by your own admission just did a "code review" and provided no helpful feedback until repeatedly requested from you.
I have access to the absolute paths of 67,348 Joomla sites, from Joomla 1.5.0 to Joomla 4.0-beta7
I have checked the absolute paths and found only two sites that has either a period or a space in their JPATH_BASE.
One site is in a folder called /home/username/restored from Dec.2014/
One site is in a folder called /home/username/My Documents/Sites/
Therefore, your assumption that "This does not work in its current state" currently effects 2 out of a random sample of 67,348 sites. 0.0000297%.
Even though those sites can actually write to the /administrator/cache
and therefore would not actually use the cached namespacemap!!!
However, of course, you are right, 0.0000297% right, and as promosed this has been fixed and improved upon and, in absense of a PR from you, I have proposed PR #33504
That PR was tested with a JPATH_BASE of /application/My Folder.Old/
and chmod 000 /administrator/cache
and it works perfectly by preg_replace
to replace the constant, the contcatenation, and the trailing single quote in one go.
Its not pretty or performant, but I fully expect this code never to be executed in the real world, it was meant to allow us to get to the first page of the Installation process on a server that required the FTP Layer to write tot he database (as, at the first page load we yet dont have FTP Credentials) - since then the Joomla PLT have decided in favour of removing the FTP Layer. I proposed the immediate reverting of this PR, and George explained he was happy to keep it merged. Then you showed up days later.
Thank you for (eventually) verbalising your objection instead of just dismissing others hard work with thumbs down and saying "it doesnt work" - that is unhelpful. As you can see, if you take the time to explain your thinking then I, and others, can improve the code we have proposed.
I have tested this item✅ successfully on 417d0f8
Test OK
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33263.