archive/zip: tolerate compressed directories with zero uncompressed size

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 <iant@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>

2 files changed, 76 insertions, 1 deletions

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index aa741028cc..449fc73c0f 100644
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -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
diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
index f0aa11a748..94cf5479fc 100644
--- a/src/archive/zip/reader_test.go
+++ b/src/archive/zip/reader_test.go
@@ -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)
+		}
+	}
+}