summaryrefslogtreecommitdiff
path: root/src/archive
Commit message (Collapse)AuthorAgeFilesLines
* archive/zip: skip large concurrent tests in race modeBrad Fitzpatrick2016-11-211-0/+7
| | | | | | | | | | | | | | | | 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>
* archive/zip: avoid overflow in record count and byte offset fieldsBrad Fitzpatrick2016-11-172-25/+302
| | | | | | | | | | | | 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>
* archive/tar: disable prefix field in WriterJoe Tsai2016-11-024-7/+98
| | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: validate sparse headers in parsePAXJoe Tsai2016-10-222-21/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: fix parsePAXTimeJoe Tsai2016-10-202-45/+106
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: fix parsePAX to be POSIX.1-2001 compliantJoe Tsai2016-10-192-2/+9
| | | | | | | | | | | | | | | | | | | 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>
* archive/tar: compact slices in testsJoe Tsai2016-10-193-573/+486
| | | | | | | | | | | | | 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>
* archive/tar: make Reader handle GNU format properlyJoe Tsai2016-10-193-3/+72
| | | | | | | | | | | | | | | | | | 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>
* archive/tar: reduce allocations in formatOctalJoe Tsai2016-10-121-4/+3
| | | | | | | | 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>
* archive/tar: fix and cleanup readOldGNUSparseMapJoe Tsai2016-10-122-39/+100
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 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 <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org>
* archive/tar: handle integer overflow on 32bit machinesJoe Tsai2016-10-122-7/+11
| | | | | | | | | | | | | | | 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>
* archive/zip: only use Extended Timestamp on non-zero MS-DOS timestampsJoe Tsai2016-10-113-9/+51
| | | | | | | | | | | | | | | 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>
* archive/zip: handle mtime in NTFS/UNIX/ExtendedTS extra fieldsYasuhiro Matsumoto2016-10-066-12/+113
| | | | | | | | | | | | | | 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>
* archive/tar: move parse/format functionality into strconv.goJoe Tsai2016-09-297-528/+573
| | | | | | | | | | | | | | 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>
* archive/tar: reapply Header.Size to regFileReader after mergingJoe Tsai2016-09-023-4/+24
| | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: make Reader error handling consistentJoe Tsai2016-08-314-104/+80
| | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: isolate regular and sparse file handling as methodsJoe Tsai2016-08-251-58/+77
| | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: preallocate slice from paxHeadersGyu-Ho Lee2016-08-161-1/+1
| | | | | | | | | | | 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>
* archive/zip: use HTTPS for documentation linkKevin Burke2016-05-141-1/+1
| | | | | | | | | 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>
* archive/tar: centralize all information about tar header formatJoe Tsai2016-05-065-209/+314
| | | | | | | | | | | | | | | | | | | | | | 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>
* archive/zip: pool flate readersNiko Dziemba2016-05-041-1/+39
| | | | | | | | | | | | | | | | | | | 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>
* archive/zip: improve BenchmarkCompressedZipGarbageJosh Bleecher Snyder2016-05-011-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: style nit: s/nano_buf/nanoBuf/Matthew Dempsky2016-04-151-6/+6
| | | | | | | | | | 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>
* all: remove unnecessary type conversionsMatthew Dempsky2016-04-152-5/+5
| | | | | | | | | | | | | 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>
* all: use new io.SeekFoo constants instead of os.SEEK_FOOBrad Fitzpatrick2016-04-132-4/+3
| | | | | | | | | | | | 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>
* all: delete dead test codeDominik Honnef2016-03-211-21/+0
| | | | | | | | | | 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>
* archive/zip: add missing argument to error messageAlberto Donizetti2016-03-151-1/+1
| | | | | | | | Silence vet. Change-Id: I987438847389500cf3b5bc545ef918c66917b51a Reviewed-on: https://go-review.googlesource.com/20683 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
* all: single space after period.Brad Fitzpatrick2016-03-021-2/+2
| | | | | | | | | | | | | | | | | | | | 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>
* all: remove public named return values when uselessBrad Fitzpatrick2016-02-291-6/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* all: use cannot instead of can notJosh Bleecher Snyder2016-02-211-1/+1
| | | | | | | | 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>
* archive/zip: handle pre-zip64 zip files containing 2³²-1-byte contentRuss Cox2016-02-022-11/+236
| | | | | | | | | | 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>
* archive/zip: clarify expectations of RegisterCompressor and RegisterDecompressorJoe Tsai2016-01-273-27/+19
| | | | | | | | | | | | 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>
* archive/zip: fix reading, writing of zip64 archivesRuss Cox2016-01-074-25/+85
| | | | | | | | | | | | | | | | | 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>
* build: shorten a few packages with long testsRuss Cox2015-12-291-0/+4
| | | | | | | | | | | | | 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>
* archive/tar: document how Reader.Read handles header-only filesJoe Tsai2015-12-171-0/+4
| | | | | | | | | | | | Commit dd5e14a7511465d20c6e95bf54c9b8f999abbbf6 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>
* archive/tar: spell license correctly in exampleJoe Tsai2015-12-171-2/+2
| | | | | | | 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>
* archive/tar: properly parse GNU base-256 encodingJoe Tsai2015-12-042-9/+96
| | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: properly format GNU base-256 encodingJoe Tsai2015-12-042-4/+105
| | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: convert Reader.Next to be loop basedJoe Tsai2015-12-024-70/+84
| | | | | | | | | | | | | | | | 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>
* archive/tar: move parse/format methods to standalone receiverJoe Tsai2015-12-024-142/+252
| | | | | | | | | | | | | 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>
* archive/tar: fix issues with readGNUSparseMap1x0Joe Tsai2015-12-012-85/+124
| | | | | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: properly handle header-only "files" in ReaderJoe Tsai2015-12-015-8/+70
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* archive/zip: enable overriding (de)compressors per fileColin Cross2015-12-013-11/+80
| | | | | | | | | | | | | | | | 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>
* archive/tar: make output deterministicMatt Layher2015-11-132-10/+64
| | | | | | | | | | | | 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>
* archive/tar: detect truncated filesJoe Tsai2015-11-064-38/+172
| | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: fix numeric overflow issues in readGNUSparseMap0x1Joe Tsai2015-10-062-33/+85
| | | | | | | | | | | | | | 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>
* archive/tar: add missing error checks to Reader.NextJoe Tsai2015-10-063-4/+18
| | | | | | | | | | | | | | | | 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>
* archive/tar: expand abilities of TestReaderJoe Tsai2015-10-061-215/+81
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* archive/tar: make Reader.Read errors persistentJoe Tsai2015-10-011-0/+4
| | | | | | | | | | 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>
* archive/tar: fix bugs with sparseFileReaderJoe Tsai2015-10-012-101/+232
| | | | | | | | | | | | | | | | | The sparseFileReader is prone to two different forms of denial-of-service attacks: * A malicious tar file can cause an infinite loop * A malicious tar file can cause arbitrary panics This results because of poor error checking/handling, which this CL fixes. While we are at it, add a plethora of unit tests to test for possible malicious inputs. Change-Id: I2f9446539d189f3c1738a1608b0ad4859c1be929 Reviewed-on: https://go-review.googlesource.com/15115 Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Andrew Gerrand <adg@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>