-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[4.0] Error in Joomla update. Invalid Ajax data. Fix #21160 #21332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this not being removed from core, @nikosdion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you are asking.
Removing the constant completely would be a massive security issue. This script must only be allowed to run under very specific circumstances. Removing this check would allow anyone to run it anytime, causing damage to the site.
Renaming the constant is possible, with the caveat that it's not a rename, it's an addition to Akeeba Restore (restore.php). As you can see, this constant is defined in the Akeeba Restore preamble and it's used by my backup software's integrated restoration to make sure post-restoration finalization won't be executable over the web for the same security reasons. We could add another line
define('_JOOMLA_UPDATE', 1);and change the line in restore_finalisation.php to
defined('_JOOMLA_UPDATE', 1) or die();Renaming it won't happen because it'd break my software.
I think that's the best solution. After all, restore.php is a third party library included with Joomla, not part of Joomla! itself. This is what the erstwhile Production Leadership Team had declared and that's the context of our conversation.
I could go one step further and make a special distribution of Akeeba Restore just for the Joomla! project. Please note that this would be a custom distribution, not code with shared OSM copyright, i.e. it will still not be part of Joomla! itself. The custom distribution could do away with all the features the Joomla! project does not need such as support for JPA and JPS archives and all post-processing engines except Hybrid (which magically figures out when to use direct file writes and FTP).
This latter option is extra work for me since it requires refactoring my code. I don't have time to do this right now but if you'd give me a rough estimate of a 4.0 beta freeze I could try. Since the custom distribution would be backwards compatible in the com_joomlaupdate context, even if I don't make it by the beta freeze we can still replace the regular restore.php with the custom one anytime in the 4.0 release cycle.
In fact, we could first add the extra define and then move to the custom distribution. Makes sense for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whatever works best. I would add the
_JOOMLA_UPDATEjoomla code and deprecate the_AKEEBA_RESTORATIONcheck. I guess we can't remove it in the closer future as it will break the old Akeeba backup installations?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you really don't want that constant in the file the only course of action is the second option, making a special distribution of restore.php for Joomla!. As I explained this will take a while.
In the meantime you can continue using the old constant. I would also recommend adding more comment text at the top of the file to explain when this file is used and what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my own education and only if you have time can you explain why removing the akeeba restoration constant will allow unwelcome access - we still have the JEXEC or die?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is run directly from
restore.phpas a cleanup step. As a result we're not in the context of a Joomla entry file but therestore.phpcontext - hence we haven't actually definedJEXECfor this single file (you can see we define it on the line below)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since other people will be reading this, there is a reason why we run this outside the Joomla! context.
When we are going between Joomla! release families (e.g. 3.8 to 3.9) we can reasonably expect that some files / folders are removed or moved around. However, extracting a ZIP file will only create files, not delete the ones which are removed.
This is not just a nuissance (dead code left behind) but a real functionality issue. Due to the way the class autoloader works, old code files might be prioritized over the new ones. This will create a disparity in what the rest of Joomla! expects to be loaded and what is actually loaded. This would lead to dead sites.
Moreover, sometimes we need to apply some database changes (either schema or data conversions) to ensure that the upgraded site will work correctly.
Finally, on sites which use a code cache (such as OPcache) we need to reset the code cache after Joomla! finishes updating. Otherwise PHP may end up loading a mix of old files from the cache and new files from disk, leading to broken sites. Unfortunately many servers either have a code cache enabled without asking the user or brand it as "site optimization" without sufficient explanation of the technical pitfalls every time you upgrade code on your site. This can be resolved programmatically in most cases.
These actions need to run after Joomla is upgraded but before you access it. Therefore we cannot run in the Joomla! CMS application context, nor can we (usually) do so safely in the context of a custom web application based on the Joomla! framework because of the deleted / moved files issue. The only way to handle it is by doing this outside of Joomla!, selectively loading the bits and pieces which are guaranteed to be working. Hence a separate script. The need to load bits of Joomla! is why we also define _JEXEC in that script; to allow those Joomla bits to load without dying.
The question is, how do you load this script over the web without accessing Joomla! and without posing a security threat (i.e. you can't have a directly web accessible file). The solution is given by Akeeba Restore itself. The last step it does after the extraction is over is calling the restore_finalisation.php script. Since communication with Akeeba Restore is secured and limited to the update process it does not pose a security risk.
The way the finalisation script is protected against direct access is similar to what Joomla! does: by checking whether a constant is defined. This thread is about renaming that constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you