Skip to content

Commit

Permalink
archive/zip: tolerate compressed directories with zero uncompressed size
Browse files Browse the repository at this point in the history 
In CL 449955 we made reading of directories with associated file data
an error, since it is a "must not" in the zip specification. It turns
out that a number of implementations make the mistake of not setting
the correct compression method on directories (in particular the Java
jar tool does this when storing the META-INF directory). If the
compression method used is not 0 (stored) then the compressed size of
the directory can be > 0, despite the uncompressed size still being 0.

Since this mistake is not uncommon, we are forced to tolerate it. We
still fail if the recorded uncompressed size is > 0, which should be
a significantly harder mistake to make.

Change-Id: Ia732b10787f26ab937ac9cf9869ac3042efb8118
Reviewed-on: https://go-review.googlesource.com/c/go/+/454475
Reviewed-by: Ian Lance Taylor <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
  • Loading branch information
@rolandshoemaker @gopherbot
rolandshoemaker authored and gopherbot committed Dec 1, 2022
1 parent 34ab0bc commit 791f758
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/archive/zip/reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,16 @@ func (f *File) Open() (io.ReadCloser, error) {
return nil, err
}
if strings.HasSuffix(f.Name, "/") {
if f.CompressedSize64 != 0 || f.hasDataDescriptor() {
// The ZIP specification (APPNOTE.TXT) specifies that directories, which
// are technically zero-byte files, must not have any associated file
// data. We previously tried failing here if f.CompressedSize64 != 0,
// but it turns out that a number of implementations (namely, the Java
// jar tool) don't properly set the storage method on directories
// resulting in a file with compressed size > 0 but uncompressed size ==
// 0. We still want to fail when a directory has associated uncompressed
// data, but we are tolerant of cases where the uncompressed size is
// zero but compressed size is not.
if f.UncompressedSize64 != 0 || f.hasDataDescriptor() {
return &dirReader{ErrFormat}, nil
} else {
return &dirReader{io.EOF}, nil
Expand Down
66 changes: 66 additions & 0 deletions src/archive/zip/reader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1642,3 +1642,69 @@ func TestDisableInsecurePathCheck(t *testing.T) {
t.Errorf("NewReader with zipinsecurepath=1: got files %q, want %q", gotPaths, want)
}
}

func TestCompressedDirectory(t *testing.T) {
// Empty Java JAR, with a compressed directory with uncompressed size 0
// which should not fail.
//
// Length Method Size Cmpr Date Time CRC-32 Name
// -------- ------ ------- ---- ---------- ----- -------- ----
// 0 Defl:N 2 0% 12-01-2022 16:50 00000000 META-INF/
// 60 Defl:N 59 2% 12-01-2022 16:50 af937e93 META-INF/MANIFEST.MF
// -------- ------- --- -------
// 60 61 -2% 2 files
data := []byte{
0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x08,
0x08, 0x00, 0x49, 0x86, 0x81, 0x55, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x09, 0x00, 0x04, 0x00, 0x4d, 0x45,
0x54, 0x41, 0x2d, 0x49, 0x4e, 0x46, 0x2f, 0xfe,
0xca, 0x00, 0x00, 0x03, 0x00, 0x50, 0x4b, 0x07,
0x08, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x4b, 0x03,
0x04, 0x14, 0x00, 0x08, 0x08, 0x08, 0x00, 0x49,
0x86, 0x81, 0x55, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14,
0x00, 0x00, 0x00, 0x4d, 0x45, 0x54, 0x41, 0x2d,
0x49, 0x4e, 0x46, 0x2f, 0x4d, 0x41, 0x4e, 0x49,
0x46, 0x45, 0x53, 0x54, 0x2e, 0x4d, 0x46, 0xf3,
0x4d, 0xcc, 0xcb, 0x4c, 0x4b, 0x2d, 0x2e, 0xd1,
0x0d, 0x4b, 0x2d, 0x2a, 0xce, 0xcc, 0xcf, 0xb3,
0x52, 0x30, 0xd4, 0x33, 0xe0, 0xe5, 0x72, 0x2e,
0x4a, 0x4d, 0x2c, 0x49, 0x4d, 0xd1, 0x75, 0xaa,
0x04, 0x0a, 0x00, 0x45, 0xf4, 0x0c, 0x8d, 0x15,
0x34, 0xdc, 0xf3, 0xf3, 0xd3, 0x73, 0x52, 0x15,
0x3c, 0xf3, 0x92, 0xf5, 0x34, 0x79, 0xb9, 0x78,
0xb9, 0x00, 0x50, 0x4b, 0x07, 0x08, 0x93, 0x7e,
0x93, 0xaf, 0x3b, 0x00, 0x00, 0x00, 0x3c, 0x00,
0x00, 0x00, 0x50, 0x4b, 0x01, 0x02, 0x14, 0x00,
0x14, 0x00, 0x08, 0x08, 0x08, 0x00, 0x49, 0x86,
0x81, 0x55, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x09, 0x00,
0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x4d, 0x45, 0x54, 0x41, 0x2d, 0x49, 0x4e, 0x46,
0x2f, 0xfe, 0xca, 0x00, 0x00, 0x50, 0x4b, 0x01,
0x02, 0x14, 0x00, 0x14, 0x00, 0x08, 0x08, 0x08,
0x00, 0x49, 0x86, 0x81, 0x55, 0x93, 0x7e, 0x93,
0xaf, 0x3b, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x00,
0x00, 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3d,
0x00, 0x00, 0x00, 0x4d, 0x45, 0x54, 0x41, 0x2d,
0x49, 0x4e, 0x46, 0x2f, 0x4d, 0x41, 0x4e, 0x49,
0x46, 0x45, 0x53, 0x54, 0x2e, 0x4d, 0x46, 0x50,
0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x02,
0x00, 0x02, 0x00, 0x7d, 0x00, 0x00, 0x00, 0xba,
0x00, 0x00, 0x00, 0x00, 0x00,
}
r, err := NewReader(bytes.NewReader(data), int64(len(data)))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
for _, f := range r.File {
_, err = f.Open()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
}

0 comments on commit 791f758

Please sign in to comment.