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>

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index aa741028cc4fa85674929995fee4929c4089f7cd..449fc73c0fb281e59dc2d0236047701978e86ca0 100644 (file)
--- 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 f0aa11a74827aca142724e19d3ff68d8cd4485d8..94cf5479fca6321d4e23134b1d25ad0e44d1fda3 100644 (file)
--- 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)
+               }
+       }
+}