-
Notifications
You must be signed in to change notification settings - Fork 798
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
Backup: Write helper script to ABSPATH by default, just like we did before #35508
Conversation
Writing to "wp-content/" has caused some issues, as some hosts don't allow PHP code from "wp-content/" to be accessed directly.
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped. Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Backup plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Migration plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
This reverts commit abd558f.
…ckage_Version proxy
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.
Thanks for working on this. I just tested it using the core plugin and the standalone backup plugin, and it is working as expected.
Thank you Rafa! |
I was taking a look into some manual upgrade tests using Beta Testers, and can confirm that upgrading from Jetpack 13.1.1 and Jetpack VaultPress Backup 2.4 to this branch is working as expected and no more PHP errors are thrown. I think we are good to go ✅ |
…efore (#35508) * Write helper script to ABSPATH, just like we did before Writing to "wp-content/" has caused some issues, as some hosts don't allow PHP code from "wp-content/" to be accessed directly. * Bump class namespace version #34739 * changelog * Make the test expect the right order of install locations * Fix up project versions * Add proxy `Package_Version` classes with different namespaces * Revert "Add proxy `Package_Version` classes with different namespaces" This reverts commit abd558f. * Don't use namespace versioning in Package_Version, add <...>\V0001\Package_Version proxy * Use correct path to Package_Version in the test * Fix package name in a test * Move test to Automattic\Jetpack\Backup namespace * Move Package_Version's compatibility class to separate file * Add (possibly) missing import * Apply the same behavior to Automattic\Jetpack\Transport_Helper\Package_Version too * Fix up project/package versions again * Fix transport-helper's actions.php to point to the right Package_Version
…efore (#35508) * Write helper script to ABSPATH, just like we did before Writing to "wp-content/" has caused some issues, as some hosts don't allow PHP code from "wp-content/" to be accessed directly. * Bump class namespace version #34739 * changelog * Make the test expect the right order of install locations * Fix up project versions * Add proxy `Package_Version` classes with different namespaces * Revert "Add proxy `Package_Version` classes with different namespaces" This reverts commit abd558f. * Don't use namespace versioning in Package_Version, add <...>\V0001\Package_Version proxy * Use correct path to Package_Version in the test * Fix package name in a test * Move test to Automattic\Jetpack\Backup namespace * Move Package_Version's compatibility class to separate file * Add (possibly) missing import * Apply the same behavior to Automattic\Jetpack\Transport_Helper\Package_Version too * Fix up project/package versions again * Fix transport-helper's actions.php to point to the right Package_Version
…efore (#35508) * Write helper script to ABSPATH, just like we did before Writing to "wp-content/" has caused some issues, as some hosts don't allow PHP code from "wp-content/" to be accessed directly. * Bump class namespace version #34739 * changelog * Make the test expect the right order of install locations * Fix up project versions * Add proxy `Package_Version` classes with different namespaces * Revert "Add proxy `Package_Version` classes with different namespaces" This reverts commit abd558f. * Don't use namespace versioning in Package_Version, add <...>\V0001\Package_Version proxy * Use correct path to Package_Version in the test * Fix package name in a test * Move test to Automattic\Jetpack\Backup namespace * Move Package_Version's compatibility class to separate file * Add (possibly) missing import * Apply the same behavior to Automattic\Jetpack\Transport_Helper\Package_Version too * Fix up project/package versions again * Fix transport-helper's actions.php to point to the right Package_Version
cherry-picked to Four-point package releases: |
In #34297, among other things, we've changed the order of directories to which we try to write the helper script.
Before that PR, the order was:
wp-content/
wp-content/uploads/
After that PR, the new order was:
wp-content/
wp-content/uploads/
This is because on some hosts, ABSPATH might point to weird locations, and on others, writing to ABSPATH won't be allowed due to security reasons.
It turned out that quite a few hosts disallow us from accessing PHP scripts from
wp-content/
directly:So, we're changing back the default to ABSPATH, i.e. the order is now going to be back to:
wp-content/
wp-content/uploads/
Also, I've moved
Package_Version
class from a versionedVXXXX
namespace because it's used in a filter, and due to the way it works, it would cause problems on plugin upgrades:Proposed changes:
wp-content/
, then towp-content/uploads/
.Package_Version
class back to theAutomattic\Jetpack\Backup
namespaceOther information:
Jetpack product discussion
p1707297095152959-slack-CS8UYNPEE
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
Just like with #34297, run a Jetpack VaultPress Backup backup without credentials set up, so that the tool is then forced to upload the helper script via Jetpack API and thus trigger this code.
Please test upgrades as well, we had some hiccups with the Backup plugin getting updated with the versioning: #34739 (comment)