Move all sentinel errors to common.go since some of them are
returned by both the reader and writer and remove errInvalidHeader
since it not used.
Also, consistently use the "tar: " prefix for errors.
Change-Id: I0afb185bbf3db80dfd9595321603924454a4c2f9
Reviewed-on: https://go-review.googlesource.com/55650
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Both the GNU and PAX formats support atime and ctime fields.
The implementation is trivial now that we have:
* support for formatting PAX records for timestamps
* dedicated methods that only handle one format (e.g., GNU)
Fixes#17876
Change-Id: I0c604fce14a47d722098afc966399cca2037395d
Reviewed-on: https://go-review.googlesource.com/55570
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We forbid empty keys or keys with '=' because it leads to ambiguous parsing.
Relevent PAX specification:
<<<
A keyword shall not include an <equals-sign>.
>>>
Also, we forbid the writer from encoding records with an empty value.
While, this is a valid record syntactically, the semantics of an empty
value is that previous records with that key should be deleted.
Since we have no support (and probably never will) for global PAX records,
deletion is a non-sensible operation.
<<<
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.
>>>
Fixes#20698Fixes#15567
Change-Id: Ia29c5c6ef2e36cd9e6d7f6cff10e92b96a62f0d1
Reviewed-on: https://go-review.googlesource.com/55571
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add support for PAX subsecond resolution times. Since the parser
supports negative timestamps, the formatter also handles negative
timestamps.
The relevant PAX specification is:
<<<
Portable file timestamps cannot be negative. 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 '-'.
>>>
<<<
All of these time records shall be formatted as a decimal
representation of the time in seconds since the Epoch.
If a <period> ( '.' ) decimal point character is present,
the digits to the right of the point shall represent the units of
a subsecond timing granularity, where the first digit is tenths of
a second and each subsequent digit is a tenth of the previous digit.
>>>
Fixes#11171
Change-Id: Ied108f3d2654390bc1b0ddd66a4081c2b83e490b
Reviewed-on: https://go-review.googlesource.com/55552
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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 write 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.
>>>
Fixes#15565
Change-Id: Id11886b723b3b13deb15221dca51c25cd778a6b5
Reviewed-on: https://go-review.googlesource.com/55553
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Both GNU and BSD tar do not care if the devmajor and devminor values are
set on entries (like regular files) that aren't character or block devices.
While this is non-sensible, it is more consistent with the Writer to actually
read these fields always. In a vast majority of the cases these will still
be zero. In the rare situation where someone actually cares about these,
at least information was not silently lost.
Change-Id: I6e4ba01cd897a1b13c28b1837e102a4fdeb420ba
Reviewed-on: https://go-review.googlesource.com/55572
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previous CLs (CL/54970, CL55231, and CL/55237) re-implemented tar.Writer
entirely using specialized methods (writeUSTARHeader, writePAXHeader,
and writeGNUHeader) allowing tar.Writer to entirely side-step the broken
and buggy logic in writeHeader.
Since writeHeader and writePAXHeaderLegacy is now dead-code,
we can delete them.
One minor change is that we call Writer.Flush at the start of WriteHeader.
This used to be performed by writeHeader, but doing so in WriteHeader
ensures each of the specialized methods can benefit from its effect.
Fixes#17665Fixes#12594
Change-Id: Iff2ef8e7310d40ac5484d2f8852fc5df25201426
Reviewed-on: https://go-review.googlesource.com/55550
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rather than going through writeHeader, which attempts to handle all formats,
implement writeGNUHeader, which only has an understanding of the GNU format.
Currently, the implementation is nearly identical to writeUSTARHeader, except:
* formatNumeric is used instead of formatOctal
* the GNU magic value is used
This is kept as a separate method since it makes more logical sense
when we add support for sparse files, long filenames, and atime/ctime fields,
which do not affect USTAR.
Updates #12594
Change-Id: I76efc0b39dc649efc22646dfc9867a7c165f34a8
Reviewed-on: https://go-review.googlesource.com/55237
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Since test files don't exceed 10KiB, print the full context of the diff,
including bytes that are equal.
Also, fix the labels for got and want; they were backwards before.
Change-Id: Ibac022e5f988d26812c3f75b643cae8b95603fc9
Reviewed-on: https://go-review.googlesource.com/55151
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Rather than going through writeHeader, which attempts to handle all formats,
implement writePAXHeader, which only has an understanding of the PAX format.
In PAX, the USTAR header is filled out in a best-effort manner.
Thus, we change logic of formatString and formatOctal to try their best to
output something (possibly truncated) in the event of an error.
The new implementation of PAX headers causes several tests to fail.
An investigation into the new output reveals that the new behavior is correct,
while the tests had actually locked in incorrect behavior before.
A dump of the differences is listed below (-before, +after):
<< writer-big.tar >>
This change is due to fact that we changed the Header.Devminor to force the
tar.Writer to choose the GNU format over the PAX one.
The ability to control the output is an open issue (see #18710).
- 00000150 00 30 30 30 30 30 30 30 00 00 00 00 00 00 00 00 |.0000000........|
+ 00000150 00 ff ff ff ff ff ff ff ff 00 00 00 00 00 00 00 |................|
<< writer-big-long.tar>>
The previous logic generated the GNU magic values for a PAX file.
The new logic correctly uses the USTAR magic values.
- 00000100 00 75 73 74 61 72 20 20 00 00 00 00 00 00 00 00 |.ustar ........|
- 00000500 00 75 73 74 61 72 20 20 00 67 75 69 6c 6c 61 75 |.ustar .guillau|
+ 00000100 00 75 73 74 61 72 00 30 30 00 00 00 00 00 00 00 |.ustar.00.......|
+ 00000500 00 75 73 74 61 72 00 30 30 67 75 69 6c 6c 61 75 |.ustar.00guillau|
The previous logic tried to use the specified timestmap in the PAX headers file,
but this is problematic as this timestamp can overflow, defeating the point
of using PAX, which is intended to extend tar.
The new logic uses the zero timestamp similar to what GNU and BSD tar do.
- 00000080 30 30 30 30 32 33 32 00 31 32 33 33 32 37 37 30 |0000232.12332770|
+ 00000080 30 30 30 30 32 35 36 00 30 30 30 30 30 30 30 30 |0000256.00000000|
The previous logic populated the devminor and devmajor fields.
The new logic leaves them zeroed just like what GNU and BSD tar do.
- 00000140 00 00 00 00 00 00 00 00 00 30 30 30 30 30 30 30 |.........0000000|
- 00000150 00 30 30 30 30 30 30 30 00 00 00 00 00 00 00 00 |.0000000........|
+ 00000140 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
+ 00000150 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
The previous logic uses PAX headers, but fails to add a record for the size.
The new logic does properly add a record for the size.
- 00000290 31 36 67 69 67 2e 74 78 74 0a 00 00 00 00 00 00 |16gig.txt.......|
- 000002a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
+ 00000290 31 36 67 69 67 2e 74 78 74 0a 32 30 20 73 69 7a |16gig.txt.20 siz|
+ 000002a0 65 3d 31 37 31 37 39 38 36 39 31 38 34 0a 00 00 |e=17179869184...|
The previous logic encoded the size as a base-256 field,
which is only valid in GNU, but the previous PAX headers implies this should
be a PAX file. This result in a strange hybrid that is neither GNU nor PAX.
The new logic uses PAX headers to store the size.
- 00000470 37 35 30 00 30 30 30 31 37 35 30 00 80 00 00 00 |750.0001750.....|
- 00000480 00 00 00 04 00 00 00 00 31 32 33 33 32 37 37 30 |........12332770|
+ 00000470 37 35 30 00 30 30 30 31 37 35 30 00 30 30 30 30 |750.0001750.0000|
+ 00000480 30 30 30 30 30 30 30 00 31 32 33 33 32 37 37 30 |0000000.12332770|
<< ustar.issue12594.tar >>
The previous logic used the specified timestamp for the PAX headers file.
The new logic just uses the zero timestmap.
- 00000080 30 30 30 30 32 33 31 00 31 32 31 30 34 34 30 32 |0000231.12104402|
+ 00000080 30 30 30 30 32 33 31 00 30 30 30 30 30 30 30 30 |0000231.00000000|
The previous logic populated the devminor and devmajor fields.
The new logic leaves them zeroed just like what GNU and BSD tar do.
- 00000140 00 00 00 00 00 00 00 00 00 30 30 30 30 30 30 30 |.........0000000|
- 00000150 00 30 30 30 30 30 30 30 00 00 00 00 00 00 00 00 |.0000000........|
+ 00000140 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
+ 00000150 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
Change-Id: I33419eb1124951968e9d5a10d50027e03133c811
Reviewed-on: https://go-review.googlesource.com/55231
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use a simple []byte instead of bytes.Buffer to create a string.
Use bytes.IndexByte instead of our own for loop.
Change-Id: Ic4a1161d79017fd3af086a05c53d5f20a5f09326
Reviewed-on: https://go-review.googlesource.com/54752
Reviewed-by: Avelino <t@avelino.xxx>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
By replacing fmt.Sprintf with a simple string concat, we see
pretty good improvements across the board on time and memory.
name old time/op new time/op delta
FormatPAXRecord 683ns ± 2% 210ns ± 5% -69.22% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
FormatPAXRecord 112B ± 0% 32B ± 0% -71.43% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
FormatPAXRecord 8.00 ± 0% 2.00 ± 0% -75.00% (p=0.000 n=10+10)
Ran with - -cpu=1 -count=10 on an AMD64 i5-5200U CPU @ 2.20GHz
Using the following benchmark:
func BenchmarkFormatPAXRecord(b *testing.B) {
for n := 0; n < b.N; n++ {
formatPAXRecord("foo", "bar")
}
}
Change-Id: I828ddbafad2e5d937f0cf5f777b512638344acfc
Reviewed-on: https://go-review.googlesource.com/55210
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Rather than going through the complicated logic of writeHeader,
implement a writeUSTARHeader that only knows about the USTAR format.
This makes the logic much easier to reason about since you only
need to be concerned about USTAR and not all the subtle
differences between USTAR, PAX, and GNU.
We seperate out the logic in writeUSTARHeader into templateV7Plus
and writeRawHeader since the planned implementations of
writePAXHeader and writeGNUHeader will use them.
Change-Id: Ie75a54ac998420ece82686159ae6fa39f8b128e9
Reviewed-on: https://go-review.googlesource.com/54970
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The current logic in writeHeader attempts to encode the Header in one
format and if it discovered that it could not it would attempt to
switch to a different format mid-way through. This makes it very
hard to reason about what format will be used in the end and whether
it will even be a valid format.
Instead, we should verify from the start what formats are allowed
to encode the given input Header. If no formats are possible,
then we can return immediately, rejecting the Header.
For now, we continue on to the hairy logic in writeHeader, but
a future CL can split that logic up and specialize them for each
format now that we know what is possible.
Update #9683
Update #12594
Change-Id: I8406ea855dfcb8b478a03a7058ddf8b2b09d46dc
Reviewed-on: https://go-review.googlesource.com/54433
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The prior logic would over-write the NUL-terminator if the octal value
was long enough. In order to prevent this, we add a fitsInOctal function
that does the proper check.
The relevant USTAR specification about NUL-terminator is:
<<<
Each numeric field is terminated by one or more <space> or NUL characters.
>>>
Change-Id: I6fbc6e8fe71168727eea201925d0fe08d43116ac
Reviewed-on: https://go-review.googlesource.com/54432
Reviewed-by: Ian Lance Taylor <iant@golang.org>
USTAR and GNU strings are NUL-terminated. Thus, we should never
allow the NUL terminator, otherwise we will lose data round-trip.
Relevant specification text:
<<<
The fields magic, uname, and gname are character strings each terminated by a NUL character.
>>>
Technically, PAX keys and values should be UTF-8, but the observance
of invalid files in the wild causes us to be more liberal.
<<<
The <length> field, <blank>, <equals-sign>, and <newline> shown shall
be limited to the portable character set, as encoded in UTF-8.
>>>
Thus, we only reject NULs in PAX keys, and NULs for PAX values
representing the USTAR string fields (i.e., path, linkpath, uname, gname).
These are treated more strictly because they represent strings that
are typically represented as C-strings on POSIX systems.
Change-Id: I305b794d9d966faad852ff660bd0b3b0964e52bf
Reviewed-on: https://go-review.googlesource.com/14724
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Given that sparse file logic is not trivial, there should be a test
in TestPartialRead to ensure that partial reads work.
Change-Id: I913da3e331da06dca6758a8be3f5099abba233a6
Reviewed-on: https://go-review.googlesource.com/54430
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The encoding/hex package provides a nice Dump formatter that
prints both hex and ASCII. Use that instead for better visual
debugging of binary diffs.
Change-Id: Iad1084e8e52d7d523595e97ae20912657cea2ab5
Reviewed-on: https://go-review.googlesource.com/14729
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Prior to Go1.8, the Writer had a bug where it would output
an invalid tar file in certain rare situations because the logic
incorrectly believed that the old GNU format had a prefix field.
This is wrong and leads to an output file that mangles the
atime and ctime fields, which are often left unused.
In order to continue reading tar files created by former, buggy
versions of Go, we skeptically parse the atime and ctime fields.
If we are unable to parse them and the prefix field looks like
an ASCII string, then we fallback on the pre-Go1.8 behavior
of treating these fields as the USTAR prefix field.
Note that this will not use the fallback logic for all possible
files generated by a pre-Go1.8 toolchain. If the generated file
happened to have a prefix field that parses as valid
atime and ctime fields (e.g., when they are valid octal strings),
then it is impossible to distinguish between an valid GNU file
and an invalid pre-Go1.8 file.
Fixes#21005
Change-Id: Iebf5c67c08e0e46da6ee41a2e8b339f84030dd90
Reviewed-on: https://go-review.googlesource.com/53635
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In Go1.0, Writer.Flush used to finish off the current file with zeros
(if it was not already finished) and then write the padding.
Since Go1.1, a regression was made (https://golang.org/cl/5777064) where it was
an error to call Flush if the current file was incomplete. Thus, Flush now only
writes out the final padding bytes, which arguably isn't very useful to anyone.
Since this has been the behavior of Flush for 9 releases of Go (1.1 to 1.9),
we should keep this behavior and just simplify the logic.
We also mark the method as deprecated since it serves no purpose.
Change-Id: I94610d942cb75cad495efd8cf799c1a275a21751
Reviewed-on: https://go-review.googlesource.com/54434
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Prior to this change, if the Writer needed to use the PAX format, it would
output a USTAR header with an empty name. This should be okay since the PAX
specification dictates that the PAX record for "path" should override the
semantic meaning of any of the old USTAR fields.
Unfortunately, the implementation of tar on OpenBSD 6.1 is too strict with
their handling of PAX files such that they check for the validity of this
bogus field even though the PAX header is present.
To allow Go's Writer output be parsible by OpenBSD's tar utility,
we write a best-effort (ASCII-only and truncated) version of the original
file name. Note that this still fails in some edge-cases (for example,
a Chinese filename containing all non-ASCII characters). OpenBSD should really
relax their checking, as you honestly can't always expect a sensible path
to be generated when USTAR cannot handle the original path.
Fixes#20707
Change-Id: Id7d77349023d2152d7291d582cd050b6681760e4
Reviewed-on: https://go-review.googlesource.com/46914
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When writing tar files by using the FileInfoHeader
the type bits was set in the mode field of the header
This is not correct according to the standard (GNU/Posix) and
other implementations.
Fixed#20150
Change-Id: I3be7d946a1923ad5827cf45c696546a5e287ebba
Reviewed-on: https://go-review.googlesource.com/42093
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When using Lstat against symlinks that point to a directory,
the function returns FileInfo with both ModeDir and ModeSymlink set.
Change that to never set ModeDir if ModeSymlink is set.
Fixes#10424Fixes#17540Fixes#17541
Change-Id: Iba280888aad108360b8c1f18180a24493fe7ad2b
Reviewed-on: https://go-review.googlesource.com/41830
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 #12594Fixes#17630Fixes#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>
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>
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>
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>
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>
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>
* 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#n7311fa9c7bf90/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>
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>
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>
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#15573Fixes#15564
Change-Id: I83b1c3f05a9ca2d3be38647425ad21a9fe450ee2
Reviewed-on: https://go-review.googlesource.com/28418
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Convert splitUSTARPath to return a bool rather than an error since
the caller never ever uses the error other than to check if it is
nil. Thus, we can remove errNameTooLong as well.
Also, fold the checking of the length <= fileNameSize and whether
the string is ASCII into the split function itself.
Lastly, remove logic to set the MAGIC since that's already done on
L200. Thus, setting the magic is redundant.
There is no overall logic change.
Updates #12638
Change-Id: I26b6992578199abad723c2a2af7f4fc078af9c17
Reviewed-on: https://go-review.googlesource.com/14723
Reviewed-by: David Symonds <dsymonds@golang.org>
Run-TryBot: David Symonds <dsymonds@golang.org>
Do not assume that if stat shows multiple links that we should mark the
file as a hardlink in the tar format. If the hardlink link was not
referenced, this caused a link to "/". On an overlay file system, all
files have multiple links.
The caller must keep the inode references and set TypeLink, Size = 0,
and LinkName themselves.
Change-Id: I873b8a235bc8f8fbb271db74ee54232da36ca013
Reviewed-on: https://go-review.googlesource.com/13045
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The issue was identified while
working with round trip FileInfo of the headers of hardlinks. Also,
additional test cases for hard link handling.
(review carried over from http://golang.org/cl/165860043)
Fixes#9027
Change-Id: I9e3a724c8de72eb1b0fbe0751a7b488894911b76
Reviewed-on: https://go-review.googlesource.com/6790
Reviewed-by: Russ Cox <rsc@golang.org>
Check for errors when reading the headers following the pax headers.
Fixes#11169.
Change-Id: Ifec4a949ec8df8b49fa7cb7a67eb826fe2282ad8
Reviewed-on: https://go-review.googlesource.com/11031
Reviewed-by: Russ Cox <rsc@golang.org>
Sanity check the pax-header size field before using it.
Fixes#11167.
Change-Id: I9d5d0210c3990e6fb9434c3fe333be0d507d5962
Reviewed-on: https://go-review.googlesource.com/10954
Reviewed-by: David Symonds <dsymonds@golang.org>
Write should return ErrWriteAfterClose instead
of ErrWriteTooLong when called after Close.
Change-Id: If5ec4ef924e4c56489e0d426976f7e5fad79be9b
Reviewed-on: https://go-review.googlesource.com/9259
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Creating a tar containing files with 0000 permission bits is
not going to be useful.
Change-Id: Ie489c2891c335d32270b18f37b0e32ecdca536a6
Reviewed-on: https://go-review.googlesource.com/3271
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
