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/tar: incompatibility in tar files generated between Go 1.7 and Go 1.8 #21005

Closed
stevvooe opened this issue Jul 14, 2017 · 7 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@stevvooe
Copy link

Tar files generated with Go 1.7 and before can no longer be read with Go 1.8 when the path name is greater than 100 characters and has two or more components and the uid is greater than octal value 010000000.

The problem was first reported in moby/moby#34092, which details some conditions and impact. With docker, this manifests itself in cases where a file with a long path name will be written out merely as the base name.

The cause seems to be around the handling of the GNU prefix field in tar headers. There were a few fixes in Go 1.8 that seem to have led to this situation in pursuit of fixing this. Unfortunately, these tars are no longer readable with Go 1.8+ but are out in the wild, as container images. Note that GNU tar also cannot read these files, which was probably the reasoning around the Go 1.8 fixes.

A full description and minimal reproduction, without the docker source, is available in https://gist.github.com/stevvooe/e2a790ad4e97425896206c0816e1a882. This provides an overview of the behavior and condition under which this is encountered. Let me know if you need me to copy this into the issue description.

Our current solution, as proposed in moby/moby#34092 (comment), is to fork archive/tar to handle this specific case, which seems unfortunate. It would be great to get some guidance from the Go team and community on how to proceed.

cc @dsnet @tonistiigi @dmcgowan

@bradfitz
Copy link
Contributor

You deleted the bug template. What version are you using?

Also, you say "Go 1.8+", but did you try Go 1.9beta2?

@bradfitz bradfitz added this to the Go1.10 milestone Jul 14, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 14, 2017
@stevvooe
Copy link
Author

You deleted the bug template. What version are you using?

Sorry, details about version are in the gist.

I'll give 1.9beta2 a try.

@stevvooe
Copy link
Author

Looks like go1.9beta2 behaves the same with file generated by go1.7.6:

$ go1.9beta2 run in.go < bad.tar
2017/07/13 19:11:36 reading tar go1.9beta2
2017/07/13 19:11:36 GOROOT /home/sjd/sdk/go1.9beta2
&tar.Header{Name:"foo", Mode:0, Uid:2097152, Gid:0, Size:0, ModTime:time.Time{wall:0x0, ext:62135596800, loc:(*time.Location)(0x558e40)}, Typeflag:0x0, Linkname:"", Uname:"", Gname:"", Devmajor:0, Devminor:0, AccessTime:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}, ChangeTime:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}, Xattrs:map[string]string(nil)}

Note that the Name is only the basename and not the full path.

A round trip with go1.9beta2 behaves the same as go1.8.3:

$ go1.9beta2 run out.go > go1.9.tar
2017/07/13 19:14:03 writing tar go1.9beta2
2017/07/13 19:14:03 GOROOT /home/sjd/sdk/go1.9beta2
2017/07/13 19:14:03 writing entry path of length 101 with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/foo
$ go1.9beta2 run in.go < go1.9.tar
2017/07/13 19:14:13 reading tar go1.9beta2
2017/07/13 19:14:13 GOROOT /home/sjd/sdk/go1.9beta2
&tar.Header{Name:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/foo", Mode:0, Uid:2097152, Gid:0, Size:0, ModTime:time.Time{wall:0x0, ext:62135596800, loc:(*time.Location)(0x558e40)}, Typeflag:0x0, Linkname:"", Uname:"", Gname:"", Devmajor:0, Devminor:0, AccessTime:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}, ChangeTime:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}, Xattrs:map[string]string(nil)}

@stevvooe
Copy link
Author

stevvooe commented Jul 14, 2017

The proposed patch for docker has been submitted in moby/moby#34106. This vendors a fork of archive/tar rolling back https://go-review.googlesource.com/c/31444/. The diff is release-branch.go1.8...tonistiigi:revert-prefix-ignore.

@dsnet
Copy link
Member

dsnet commented Aug 7, 2017

This bug from Go1.7 and below is really unfortunate.

When I submitted https://golang.org/cl/31444, I tried to add logic to at least have the Reader be able to read these buggy files, even if the Name was wrong. We could have it fallback to parsing those bytes as the prefix, but the problem is that there is no reliable way to distinguish between the following:

  • Whether the tar file was generated by an old and buggy version of Go
  • Whether the tar file is a valid GNU file with a valid a AccessTime and ChangeTime

There are situations where the file generated by the buggy Go version also happens to be a valid GNU file. The situation where this classification fails would be the case where the prefix field was consisting of entirely octal numbers (e.g., "13040433502\x00") that happen to make the file look like a properly formatting GNU file.

However, this is an exceedingly rare situation. I'm willing to add logic to handle to fallback to parsing the prefix field, but users must understand that this only extends the range of how many buggy files generated by Go we can read properly. The correct behavior must take precedence over the buggy behavior.

@stevvooe
Copy link
Author

stevvooe commented Aug 8, 2017

@dsnet Thanks for looking into this!

Agreed that this is extremely unfortunate.

In general, we'll be in favor of any solution that ensures we no longer have to carry forks of the archive/tar package. The number of layers affected is minimal, so I think such a fallback would be appropriate.

cc @dmcgowan @tonistiigi

@gopherbot
Copy link

Change https://golang.org/cl/53635 mentions this issue: archive/tar: Fallback to pre-Go1.8 behavior on certain GNU files

@dsnet dsnet added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 8, 2017
This was referenced Sep 21, 2017
@golang golang locked and limited conversation to collaborators Aug 11, 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.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants