Skip to content
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

[Fix] Zip spec compliance for exported backup file #2157

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

brandonpayton
Copy link
Member

Motivation for the change, related issues

Files in a zip file should only have relative urls as per spec. Right now, when exporting the backup for the site, we create a zip file with a leading slash in the files inside it.

This got surfaced in Studio app when we switched to a diff zip library that strictly adheres to zip spec and PG exported zip files fail to process.

Related to: https://github.com/Automattic/dotcom-forge/issues/10369

Note: There are other places where ZipArchive is used but this PR only modifies the location that impacts the site export. Screenshot 2025-01-30 at 11 44 32
Probably other places can use the same change, and I can follow that up in a separate PR, if you like.

Testing Instructions (or ideally a Blueprint)

Export zip of a site and examine the file paths inside it using the following script:

<?php
$zip = new ZipArchive();
$zip->open('/Users/ashfame/Downloads/wordpress-playground-16.zip');
echo "=== ZIP Contents ===".PHP_EOL;
for($i = 0; $i < $zip->numFiles; $i++) {
	echo $zip->getNameIndex($i) . PHP_EOL;
}
echo "=== End ZIP Contents ===".PHP_EOL;
$zip->close();

Prior to this PR, you would see file paths like:

/wp-content/themes/twentytwentyfive/patterns/more-posts.php

and with this PR you will see file paths without the leading slash:

wp-content/themes/twentytwentyfive/patterns/more-posts.php

## Motivation for the change, related issues
Files in a zip file should only have relative urls as per spec. Right
now, when exporting the backup for the site, we create a zip file with a
leading slash in the files inside it.

This got surfaced in Studio app when we switched to a diff zip library
that strictly adheres to zip spec and PG exported zip files fail to
process.

Related to: Automattic/dotcom-forge#10369

Note: There are other places where `ZipArchive` is used but this PR only
modifies the location that impacts the site export.
<img width="531" alt="Screenshot 2025-01-30 at 11 44 32"
src="https://github.com/user-attachments/assets/561896ee-0fe9-40c1-b413-d226c68fc0be"
/>
Probably other places can use the same change, and I can follow that up
in a separate PR, if you like.

## Testing Instructions (or ideally a Blueprint)
Export zip of a site and examine the file paths inside it using the
following script:

```php
<?php
$zip = new ZipArchive();
$zip->open('/Users/ashfame/Downloads/wordpress-playground-16.zip');
echo "=== ZIP Contents ===".PHP_EOL;
for($i = 0; $i < $zip->numFiles; $i++) {
	echo $zip->getNameIndex($i) . PHP_EOL;
}
echo "=== End ZIP Contents ===".PHP_EOL;
$zip->close();
```

Prior to this PR, you would see file paths like:
```
/wp-content/themes/twentytwentyfive/patterns/more-posts.php
```
and with this PR you will see file paths without the leading slash:
```
wp-content/themes/twentytwentyfive/patterns/more-posts.php
```
@brandonpayton brandonpayton added [Type] Bug An existing feature does not function as intended [Feature] Import Export labels Feb 25, 2025
@brandonpayton brandonpayton requested a review from a team February 25, 2025 20:36
@brandonpayton brandonpayton changed the title [Fix] Zip spec compliance for exported backup file (#7) [Fix] Zip spec compliance for exported backup file Feb 25, 2025
@brandonpayton brandonpayton merged commit f41e541 into trunk Feb 26, 2025
10 checks passed
@brandonpayton brandonpayton deleted the make-exports-comply-with-zip-spec branch February 26, 2025 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Import Export [Type] Bug An existing feature does not function as intended
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants