Skip to content

Fix #50678 files extracted by ZipArchive class lost their original mo… #5244

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

Closed
wants to merge 5 commits into from

Conversation

remicollet
Copy link
Member

…dified time

@remicollet
Copy link
Member Author

@cmb69 as this is another implementation of pr #1291

nikic and others added 3 commits March 6, 2020 15:29
This has been fixed in PHP 7.4, add a test for it.
Marco broke things again.
@remicollet
Copy link
Member Author

There is small TZ issue, which looks like (to me) as an issue in libzip.

@cmb69
Copy link
Member

cmb69 commented Mar 6, 2020

There is small TZ issue, which looks like (to me) as an issue in libzip.

Yeah, I faced that issue with the othe PR as well; it worked locally, but failed on CI. The problem might be that at least the MS DOS time format does not even specify a timezone.

Anyhow, to work around that in the test case by just ignoring the hours/minutes/seconds might still be brittle; maybe it would be better to not print the timestamp directly, but rather to check whether it is within a permissible range?

@remicollet
Copy link
Member Author

remicollet commented Mar 6, 2020

Ok, found the explanation, "local" time is stored (which may be different from file system time)

BTW, everything works as expected:

  • zip / unzip => same time
  • zip / ZipArchive::extractTo => same time
  • ZipArchvie::addFile / unzip => same time
  • ZipArchvie::addFile / ZipArchive::extractTo => same time

And finally found a better test (compare date of saved / restored file, which is exactly the goal)

This reverts commit fb1b70a.
@remicollet
Copy link
Member Author

Grr... "better" test fails... do definitively some issue in testing env, so reverting to the previous partial test (checking only date, not time)

@remicollet
Copy link
Member Author

About test; "zip" format only allow even second, which may explain some failure

See https://libzip.org/documentation/zip_file_set_dostime.html

@remicollet
Copy link
Member Author

Merged

@remicollet remicollet closed this Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants