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

Jump to bottom

archive/tar: doesn't generate atime entries #17876

Closed
cyphar opened this issue Nov 10, 2016 · 4 comments
Closed

archive/tar: doesn't generate atime entries #17876

cyphar opened this issue Nov 10, 2016 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.10

Comments

@cyphar
Copy link

cyphar commented Nov 10, 2016 •
edited 

% go version
go version go1.7 linux/amd64
% go env
GOARCH="amd64"
GOBIN="/home/cyphar/go/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/cyphar/.local"
GORACE=""
GOROOT="/usr/lib64/go"
GOTOOLDIR="/usr/lib64/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build013320516=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

This also happens on https://play.golang.org/, which you can see in this snippet: https://play.golang.org/p/QURChe1JI_.

package main

import (
        "archive/tar"
        "fmt"
        "io"
        "time"
)

func main() {
        reader, writer := io.Pipe()

        atime := time.Unix(123, 0)
        mtime := time.Unix(777, 0)
        ctime := time.Unix(1337, 0)

        tw := tar.NewWriter(writer)
        tr := tar.NewReader(reader)

        go func() {
                tw.WriteHeader(&tar.Header{
                        Name:       "somefile",
                        Mode:       0777,
                        ModTime:    mtime,
                        AccessTime: atime,
                        ChangeTime: ctime,
                })
                tw.Close()
        }()

        hdr, err := tr.Next()
        if err != nil {
                panic(err)
        }

        if !hdr.ModTime.Equal(mtime) {
                fmt.Printf("mtime not equal! got %s, expected %s\n", hdr.ModTime, mtime)
        }
        if !hdr.AccessTime.Equal(atime) {
                fmt.Printf("atime not equal! got %s, expected %s\n", hdr.AccessTime, atime)
        }
        if !hdr.ChangeTime.Equal(ctime) {
                fmt.Printf("ctime not equal! got %s, expected %s\n", hdr.ChangeTime, ctime)
        }
}

What did you expect to see?

No errors in the above program.

What did you see instead?

% go run broken.go
atime not equal! got 0001-01-01 00:00:00 +0000 UTC, expected 1970-01-01 10:02:03 +1000 AEST
ctime not equal! got 0001-01-01 00:00:00 +0000 UTC, expected 1970-01-01 10:22:17 +1000 AEST

From what I can tell, this is caused by the fact that WriteHeader doesn't in any way touch the AccessTime of the structure. The same goes for the ChangeTime.
@cyphar cyphar mentioned this issue Nov 10, 2016
Closed
@cyphar
Copy link
Author

cyphar commented Nov 10, 2016

I played with this a bit, and it looks like the issue is that currently we're generating USTAR headers, but only GNU and STAR have support for those headers. This was my attempt, but it obviously doesn't work:

diff --git a/src/archive/tar/writer.go b/src/archive/tar/writer.go
index 596fb8b9e171..0d866d3fbf4a 100644
--- a/src/archive/tar/writer.go
+++ b/src/archive/tar/writer.go
@@ -145,11 +145,17 @@ func (tw *Writer) writeHeader(hdr *Header, allowPax bool) error {
                f.formatNumeric(b, x)
        }

-       // Handle out of range ModTime carefully.
-       var modTime int64
+       // Handle out of range ModTime, AccessTime, and ChangeTime carefully.
+       var modTime, accessTime, changeTime int64
        if !hdr.ModTime.Before(minTime) && !hdr.ModTime.After(maxTime) {
                modTime = hdr.ModTime.Unix()
        }
+       if !hdr.AccessTime.Before(minTime) && !hdr.AccessTime.After(maxTime) {
+               accessTime = hdr.AccessTime.Unix()
+       }
+       if !hdr.ChangeTime.Before(minTime) && !hdr.ChangeTime.After(maxTime) {
+               changeTime = hdr.ChangeTime.Unix()
+       }

        v7 := header.V7()
        formatString(v7.Name(), hdr.Name, paxPath)
@@ -169,6 +175,9 @@ func (tw *Writer) writeHeader(hdr *Header, allowPax bool) error {
        formatString(ustar.GroupName(), hdr.Gname, paxGname)
        formatNumeric(ustar.DevMajor(), hdr.Devmajor, paxNone)
        formatNumeric(ustar.DevMinor(), hdr.Devminor, paxNone)
+       // TODO: This doesn't work because we output USTAR rather than STAR or GNU.
+       //formatNumeric(ustar.AccessTime(), accessTime, paxAtime)
+       //formatNumeric(utar.ChangeTime(), changeTime, paxCtime)

        // TODO(dsnet): The logic surrounding the prefix field is broken when trying
        // to encode the header as GNU format. The challenge with the current logic

@dsnet dsnet self-assigned this Nov 10, 2016
@dsnet dsnet added this to the Go1.9 milestone Nov 10, 2016
@dsnet
Copy link
Member

dsnet commented Nov 10, 2016

I plan on overhauling the Writer logic in Go 1.9 and I have a fix for this. Thanks for the report.

@dsnet dsnet modified the milestones: Go1.10, Go1.9 May 22, 2017
@gopherbot
Copy link

Change https://golang.org/cl/55570 mentions this issue: archive/tar: add support for atime and ctime to Writer

@gopherbot
Copy link

Change https://golang.org/cl/59230 mentions this issue: archive/tar: require opt-in to PAX or GNU format for time features

gopherbot pushed a commit that referenced this issue Aug 30, 2017
@dsnet
archive/tar: require opt-in to PAX or GNU format for time features
f85dc05 
Nearly every Header obtained from FileInfoHeader via the FS has
timestamps with sub-second resolution and the AccessTime
and ChangeTime fields populated. This forces the PAX format
to almost always be used, which has the following problems:
* PAX is still not as widely supported compared to USTAR
* The PAX headers will occupy at minimum 1KiB for every entry

The old behavior of tar Writer had no support for sub-second resolution
nor any support for AccessTime or ChangeTime, so had neither problem.
Instead the Writer would just truncate sub-second information and
ignore the AccessTime and ChangeTime fields.

In this CL, we preserve the behavior such that the *default* behavior
would output a USTAR header for most cases by truncating sub-second
time measurements and ignoring AccessTime and ChangeTime.
To use either of the features, users will need to explicitly specify
that the format is PAX or GNU.

The exact policy chosen is this:
* USTAR and GNU may still be chosen even if sub-second measurements
are present; they simply truncate the timestamp to the nearest second.
As before, PAX uses sub-second resolutions.
* If the Format is unspecified, then WriteHeader ignores AccessTime
and ChangeTime when using the USTAR format.

This ensures that USTAR may still be chosen for a vast majority of
file entries obtained through FileInfoHeader.

Updates #11171
Updates #17876

Change-Id: Icc5274d4245922924498fd79b8d3ae94d5717271
Reviewed-on: https://go-review.googlesource.com/59230
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Aug 26, 2018
@rsc rsc unassigned dsnet Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
3 participants
@cyphar @dsnet @gopherbot