delta/go-git.git/src/archive/tar/reader.go, branch dev.inline

archive/tar: validate sparse headers in parsePAX

2016-10-22T16:35:14+00:00

According to the GNU manual, the format is:
<<<
GNU.sparse.size=size
GNU.sparse.numblocks=numblocks
repeat numblocks times
  GNU.sparse.offset=offset
  GNU.sparse.numbytes=numbytes
end repeat
>>>

The logic in parsePAX converts the repeating sequence of
(offset, numbytes) pairs (which is not PAX compliant) into a single
comma-delimited list of numbers (which is now PAX compliant).

Thus, we validate the following:
* The (offset, numbytes) headers must come in the correct order.
* The ',' delimiter cannot appear in the value.
We do not validate that the value is a parsible decimal since that
will be determined later.

Change-Id: I8d6681021734eb997898227ae8603efb1e17c0c8
Reviewed-on: https://go-review.googlesource.com/31439
Run-TryBot: Joe Tsai 
TryBot-Result: Gobot Gobot 
Reviewed-by: Brad Fitzpatrick

archive/tar: fix parsePAX to be POSIX.1-2001 compliant

2016-10-19T18:39:30+00:00

Relevant PAX specification:
<<<
If the  field is zero length, it shall delete any header
block field, previously entered extended header value, or
global extended header value of the same name.
>>>

We don't delete global extender headers since the Reader doesn't
even support global headers (which the specification admits was
a controversial feature).

Change-Id: I2125a5c907b23a3dc439507ca90fa5dc47d474a9
Reviewed-on: https://go-review.googlesource.com/31440
Run-TryBot: Brad Fitzpatrick 
TryBot-Result: Gobot Gobot 
Reviewed-by: Brad Fitzpatrick

archive/tar: make Reader handle GNU format properly

2016-10-19T18:07:55+00:00

The GNU format does not have a prefix field, so we should make
no attempt to read it. It does however have atime and ctime fields.
Since Go previously placed incorrect values here, we liberally
read the atime and ctime fields and ignore errors so that old tar
files written by Go can at least be partially read.

This fixes half of #12594. The Writer is much harder to fix.

Updates #12594

Change-Id: Ia32845e2f262ee53366cf41dfa935f4d770c7a30
Reviewed-on: https://go-review.googlesource.com/31444
Run-TryBot: Joe Tsai 
TryBot-Result: Gobot Gobot 
Reviewed-by: Brad Fitzpatrick

archive/tar: fix and cleanup readOldGNUSparseMap

2016-10-12T16:36:57+00:00

* Assert that the format is GNU.
Both GNU and STAR have some form of sparse file support with
incompatible header structures. Worse yet, both formats use the
'S' type flag to indicate the presence of a sparse file.
As such, we should check the format (based on magic numbers)
and fail early.

* Move realsize parsing logic into readOldGNUSparseMap.
This is related to the sparse parsing logic and belongs here.

* Fix the termination condition for parsing sparse fields.
The termination condition for reading the sparse fields
is to simply check if the first byte of the offset field is NULL.
This does not seem to be documented in the GNU manual, but this is
the check done by the both the GNU and BSD implementations:
http://git.savannah.gnu.org/cgit/tar.git/tree/src/sparse.c?id=9a33077a7b7ad7d32815a21dee54eba63b38a81c#n731
https://github.com/libarchive/libarchive/blob/1fa9c7bf90f0862036a99896b0501c381584451a/libarchive/archive_read_support_format_tar.c#L2207

* Fix the parsing of sparse fields to use parseNumeric.
This is what GNU and BSD do. The previous two links show that
GNU and BSD both handle base-256 and base-8.

* Detect truncated streams.
The call to io.ReadFull does not check if the error is io.EOF.
Getting io.EOF in this method is never okay and should always be
converted to io.ErrUnexpectedEOF.

* Simplify the function.
The logic is essentially a do-while loop so we can remove
some redundant code.

Change-Id: Ib2f601b1a283eaec1e41b1d3396d649c80749c4e
Reviewed-on: https://go-review.googlesource.com/28471
Reviewed-by: Russ Cox
Run-TryBot: Russ Cox

archive/tar: handle integer overflow on 32bit machines

2016-10-12T16:36:41+00:00

