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

archive/zip: discrepancy between os.Fileinfo.Modtime and zip.FileHeader.Modified in Go 1.13 #33494

Closed
eclipseo opened this issue Aug 6, 2019 · 7 comments

Comments

@eclipseo
Copy link

@eclipseo eclipseo commented Aug 6, 2019

In the package github.com/rakyll/statik, there is a test function that compares mtime obtained through os.Fileinfo.Modtime and zip.FileHeader.Modified:

			for name, wantFile := range tc.wantFiles {
				f, err := fs.Open(name)

				stat, err := f.Stat()
 
				if got, want := stat.ModTime(), wantFile.modTime; got != want {
					t.Errorf("ModTime(%v) = %v; want %v", name, got, want)
				}

where wantFile.modTime is

tests := []struct {
		description string
		zipData     string
		wantFiles   map[string]wantFile
	}{
		{
			description: "Files should retain their original file mode and modified time",
			zipData:     mustZipTree("../testdata/file"),
			wantFiles: map[string]wantFile{
				"/file.txt": {
					data:    mustReadFile("../testdata/file/file.txt"),
					isDir:   false,
					modTime: fileTxtHeader.ModTime(),
					mode:    fileTxtHeader.Mode(),
					name:    fileTxtHeader.Name,
					size:    int64(fileTxtHeader.UncompressedSize64),
				},
			},
		},

where fileTxtHeader.ModTime() is zip.FileInfoHeader.ModTime()

In 1.13, zip.FileInfoHeader.ModTime() is now equivalent to zip.FileInfoHeader.Modified.UTC() with this commit 20995f6

Since this commit, the comparison is not working anymore, there is a 1 second discrepency between the two:

Testing    in: /builddir/build/BUILD/statik-0.1.6/_build/src
         PATH: /builddir/build/BUILD/statik-0.1.6/_build/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/sbin
       GOPATH: /builddir/build/BUILD/statik-0.1.6/_build:/usr/share/gocode
  GO111MODULE: off
      command: go test -buildmode pie -compiler gc -ldflags "-X github.com/rakyll/statik/version=0.1.6 -extldflags '-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld '"
      testing: github.com/rakyll/statik
github.com/rakyll/statik/fs
--- FAIL: TestOpen (0.00s)
    --- FAIL: TestOpen/Files_should_retain_their_original_file_mode_and_modified_time (0.00s)
        fs_test.go:195: ModTime(/file.txt) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC
    --- FAIL: TestOpen/Images_should_successfully_unpack (0.00s)
        fs_test.go:195: ModTime(/pixel.gif) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC
    --- FAIL: TestOpen/'index.html'_files_should_be_returned_at_their_original_path_and_their_directory_path (0.00s)
        fs_test.go:195: ModTime(/index.html) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC
        fs_test.go:195: ModTime(/sub_dir/index.html) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC
    --- FAIL: TestOpen/listed_all_sub_directories_in_deep_directory (0.00s)
        fs_test.go:195: ModTime(/a) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC
        fs_test.go:195: ModTime(/aa/bb/c) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC

If I get the mtime for these files from my Linux system, it seems that zip.FileInfoHeader.Modified.UTC() give the exact result but os.Fileinfo.Modtime add 1 second to the UTC time. It seems that before 1.13, zip.FileInfoHeader.ModTime() was also delaying by one second so the problem was not detected.

Has anyone any idea from where that discrepency come from?

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Aug 6, 2019

/cc @Xjs @mvdan

@dmitshur dmitshur changed the title Go 1.13: Discrepency between os.Fileinfo.Modtime and zip.FileHeader.Modified archive/zip: discrepancy between os.Fileinfo.Modtime and zip.FileHeader.Modified in Go 1.13 Aug 6, 2019
@dmitshur dmitshur added this to the Go1.13 milestone Aug 6, 2019
@eclipseo
Copy link
Author

@eclipseo eclipseo commented Aug 6, 2019

Actually it's the opposite, the UNIX timestamp of the file is 1553108171. So os.Fileinfo.Modtime give the correct 2019-03-20 18:56:11 +0000 UTC time but zip.FileInfoHeader.ModTime() gives 2019-03-20 18:56:10 +0000 UTC time, so one second less.

@eclipseo
Copy link
Author

@eclipseo eclipseo commented Aug 6, 2019

That's curious: if the UNIX timestamp is even, os.Fileinfo.Modtime and zip.FileInfoHeader.ModTime() returns the same and correct time, if the UNIX timestamp is odd, zip.FileInfoHeader.ModTime() returns a time with one second less.

@eclipseo
Copy link
Author

@eclipseo eclipseo commented Aug 6, 2019

Repro:

package main

import (
	"archive/zip"
	"fmt"
	"os"
	"time"
)

func main() {
	name := "test.txt"
	var got, want time.Time
	f, _ := os.Open(name)
	info, _ := f.Stat()
	got = info.ModTime().UTC()
	r, _ := zip.FileInfoHeader(info)
	want = r.ModTime()
	fmt.Printf("ModTime(%v) = %s; want %s", name, got, want)
}
touch -t "$(date +'%Y%m%d%H%M.%S'  --date='@1553108171')" test.txt`
go run time.go
ModTime(test.txt) = 2019-03-20 18:56:11 +0000 UTC; want 2019-03-20 18:56:10 +0000 UTC
touch -t "$(date +'%Y%m%d%H%M.%S'  --date='@1553108172')" test.txt`
go run time.go
ModTime(test.txt) = 2019-03-20 18:56:12 +0000 UTC; want 2019-03-20 18:56:12 +0000 UTC
@rsc
Copy link
Contributor

@rsc rsc commented Aug 6, 2019

The native zip file format stores times in MS-DOS FAT encoding, which has only 2-second granularity. That is, it can only store even Unix times. Odd Unix times are truncated, as you have discovered. Note that using the ModTime header as in your repro case is deprecated:

$ go doc zip.FileHeader.ModTime
package zip // import "archive/zip"

func (h *FileHeader) ModTime() time.Time
    ModTime returns the modification time in UTC using the legacy ModifiedDate
    and ModifiedTime fields.

    Deprecated: Use Modified instead.

$ 

I don't believe there is anything to do here.

@dsnet
Copy link
Member

@dsnet dsnet commented Aug 6, 2019

I concur with Russ' assessment.

@eclipseo
Copy link
Author

@eclipseo eclipseo commented Aug 7, 2019

The native zip file format stores times in MS-DOS FAT encoding, which has only 2-second granularity. That is, it can only store even Unix times. Odd Unix times are truncated, as you have discovered. Note that using the ModTime header as in your repro case is deprecated:

$ go doc zip.FileHeader.ModTime
package zip // import "archive/zip"

func (h *FileHeader) ModTime() time.Time
    ModTime returns the modification time in UTC using the legacy ModifiedDate
    and ModifiedTime fields.

    Deprecated: Use Modified instead.

$ 

I don't believe there is anything to do here.

Thanks, I'll patch upstream with Modified, but that implies it won't be compatible with Go 1.9.x anymore.

@eclipseo eclipseo closed this Aug 7, 2019
@golang golang locked and limited conversation to collaborators Aug 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.