WordPress.org

Make WordPress Core

Opened 3 weeks ago

Closed 13 days ago

Last modified 5 days ago

#43908 closed defect (bug) (fixed)

Export keeps generating new .zip files on Windows installations

Reported by: xkon Owned by: sergeybiryukov
Milestone: 4.9.6 Priority: normal
Severity: normal Version:
Component: Privacy Keywords: gdpr has-patch
Focuses: Cc:

Description

In file.php line 2146 we have update_post_meta( $request_id, '_export_file_path', $archive_pathname ); to update the filepath of the exported zip file.

In windows the file path uses \ instead of / and they get stripped out when the update_post_meta happens.

This results in an error when downloading / mailing the file so it always generates a new .zip file.

I've made a check by doing an $archive_pathname = str_replace( '\\', '/', $archive_pathname ); just before the update_post_meta and it works but not sure if that's the way to go.

( not sure if it's my installation only either but I've never seen it happen again and I was always working on Windows maybe it's an update_post_meta thing? )

Attachments (5)

xk20180430200820.jpg (68.5 KB) - added by xkon 3 weeks ago.
43908.diff (529 bytes) - added by xkon 3 weeks ago.
43908.2.diff (1.0 KB) - added by xkon 3 weeks ago.
43908.3.diff (1.2 KB) - added by pmbaldha 2 weeks ago.
Refreshed patch
test.png (32.6 KB) - added by pmbaldha 2 weeks ago.
Test result

Download all attachments as: .zip

Change History (24)

@xkon
3 weeks ago

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


3 weeks ago

#2 @desrosj
3 weeks ago

  • Milestone changed from Awaiting Review to 4.9.6

@xkon
3 weeks ago

#3 @xkon
3 weeks ago

  • Keywords has-patch added

43908.diff uses wp_normalize_path to fix \ into / that are causing the issues under windows.

Please do test, but my tests where looking good and the issues stopped.

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


3 weeks ago

#5 @allendav
3 weeks ago

This works well for the archive (ZIP) on my Mac, but I wonder if you ( @xkon ) are also having (silent) trouble on Windows with the temporary HTML report path (we write the HTML report then use it to addFile to the ZIP and then we delete the temporary HTML report)

I recommend you also dump $html_report_pathname near line 2046 in file.php and see if the path for that file is also malformed. If so, you might want to normalize it too before using it.

@xkon
3 weeks ago

#6 @xkon
3 weeks ago

@allendav you were correct. Although the file creation was working since the \ where not getting removed still there was a mix of \ + / so I added another normalize as you said in 43908.2.diff .

Works properly again without any issues.

This ticket was mentioned in Slack in #gdpr-compliance by xkon. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #core by desrosj. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #gdpr-compliance by allendav. View the logs.


2 weeks ago

@pmbaldha
2 weeks ago

Refreshed patch

#10 @pmbaldha
2 weeks ago

@desrosj The last patch was need to be refreshed. I have refreshed it.

@pmbaldha
2 weeks ago

Test result

#11 @pmbaldha
2 weeks ago

I have tested the 43908.3.diff patch and it is working fine.

This ticket was mentioned in Slack in #gdpr-compliance by sergey. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #core by desrosj. View the logs.


13 days ago

#14 @desrosj
13 days ago

  • Owner set to sergeybiryukov
  • Status changed from new to reviewing

#15 @SergeyBiryukov
13 days ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 43234:

Privacy: Normalize file paths in wp_privacy_generate_personal_data_export_file() to make sure Windows paths don't have their backslashes stripped.

Props xkon, pmbaldha.
Fixes #43908.

#16 @SergeyBiryukov
13 days ago

In 43235:

Privacy: Normalize file paths in wp_privacy_generate_personal_data_export_file() to make sure Windows paths don't have their backslashes stripped.

Props xkon, pmbaldha.
Merges [43234] to the 4.9 branch.
Fixes #43908.

#17 @SergeyBiryukov
13 days ago

Follow-up: #44038

Last edited 13 days ago by SergeyBiryukov (previous) (diff)

#18 @desrosj
7 days ago

  • Component changed from General to Privacy

Moving to the new Privacy component.

This ticket was mentioned in Slack in #meta by pmbaldha. View the logs.


5 days ago

Note: See TracTickets for help on using tickets.