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: Reader doesn't decode binary header fields #4358

Closed
mxk opened this issue Nov 7, 2012 · 6 comments
Closed

archive/tar: Reader doesn't decode binary header fields #4358

mxk opened this issue Nov 7, 2012 · 6 comments
Milestone

Comments

@mxk
Copy link

mxk commented Nov 7, 2012

Discussion: https://groups.google.com/d/topic/golang-nuts/9nEtC8moVDw/discussion

Numeric header fields written by tar.Writer.numeric use the binary encoding when they
cannot be represented in the octal form. tar.Reader seems to only understand octal
fields. When Header.ModTime is left as the zero value (as in the package examples), it
is encoded using the binary format. As a result, Reader returns ErrHeader when
attempting to decode the archive.

What steps will reproduce the problem?
1. http://play.golang.org/p/npEgUjWE8I

Uncomment ModTime field to fix the panic.

What is the expected output?
hello, world

What do you see instead?
panic: archive/tar: invalid tar header

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Windows

Which version are you using?  (run 'go version')
go1.0.3
@dsymonds
Copy link
Contributor

dsymonds commented Nov 7, 2012

Comment 1:

Labels changed: added priority-soon, packagebug, removed priority-triage.

Owner changed to @dsymonds.

Status changed to Accepted.

@dsymonds
Copy link
Contributor

dsymonds commented Nov 7, 2012

Comment 2:

The problem is that a zero ModTime gives a large negative t.Unix() value, which confuses
everything. I'll avoid writing a mod time outside the specified range.

Status changed to Started.

@mxk
Copy link
Author

mxk commented Nov 7, 2012

Comment 3:

I think the same problem will affect other numeric fields, such as Size. Wouldn't it be
better to modify Reader.octal, or add a new Reader.numeric function, to decode the
binary value if the most significant bit is set?
If you do add extra checks to the Writer, I suggest you also return ErrFieldTooLong when
len(Header.Name) > 100. Until support is added for longer file names, it's better to
report an error instead of silently truncating the value.

@dsymonds
Copy link
Contributor

dsymonds commented Nov 7, 2012

Comment 4:

The other fields, I believe, have plenty of space for the range of values permitted by
their fields. Bigger numbers already switch to the binary format if required. ModTime is
the problematic one because its use of .Unix() yields a negative number that is not even
supported by most tars.
However, you are right that Reader does not support the binary format at all. That would
be the other half of this issue.

@dsymonds
Copy link
Contributor

dsymonds commented Nov 7, 2012

Comment 5:

This issue was updated by revision 0ac3178.

Still to do: support binary numeric format in Reader.
R=golang-dev, rsc
CC=golang-dev
http://golang.org/cl/6818101

@dsymonds
Copy link
Contributor

dsymonds commented Nov 8, 2012

Comment 6:

This issue was closed by revision 86b9e3e.

Status changed to Fixed.

@mxk mxk added fixed labels Nov 8, 2012
@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants