1
0
mirror of https://github.com/golang/go synced 2024-11-15 03:40:29 -07:00
Commit Graph

76 Commits

Author SHA1 Message Date
Joe Tsai
5df59a4fc9 Revert: "archive/zip: handle mtime in NTFS/UNIX/ExtendedTS extra fields"
This change reverts the following CLs:
	CL/18274: handle mtime in NTFS/UNIX/ExtendedTS extra fields
	CL/30811: only use Extended Timestamp on non-zero MS-DOS timestamps

We are reverting support for extended timestamps since the support was not
not complete. CL/18274 added full support for reading extended timestamp fields
and minimal support for writing them. CL/18274 is incomplete because it made
no changes to the FileHeader struct, so timezone information was lost when
reading and/or writing.

While CL/18274 was a step in the right direction, we should provide full
support for high precision timestamps in both the reader and writer.
This will probably require that we add a new field of type time.Time.
The complete fix is too involved to add in the time remaining for Go 1.8
and will be completed in Go 1.9.

Updates #10242
Updates #17403
Updates #18359
Fixes #18378

Change-Id: Icf6d028047f69379f7979a29bfcb319a02f4783e
Reviewed-on: https://go-review.googlesource.com/34651
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-12-20 01:39:35 +00:00
Brad Fitzpatrick
1368de3db2 archive/zip: skip large concurrent tests in race mode
We recently added these large zip64 tests. They're slow-ish already,
but fast enough in non-race mode with t.Parallel. But in race mode,
the concurrency makes them much slower than the normal
non-race-to-race multiplier.

They're taking so long now that it's causing test failures when it
sometimes is over the test timeout threshold.

Change-Id: I02f4ceaa9d6cab826708eb3860f47a57b05bdfee
Reviewed-on: https://go-review.googlesource.com/33423
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-11-21 20:51:42 +00:00
Brad Fitzpatrick
c1e9760d4c archive/zip: avoid overflow in record count and byte offset fields
This is Quentin's https://golang.org/cl/33012 with updated tests.

Fixes #14186

Change-Id: Ib51deaab0368c6bad32ce9d6345119ff44f3c2d6
Reviewed-on: https://go-review.googlesource.com/33291
Reviewed-by: Quentin Smith <quentin@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-11-17 18:54:33 +00:00
Joe Tsai
2d3cd51dbe archive/tar: disable prefix field in Writer
The proper fix for the Writer is too involved to be done in time
for Go 1.8. Instead, we do a localized fix that simply disables the
prefix encoding logic. While this will prevent some legitimate uses
of prefix, it will ensure that we don't keep outputting invalid
GNU format files that have the prefix field populated.

For headers with long filenames that could have used the prefix field,
they will be promoted to use the PAX format, which ensures that we
will still be able to encode all headers that we were able to do before.

Updates #12594
Fixes #17630
Fixes #9683

Change-Id: Ia97b524ac69865390e2ae8bb0dfb664d40a05add
Reviewed-on: https://go-review.googlesource.com/32234
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-11-02 20:18:38 +00:00
Joe Tsai
3d4ea227c6 archive/tar: validate sparse headers in parsePAX
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 <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-22 16:35:14 +00:00
Joe Tsai
ef8e85e8b8 archive/tar: fix parsePAXTime
Issues fixed:
* Could not handle quantity of seconds greater than 1<<31 on
32bit machines since strconv.ParseInt did not treat integers as 64b.
* Did not handle negative timestamps properly if nanoseconds were used.
Note that "-123.456" should result in a call to time.Unix(-123, -456000000).
* Incorrectly allowed a '-' right after the '.' (e.g., -123.-456)
* Did not detect invalid input after the truncation point (e.g., 123.123456789badbadbad).

Note that negative timestamps are allowed by PAX, but are not guaranteed
to be portable. See the relevant specification:
<<<
If pax encounters a file with a negative timestamp in copy or write mode,
it can reject the file, substitute a non-negative timestamp, or generate
a non-portable timestamp with a leading '-'.
>>>

Since the previous behavior already partially supported negative timestamps,
we are bound by Go's compatibility rules to keep support for them.
However, we should at least make sure we handle them properly.

Change-Id: I5686997708bfb59110ea7981175427290be737d1
Reviewed-on: https://go-review.googlesource.com/31441
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-20 01:06:09 +00:00
Joe Tsai
12c9844cc6 archive/tar: fix parsePAX to be POSIX.1-2001 compliant
Relevant PAX specification:
<<<
If the <value> 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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-19 18:39:30 +00:00
Joe Tsai
04262986a0 archive/tar: compact slices in tests
Took this opportunity to also embed tables in the functions
that they are actually used in and other stylistic cleanups.

There was no logical changes to the tests.

Change-Id: Ifa724060532175f6f4407d6cedc841891efd8f7b
Reviewed-on: https://go-review.googlesource.com/31436
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-19 18:38:34 +00:00
Joe Tsai
d2aa8601b5 archive/tar: make Reader handle GNU format properly
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 <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-19 18:07:55 +00:00
Joe Tsai
14e545b60a archive/tar: reduce allocations in formatOctal
Change-Id: I9ddb7d2a97d28aba7a107b65f278993daf7807fa
Reviewed-on: https://go-review.googlesource.com/30960
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-10-12 21:27:51 +00:00
Joe Tsai
0e36456bf9 archive/tar: fix and cleanup readOldGNUSparseMap
* 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
	1fa9c7bf90/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 <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
2016-10-12 16:36:57 +00:00
Joe Tsai
14204662c8 archive/tar: handle integer overflow on 32bit machines
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 <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2016-10-12 16:36:41 +00:00
Joe Tsai
35220534d5 archive/zip: only use Extended Timestamp on non-zero MS-DOS timestamps
We should preserve the fact that a roundtrip read on fields with the zero
value should remain the zero for those that are reasonable to stay that way.
If the zero value for a MS-DOS timestamp was used, then it is sensible for
that zero value to also be read back later.

Fixes #17403

Change-Id: I32c3915eab180e91ddd2499007374f7b85f0bd76
Reviewed-on: https://go-review.googlesource.com/30811
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-10-11 21:31:25 +00:00
Yasuhiro Matsumoto
4c79ed5f44 archive/zip: handle mtime in NTFS/UNIX/ExtendedTS extra fields
Handle NTFS timestamp, UNIX timestamp, Extended extra timestamp.
Writer supports only Extended extra timestamp field, matching most
zip creators.

Fixes #10242.

Change-Id: Id665db274e63def98659231391fb77392267ac1e
Reviewed-on: https://go-review.googlesource.com/18274
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2016-10-06 19:05:52 +00:00
Joe Tsai
6fea452e38 archive/tar: move parse/format functionality into strconv.go
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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-09-29 18:38:28 +00:00
Joe Tsai
0b84a64da1 archive/tar: reapply Header.Size to regFileReader after merging
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 <bradfitz@golang.org>
2016-09-02 21:58:53 +00:00
Joe Tsai
cd0ba4c169 archive/tar: make Reader error handling consistent
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 <thebrokentoaster@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-08-31 23:22:53 +00:00
Joe Tsai
6af7639ae1 archive/tar: isolate regular and sparse file handling as methods
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 <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-08-25 18:33:09 +00:00
Gyu-Ho Lee
77e68ea78a archive/tar: preallocate slice from paxHeaders
Preallocate keys slice with the length of paxHeaders map
to prevent slice growth with append operations.

Change-Id: Ic9a927c4eaa775690a4ef912d61dd06f38e11510
Reviewed-on: https://go-review.googlesource.com/23782
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-08-16 02:46:50 +00:00
Kevin Burke
5f833121cb archive/zip: use HTTPS for documentation link
The resource is available over (and redirects to) HTTPS, it seems like a good
idea to save a redirect and ensure an encrypted connection.

Change-Id: I262c7616ae289cdd756b6f67573ba6bd7e3e0ca6
Reviewed-on: https://go-review.googlesource.com/23104
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-05-14 17:27:44 +00:00
Joe Tsai
30bfafc319 archive/tar: centralize all information about tar header format
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 <bradfitz@golang.org>
2016-05-06 00:57:22 +00:00
Niko Dziemba
e155bf0ce8 archive/zip: pool flate readers
Similar to the flate Writer pools already used,
this adds pooling for flate Readers.

compress/flate allows re-using of Readers, see
https://codereview.appspot.com/97140043/

In a real-world scenario when reading ~ 500 small files from a ZIP
archive this gives a speedup of 1.5x-2x.

Fixes #14289

Change-Id: I2d98ad983e95ab7d97e06fd0145f619b4f47caa4
Reviewed-on: https://go-review.googlesource.com/19416
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-05-04 14:28:27 +00:00
Josh Bleecher Snyder
d713e8e806 archive/zip: improve BenchmarkCompressedZipGarbage
Before this CL:

$ go test -bench=CompressedZipGarbage -count=5 -run=NONE archive/zip
BenchmarkCompressedZipGarbage-8        50  20677087 ns/op   42973 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8       100  20584764 ns/op   24294 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8        50  20859221 ns/op   42973 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8       100  20901176 ns/op   24294 B/op      47 allocs/op
BenchmarkCompressedZipGarbage-8        50  21282409 ns/op   42973 B/op      47 allocs/op

The B/op number is effectively meaningless. There
is a surprisingly large one-time cost that gets
divided by the number of iterations that your
machine can get through in a second.

This CL discards the first run, which helps.
It is not a panacea. Running with -benchtime=10s
will allow the sync.Pool to be emptied,
which brings the problem back.
However, since there are more iterations to divide
the cost through, it’s not quite as bad,
and running with a high benchtime is rare.

This CL changes the meaning of the B/op number,
which is unfortunate, since it won’t have the
same order of magnitude as previous Go versions.
But it wasn’t really comparable before anyway,
since it didn’t have any reliable meaning at all.

After this CL:

$ go test -bench=CompressedZipGarbage -count=5 -run=NONE archive/zip
BenchmarkCompressedZipGarbage-8   	     100	  20881890 ns/op	    5616 B/op	      47 allocs/op
BenchmarkCompressedZipGarbage-8   	      50	  20622757 ns/op	    5616 B/op	      47 allocs/op
BenchmarkCompressedZipGarbage-8   	      50	  20628193 ns/op	    5616 B/op	      47 allocs/op
BenchmarkCompressedZipGarbage-8   	     100	  20756612 ns/op	    5616 B/op	      47 allocs/op
BenchmarkCompressedZipGarbage-8   	     100	  20639774 ns/op	    5616 B/op	      47 allocs/op

Change-Id: Iedee04f39328974c7fa272a6113d423e7ffce50f
Reviewed-on: https://go-review.googlesource.com/22585
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-05-01 04:42:00 +00:00
Matthew Dempsky
75c079cb97 archive/tar: style nit: s/nano_buf/nanoBuf/
Pointed out during review of golang.org/cl/22104.

Change-Id: If8842e7f8146441e918ec6a2b6e893b7cf88615c
Reviewed-on: https://go-review.googlesource.com/22120
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-04-15 21:29:18 +00:00
Matthew Dempsky
0da4dbe232 all: remove unnecessary type conversions
cmd and runtime were handled separately, and I'm intentionally skipped
syscall. This is the rest of the standard library.

CL generated mechanically with github.com/mdempsky/unconvert.

Change-Id: I9e0eff886974dedc37adb93f602064b83e469122
Reviewed-on: https://go-review.googlesource.com/22104
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-04-15 07:31:45 +00:00
Brad Fitzpatrick
381e5eee39 all: use new io.SeekFoo constants instead of os.SEEK_FOO
Automated change.

Fixes #15269

Change-Id: I8deb2ac0101d3f7c390467ceb0a1561b72edbb2f
Reviewed-on: https://go-review.googlesource.com/21962
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-04-13 05:25:23 +00:00
Dominik Honnef
b2cf571040 all: delete dead test code
This deletes unused code and helpers from tests.

Change-Id: Ie31d46115f558ceb8da6efbf90c3c204e03b0d7e
Reviewed-on: https://go-review.googlesource.com/20927
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-21 07:10:08 +00:00
Alberto Donizetti
ad4410d40b archive/zip: add missing argument to error message
Silence vet.

Change-Id: I987438847389500cf3b5bc545ef918c66917b51a
Reviewed-on: https://go-review.googlesource.com/20683
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-15 19:13:21 +00:00
Brad Fitzpatrick
5fea2ccc77 all: single space after period.
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.

This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:

$ perl -i -npe 's,^(\s*// .+[a-z]\.)  +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.)  +([A-Z])')
$ go test go/doc -update

Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-02 00:13:47 +00:00
Brad Fitzpatrick
351c15f1ce all: remove public named return values when useless
Named returned values should only be used on public funcs and methods
when it contributes to the documentation.

Named return values should not be used if they're only saving the
programmer a few lines of code inside the body of the function,
especially if that means there's stutter in the documentation or it
was only there so the programmer could use a naked return
statement. (Naked returns should not be used except in very small
functions)

This change is a manual audit & cleanup of public func signatures.

Signatures were not changed if:

* the func was private (wouldn't be in public godoc)
* the documentation referenced it
* the named return value was an interesting name. (i.e. it wasn't
  simply stutter, repeating the name of the type)

There should be no changes in behavior. (At least: none intended)

Change-Id: I3472ef49619678fe786e5e0994bdf2d9de76d109
Reviewed-on: https://go-review.googlesource.com/20024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
2016-02-29 03:31:19 +00:00
Josh Bleecher Snyder
e43c74a0d8 all: use cannot instead of can not
You can not use cannot, but you cannot spell cannot can not.

Change-Id: I2f0971481a460804de96fd8c9e46a9cc62a3fc5b
Reviewed-on: https://go-review.googlesource.com/19772
Reviewed-by: Rob Pike <r@golang.org>
2016-02-21 15:35:50 +00:00
Russ Cox
b6c5edae7c archive/zip: handle pre-zip64 zip files containing 2³²-1-byte content
This corrects a regression from Go 1.5 introduced by CL 18317.

Fixes #14185.

Change-Id: Ic3215714846d9f28809cd04e3eb3664b599244f4
Reviewed-on: https://go-review.googlesource.com/19151
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-02-02 16:36:59 +00:00
Joe Tsai
c736280e22 archive/zip: clarify expectations of RegisterCompressor and RegisterDecompressor
Clarify that Compressor and Decompressor callbacks must support being invoked
concurrently, but that the writer or reader returned need not be.

Updates #8359

Change-Id: Ia407b581dd124185f165c25f5701018a8ce4357a
Reviewed-on: https://go-review.googlesource.com/18627
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2016-01-27 00:22:03 +00:00
Russ Cox
4aedbf5be4 archive/zip: fix reading, writing of zip64 archives
Read zip files that contain only 64-bit header offset, not 64-bit sizes.
Fixes #13367.

Read zip files that contain completely unexpected Extra fields,
provided we do not need to find 64-bit size or header offset information there.
Fixes #13166.

Write zip file entries with 0xFFFFFFFF uncompressed data bytes
correctly (must use zip64 header, since that's the magic indicator).
Fixes new TestZip64EdgeCase. (Noticed while working on the CL.)

Change-Id: I84a22b3995fafab8052b99de8094a9f35a25de5b
Reviewed-on: https://go-review.googlesource.com/18317
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-01-07 00:44:04 +00:00
Russ Cox
04d732b4c2 build: shorten a few packages with long tests
Takes 3% off my all.bash run time.

For #10571.

Change-Id: I8f00f523d6919e87182d35722a669b0b96b8218b
Reviewed-on: https://go-review.googlesource.com/18087
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-12-29 15:46:44 +00:00
Joe Tsai
7a2913c9f5 archive/tar: document how Reader.Read handles header-only files
Commit dd5e14a751 ensured that no data
could be read for header-only files regardless of what the Header.Size
said. We should document this fact in Reader.Read.

Updates #13647

Change-Id: I4df9a2892bc66b49e0279693d08454bf696cfa31
Reviewed-on: https://go-review.googlesource.com/17913
Reviewed-by: Russ Cox <rsc@golang.org>
2015-12-17 15:48:10 +00:00
Joe Tsai
2ae895c0ce archive/tar: spell license correctly in example
Change-Id: Ice85d161f026a991953bd63ecc6ec80f8d06dfbd
Reviewed-on: https://go-review.googlesource.com/17901
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-17 02:46:41 +00:00
Joe Tsai
5ebb374f96 archive/tar: properly parse GNU base-256 encoding
Motivation:
* Previous implementation did not detect integer overflow when
parsing a base-256 encoded field.
* Previous implementation did not treat the integer as a two's
complement value as specified by GNU.

The relevant GNU specification says:
<<<
GNU format uses two's-complement base-256 notation to store values
that do not fit into standard ustar range.
>>>

Fixes #12435

Change-Id: I4639bcffac8d12e1cb040b76bd05c9d7bc6c23a8
Reviewed-on: https://go-review.googlesource.com/17424
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-12-04 22:38:15 +00:00
Joe Tsai
481eedce62 archive/tar: properly format GNU base-256 encoding
Motivation:
* Previous implementation silently failed when an integer overflow
occurred. Now, we report an ErrFieldTooLong.
* Previous implementation did not encode in two's complement format and was
unable to encode negative numbers.

The relevant GNU specification says:
<<<
GNU format uses two's-complement base-256 notation to store values
that do not fit into standard ustar range.
>>>

Fixes #12436

Change-Id: I09c20602eabf8ae3a7e0db35b79440a64bfaf807
Reviewed-on: https://go-review.googlesource.com/17425
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-12-04 21:38:38 +00:00
Joe Tsai
b8a12928a6 archive/tar: convert Reader.Next to be loop based
Motivation for change:
* Recursive logic is hard to follow, since it tends to apply
things in reverse. On the other hand, the tar formats tend to
describe meta headers as affecting the next entry.
* Recursion also applies changes in the wrong order. Two test
files are attached that use multiple headers. The previous Go
behavior differs from what GNU and BSD tar do.

Change-Id: Ic1557256fc1363c5cb26570e5d0b9f65a9e57341
Reviewed-on: https://go-review.googlesource.com/14624
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-02 02:27:27 +00:00
Joe Tsai
38f8e4407c archive/tar: move parse/format methods to standalone receiver
Motivations for this change:
* It allows these functions to be used outside of Reader/Writer.
* It allows these functions to be more easily unit tested.

Change-Id: Iebe2b70bdb8744371c9ffa87c24316cbbf025b59
Reviewed-on: https://go-review.googlesource.com/15113
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-02 00:29:33 +00:00
Joe Tsai
7823197e5d archive/tar: fix issues with readGNUSparseMap1x0
Motivations:
* Use of strconv.ParseInt does not properly treat integers as 64bit,
preventing this function from working properly on 32bit machines.
* Use of io.ReadFull does not properly detect truncated streams
when the file suddenly ends on a block boundary.
* The function blindly trusts user input for numEntries and allocates
memory accordingly.
* The function does not validate that numEntries is not negative,
allowing a malicious sparse file to cause a panic during make.

In general, this function was overly complicated for what it was
accomplishing and it was hard to reason that it was free from
bounds errors. Instead, it has been rewritten and relies on
bytes.Buffer.ReadString to do the main work. So long as invariants
about the number of '\n' in the buffer are maintained, it is much
easier to see why this approach is correct.

Change-Id: Ibb12c4126c26e0ea460ea063cd17af68e3cf609e
Reviewed-on: https://go-review.googlesource.com/15174
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-12-01 20:22:38 +00:00
Joe Tsai
dd5e14a751 archive/tar: properly handle header-only "files" in Reader
Certain special type-flags, specifically 1, 2, 3, 4, 5, 6,
do not have a data section. Thus, regardless of what the size field
says, we should not attempt to read any data for these special types.

The relevant PAX and USTAR specification says:
<<<
If the typeflag field is set to specify a file to be of type 1 (a link)
or 2 (a symbolic link), the size field shall be specified as zero.
If the typeflag field is set to specify a file of type 5 (directory),
the size field shall be interpreted as described under the definition
of that record type. No data logical records are stored for types 1, 2, or 5.
If the typeflag field is set to 3 (character special file),
4 (block special file), or 6 (FIFO), the meaning of the size field is
unspecified by this volume of POSIX.1-2008, and no data logical records shall
be stored on the medium.
Additionally, for type 6, the size field shall be ignored when reading.
If the typeflag field is set to any other value, the number of logical
records written following the header shall be (size+511)/512, ignoring
any fraction in the result of the division.
>>>

Contrary to the specification, we do not assert that the size field
is zero for type 1 and 2 since we liberally accept non-conforming formats.

Change-Id: I666b601597cb9d7a50caa081813d90ca9cfc52ed
Reviewed-on: https://go-review.googlesource.com/16614
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-12-01 20:16:26 +00:00
Colin Cross
46300a058d archive/zip: enable overriding (de)compressors per file
Implement setting the compression level for a zip archive by registering
a per-Writer compressor through Writer.RegisterCompressor.  If no
compressors are registered, fall back to the ones registered at the
package level.  Also implements per-Reader decompressors.

Fixes #8359

Change-Id: I93b27c81947b0f817b42e0067aa610ff267fdb21
Reviewed-on: https://go-review.googlesource.com/16669
Reviewed-by: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Klaus Post <klauspost@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-01 20:04:20 +00:00
Matt Layher
3a3049897c archive/tar: make output deterministic
Replaces PID in PaxHeaders with 0.  Sorts PAX header keys before writing
them to the archive.

Fixes #12358

Change-Id: If239f89c85f1c9d9895a253fb06a47ad44960124
Reviewed-on: https://go-review.googlesource.com/13975
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Joe Tsai <joetsai@digital-static.net>
2015-11-13 02:02:32 +00:00
Joe Tsai
e3b615fd6c archive/tar: detect truncated files
Motivation:
* Reader.skipUnread never reports io.ErrUnexpectedEOF. This is strange
given that io.ErrUnexpectedEOF is given through Reader.Read if the
user manually reads the file.
* Reader.skipUnread fails to detect truncated files since io.Seeker
is lazy about reporting errors. Thus, the behavior of Reader differs
whether the input io.Reader also satisfies io.Seeker or not.

To solve this, we seek to one before the end of the data section and
always rely on at least one call to io.CopyN. If the tr.r satisfies
io.Seeker, this is guarunteed to never read more than blockSize.

Fixes #12557

Change-Id: I0ddddfc6bed0d74465cb7e7a02b26f1de7a7a279
Reviewed-on: https://go-review.googlesource.com/15175
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-11-06 04:31:26 +00:00
Joe Tsai
e4add8d569 archive/tar: fix numeric overflow issues in readGNUSparseMap0x1
Motivation:
* The logic to verify the numEntries can overflow and incorrectly
pass, allowing a malicious file to allocate arbitrary memory.
* The use of strconv.ParseInt does not set the integer precision
to 64bit, causing this code to work incorrectly on 32bit machines.

Change-Id: I1b1571a750a84f2dde97cc329ed04fe2342aaa60
Reviewed-on: https://go-review.googlesource.com/15173
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-10-06 17:49:05 +00:00
Joe Tsai
281eabe46f archive/tar: add missing error checks to Reader.Next
A recursive call to Reader.Next did not check the error before
trying to use the result, leading to a nil pointer panic.
This specific CL addresses the immediate issue, which is the panic,
but does not solve the root issue, which is due to an integer
overflow in the base-256 parser.

Updates #12435

Change-Id: Ia908671f0f411a409a35e24f2ebf740d46734072
Reviewed-on: https://go-review.googlesource.com/15437
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-10-06 17:13:11 +00:00
Joe Tsai
cfb116d453 archive/tar: expand abilities of TestReader
Motivation:
* There are an increasing number of "one-off" corrupt files added
to make sure that package does not succeed or crash on them.
Instead, allow for the test to specify the error that is expected
to occur (if any).
* Also, fold in the logic to check the MD5 checksum into this
function.

The following tests are being removed:
* TestIncrementalRead: Done by TestReader by using io.CopyBuffer
with a buffer of 8. This achieves the same behavior as this test.
* TestSparseEndToEnd: Since TestReader checks the MD5 checksums
if the input corpus provides them, then this is redundant.
* TestSparseIncrementalRead: Redundant for the same reasons that
TestIncrementalRead is now redundant
* TestNegativeHdrSize: Added to TestReader corpus
* TestIssue10968: Added to TestReader corpus
* TestIssue11169: Added to TestReader corpus

With this change, code coverage did not change: 85.3%

Change-Id: I8550d48657d4dbb8f47dfc3dc280758ef73b47ec
Reviewed-on: https://go-review.googlesource.com/15176
Reviewed-by: Andrew Gerrand <adg@golang.org>
2015-10-06 05:06:58 +00:00
Joe Tsai
02d2db18a7 archive/tar: make Reader.Read errors persistent
If the stream is in an inconsistent state, it does not make sense
that Reader.Read can be called and possibly succeed.

Change-Id: I9d1c5a1300b2c2b45232188aa7999e350809dcf2
Reviewed-on: https://go-review.googlesource.com/15177
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2015-10-01 22:33:33 +00:00