Most calls to strconv.ParseInt(x, 10, 0) should really be
calls to strconv.ParseInt(x, 10, 64) in order to ensure that they
do not overflow on 32b architectures.

Furthermore, we should document a bug where Uid and Gid may
overflow on 32b machines since the type is declared as int.

Change-Id: I99c0670b3c2922e4a9806822d9ad37e1a364b2b8
Reviewed-on: https://go-review.googlesource.com/28472
Run-TryBot: Joe Tsai 
TryBot-Result: Gobot Gobot 
Reviewed-by: Russ Cox

archive/tar: move parse/format functionality into strconv.go

2016-09-29T18:38:28+00:00

Move all parse/format related functionality into strconv.go
and thoroughly test them. This also reduces the amount of noise
inside reader.go and writer.go.

There was zero functionality change other than moving code around.

Change-Id: I3bc288d10c20ebb3814b30b75d8acd7be62b85d7
Reviewed-on: https://go-review.googlesource.com/28470
Run-TryBot: Brad Fitzpatrick 
TryBot-Result: Gobot Gobot 
Reviewed-by: Brad Fitzpatrick

archive/tar: reapply Header.Size to regFileReader after merging

2016-09-02T21:58:53+00:00

The use of PAX headers can modify the overall file size, thus the
formerly created regFileReader may be stale.

The relevant PAX specification for this behavior is:
<<<
Any fields in the preceding optional extended header shall override
the associated fields in this header block for this file.
>>>
Where "optional extended header" refers to the preceding PAX header.
Where "this header block" refers to the subsequent USTAR header.

Fixes #15573
Fixes #15564

Change-Id: I83b1c3f05a9ca2d3be38647425ad21a9fe450ee2
Reviewed-on: https://go-review.googlesource.com/28418
Reviewed-by: Brad Fitzpatrick

archive/tar: make Reader error handling consistent

2016-08-31T23:22:53+00:00

The tar.Reader guarantees stickiness of errors. Ensuring this property means
that the methods of Reader need to be consistent about whose responsibility it
is to actually ensure that errors are sticky.

In this CL, we make it only the responsibility of the exported methods
(Next and Read) to store tr.err. All other methods just return the error as is.

As part of this change, we also check the error value of mergePAX (and test
that it properly detects invalid PAX files). Since the value of mergePAX was
never used before, we change it such that it always returns ErrHeader instead
of strconv.SyntaxError. This keeps it consistent with other usages of strconv
in the same tar package.

Change-Id: Ia1c31da71f1de4c175da89a385dec665d3edd167
Reviewed-on: https://go-review.googlesource.com/28215
Run-TryBot: Joe Tsai 
Reviewed-by: Brad Fitzpatrick

archive/tar: isolate regular and sparse file handling as methods

2016-08-25T18:33:09+00:00

Factor out the regular file handling logic into handleRegularFile
from nextHeader. We will need to reuse this logic when fixing #15573
in a future CL.

Factor out the sparse file handling logic into handleSparseFile.
Currently this logic is split between nextHeader (for GNU sparse
files) and Next (for PAX sparse files). Instead, we move this
related code into a single method.

There is no overall logic change. Thus, no unit tests.

Updates #15573 #15564

Change-Id: I3b8270d8b4e080e77d6c0df6a123d677c82cc466
Reviewed-on: https://go-review.googlesource.com/27454
Reviewed-by: Brad Fitzpatrick 
Run-TryBot: Brad Fitzpatrick 
TryBot-Result: Gobot Gobot

archive/tar: centralize all information about tar header format

2016-05-06T00:57:22+00:00

The Reader and Writer have hard-coded constants regarding the
offsets and lengths of certain fields in the tar format sprinkled
all over. This makes it harder to verify that the offsets are
correct since a reviewer would need to search for them throughout
the code. Instead, all information about the layout of header
fields should be centralized in one single file. This has the
advantage of being both centralized, and also acting as a form
of documentation about the header struct format.

This method was chosen over using "encoding/binary" since that
method would cause an allocation of a header struct every time
binary.Read was called. This method causes zero allocations and
its logic is no longer than if structs were declared.

Updates #12594

Change-Id: Ic7a0565d2a2cd95d955547ace3b6dea2b57fab34
Reviewed-on: https://go-review.googlesource.com/14669
Reviewed-by: Brad Fitzpatrick