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: Does not handle extended pax values with nulls #32264
Comments
FYI I am happy to contribute a fix for this, just need guidance on what is the appropriate fix. |
The specification for PAX regarding
This suggest that NUL is not a valid character to appear the extended record for |
This seems reasonable, but it requires a scrub through of the entire implementation to identify fatal vs. non-fatal errors. It also needs an API design to surface non-fatal errors. One problem is that the implementation has always returned |
I read over that bit of the spec a few times. I believe it is saying use an extended header record if However, the spec also states UTF-8. But the
I agree, I think that rules out this possibility without further investigation. Given what gnutar and bsdtar do, I would be tempted to just remove the contains nul validation. If that was done, the effect on existing use cases:
|
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What did you do?
Read a tar file generated by
git archive
:Alternatively you can fetch this archive directly from github: https://github.com/SSW-SCIENTIFIC/NNDD/tarball/c21b98da2ca7f007230e696b2eda5da6589fe137
If you try to read this with golang's
archive/tar
you will get atar.ErrHeader
on one of the entries. Here is a reproduction program:If you try read the same file with bsdtar or gnutar they both read the whole tarball. bsdtar (depending on version) will log a warning to stderr but still extract the rest of the tarball.
The archive contains a symbolic link which instead of a path for value, contains a large value which includes null. The
validatePAXRecord
explicitly disallows null from appearing in the value for PAX values for links. This leads totar.Reader.Next()
returningtar.ErrHeader
.https://github.com/golang/go/blob/go1.12.5/src/archive/tar/strconv.go#L321-L322
I reported this to the git mailing list: https://public-inbox.org/git/CAMVcy0Q0TL6uEGR2NeudJrOiXdQ87XcducL0EwMidWucjk5XYw@mail.gmail.com/T/#u Read that issue for more details on the "bad" entry in the archive.
The opinion there seems to be that generating values that aren't C-strings for links is valid. Not all tarballs are destined for filesystems. And the latest bsdtar and gnutar do not complain about the value. gnutar will write the symbolic link value upto the first null.
What did you expect to see?
I think any of the follow behaviours would be better:
What did you see instead?
tar.ErrHeader
on a tarball both bsdtar and gnutar can read.The text was updated successfully, but these errors were encountered: