archive/zip: avoid data descriptor when writing directories

gwik · dsnet · commit 7b451dc715a7 · 2018-05-02T23:34:50.000Z
Java fails to unzip archives created by archive/zip because directories are written with the "data descriptor" flag (bit 3) set, but emits no such descriptor. To fix this, we explicitly clear the flag. Fixes golang#25215 Change-Id: Id3af4c7f863758197063df879717c1710f86c0e5 Reviewed-on: https://go-review.googlesource.com/110795 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/src/archive/zip/writer.go b/src/archive/zip/writer.go
@@ -263,8 +263,6 @@ func (w *Writer) CreateHeader(fh *FileHeader) (io.Writer, error) {
 		return nil, errors.New("archive/zip: invalid duplicate FileHeader")
 	}
 
-	fh.Flags |= 0x8 // we will write a data descriptor
-
 	// The ZIP format has a sad state of affairs regarding character encoding.
 	// Officially, the name and comment fields are supposed to be encoded
 	// in CP-437 (which is mostly compatible with ASCII), unless the UTF-8
@@ -331,8 +329,17 @@ func (w *Writer) CreateHeader(fh *FileHeader) (io.Writer, error) {
 	}
 
 	if strings.HasSuffix(fh.Name, "/") {
+		// Set the compression method to Store to ensure data length is truly zero,
+		// which the writeHeader method always encodes for the size fields.
+		// This is necessary as most compression formats have non-zero lengths
+		// even when compressing an empty string.
+		fh.Method = Store
+		fh.Flags &^= 0x8 // we will not write a data descriptor
+
 		ow = dirWriter{}
 	} else {
+		fh.Flags |= 0x8 // we will write a data descriptor
+
 		fw = &fileWriter{
 			zipw:      w.cw,
 			compCount: &countWriter{w: w.cw},
diff --git a/src/archive/zip/writer_test.go b/src/archive/zip/writer_test.go
@@ -6,6 +6,7 @@ package zip
 
 import (
 	"bytes"
+	"encoding/binary"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -310,6 +311,41 @@ func TestWriterDir(t *testing.T) {
 	}
 }
 
+func TestWriterDirAttributes(t *testing.T) {
+	var buf bytes.Buffer
+	w := NewWriter(&buf)
+	if _, err := w.Create("dir/"); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	b := buf.Bytes()
+
+	var sig [4]byte
+	binary.LittleEndian.PutUint32(sig[:], uint32(fileHeaderSignature))
+
+	idx := bytes.Index(b, sig[:])
+	if idx == -1 {
+		t.Fatal("file header not found")
+	}
+	b = b[idx:]
+
+	if !bytes.Equal(b[6:10], []byte{0, 0, 0, 0}) { // FileHeader.Flags: 0, FileHeader.Method: 0
+		t.Errorf("unexpected method and flags: %v", b[6:10])
+	}
+
+	if !bytes.Equal(b[14:26], make([]byte, 12)) { // FileHeader.{CRC32,CompressSize,UncompressedSize} all zero.
+		t.Errorf("unexpected crc, compress and uncompressed size to be 0 was: %v", b[14:26])
+	}
+
+	binary.LittleEndian.PutUint32(sig[:], uint32(dataDescriptorSignature))
+	if bytes.Index(b, sig[:]) != -1 {
+		t.Error("there should be no data descriptor")
+	}
+}
+
 func testCreate(t *testing.T, w *Writer, wt *WriteTest) {
 	header := &FileHeader{
 		Name:   wt.Name,
