[go] archive/zip: only write necessary Zip64 extra field fields
5 views
Skip to first unread message
Cyber Hamza (Gerrit)
Feb 19, 2026, 6:25:57 AMFeb 19
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Cyber Hamza has uploaded the change for review![]( "Open in Gerrit")
Commit message
archive/zip: only write necessary Zip64 extra field fields
The ZIP specification (APPNOTE.TXT) specifies that the Zip64 extra
field in the central directory should only contain those fields that
actually exceed the 32-bit limits.
Go previously always wrote all three fields (uncompressed size,
compressed size, and offset) if any of them required Zip64. This
caused issues with some strict unarchivers.
This change optimizes the writing logic to only include the necessary
fields and dynamically calculate the extra field size.
Fixes #33116
Change-Id: Iaf97b782344563df325d679ef3c6ca23069518bd
Change diff
diff --git a/src/archive/zip/issue33116_test.go b/src/archive/zip/issue33116_test.go
new file mode 100644
index 0000000..774e73f
--- /dev/null
+++ b/src/archive/zip/issue33116_test.go
@@ -0,0 +1,154 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package zip
+
+import (
+ "bytes"
+ "io"
+ "testing"
+)
+
+func TestIssue33116(t *testing.T) {
+ const uint32max = 0xffffffff
+
+ // Test case 1: Large offset, small sizes.
+ // Extra field should contain only the offset (8 bytes).
+ t.Run("LargeOffsetSmallSizes", func(t *testing.T) {
+ var buf bytes.Buffer
+ zw := NewWriter(&buf)
+ // Mock a large offset by setting it directly.
+ const offset = int64(uint32max) + 123
+ zw.SetOffset(offset)
+
+ w, err := zw.Create("test.txt")
+ if err != nil {
+ t.Fatal(err)
+ }
+ w.Write([]byte("hello"))
+ zw.Close()
+
+ // Use a mock ReaderAt to simulate the large offset without allocating 4GB.
+ ra := &mockReaderAt{
+ data: buf.Bytes(),
+ offset: offset,
+ }
+ totalSize := offset + int64(buf.Len())
+
+ r, err := NewReader(ra, totalSize)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(r.File) != 1 {
+ t.Fatalf("expected 1 file, got %d", len(r.File))
+ }
+ f := r.File[0]
+
+ extra := f.Extra
+ found := false
+ for len(extra) >= 4 {
+ tag := uint16(extra[0]) | uint16(extra[1])<<8
+ size := uint16(extra[2]) | uint16(extra[3])<<8
+ if tag == zip64ExtraID {
+ found = true
+ if size != 8 {
+ t.Errorf("expected 8 bytes in Zip64 extra field for large offset only, got %d", size)
+ }
+ break
+ }
+ extra = extra[4+size:]
+ }
+ if !found {
+ t.Error("Zip64 extra field not found")
+ }
+
+ // Also verify it's readable.
+ rc, err := f.Open()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer rc.Close()
+ content, err := io.ReadAll(rc)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(content) != "hello" {
+ t.Errorf("expected 'hello', got %q", string(content))
+ }
+ })
+
+ // Test case 2: Large size, small offset.
+ // Extra field should contain only the sizes (16 bytes).
+ t.Run("LargeSizeSmallOffset", func(t *testing.T) {
+ var buf bytes.Buffer
+ zw := NewWriter(&buf)
+
+ fh := &FileHeader{
+ Name: "large.txt",
+ UncompressedSize64: uint64(uint32max) + 1,
+ CompressedSize64: uint64(uint32max) + 1,
+ Method: Store,
+ }
+ _, err := zw.CreateRaw(fh)
+ if err != nil {
+ t.Fatal(err)
+ }
+ zw.Close()
+
+ r, err := NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
+ if err != nil {
+ t.Fatal(err)
+ }
+ f := r.File[0]
+ extra := f.Extra
+ found := false
+ for len(extra) >= 4 {
+ tag := uint16(extra[0]) | uint16(extra[1])<<8
+ size := uint16(extra[2]) | uint16(extra[3])<<8
+ if tag == zip64ExtraID {
+ found = true
+ if size != 16 {
+ t.Errorf("expected 16 bytes in Zip64 extra field for large sizes only, got %d", size)
+ }
+ break
+ }
+ extra = extra[4+size:]
+ }
+ if !found {
+ t.Error("Zip64 extra field not found")
+ }
+ })
+}
+
+type mockReaderAt struct {
+ data []byte
+ offset int64
+}
+
+func (r *mockReaderAt) ReadAt(p []byte, off int64) (int, error) {
+ if off < r.offset {
+ n := int64(len(p))
+ if off+n > r.offset {
+ n = r.offset - off
+ }
+ // just return zeros for the prefix
+ for i := int64(0); i < n; i++ {
+ p[i] = 0
+ }
+ if n < int64(len(p)) {
+ nn, err := r.ReadAt(p[n:], r.offset)
+ return int(n) + nn, err
+ }
+ return int(n), nil
+ }
+ off -= r.offset
+ if off >= int64(len(r.data)) {
+ return 0, io.EOF
+ }
+ n := copy(p, r.data[off:])
+ if n < len(p) {
+ return n, io.EOF
+ }
+ return n, nil
+}
diff --git a/src/archive/zip/writer.go b/src/archive/zip/writer.go
index 0a31005..c4429df 100644
--- a/src/archive/zip/writer.go
+++ b/src/archive/zip/writer.go
@@ -90,6 +90,15 @@
// write central directory
start := w.cw.count
for _, h := range w.dir {
+ needU := h.UncompressedSize64 >= uint32max
+ needC := h.CompressedSize64 >= uint32max
+ needO := h.offset >= uint32max
+ if needU || needC || needO {
+ if h.ReaderVersion < zipVersion45 {
+ h.ReaderVersion = zipVersion45
+ }
+ }
+
var buf [directoryHeaderLen]byte
b := writeBuf(buf[:])
b.uint32(uint32(directoryHeaderSignature))
@@ -100,25 +109,43 @@
b.uint16(h.ModifiedTime)
b.uint16(h.ModifiedDate)
b.uint32(h.CRC32)
- if h.isZip64() || h.offset >= uint32max {
- // the file needs a zip64 header. store maxint in both
- // 32 bit size fields (and offset later) to signal that the
- // zip64 extra header should be used.
- b.uint32(uint32max) // compressed size
- b.uint32(uint32max) // uncompressed size
+ if needC {
+ b.uint32(uint32max)
+ } else {
+ b.uint32(uint32(h.CompressedSize64))
+ }
+ if needU {
+ b.uint32(uint32max)
+ } else {
+ b.uint32(uint32(h.UncompressedSize64))
+ }
+ if needU || needC || needO {
// append a zip64 extra block to Extra
var buf [28]byte // 2x uint16 + 3x uint64
eb := writeBuf(buf[:])
eb.uint16(zip64ExtraID)
- eb.uint16(24) // size = 3x uint64
- eb.uint64(h.UncompressedSize64)
- eb.uint64(h.CompressedSize64)
- eb.uint64(h.offset)
- h.Extra = append(h.Extra, buf[:]...)
- } else {
- b.uint32(h.CompressedSize)
- b.uint32(h.UncompressedSize)
+ var size uint16
+ if needU {
+ size += 8
+ }
+ if needC {
+ size += 8
+ }
+ if needO {
+ size += 8
+ }
+ eb.uint16(size)
+ if needU {
+ eb.uint64(h.UncompressedSize64)
+ }
+ if needC {
+ eb.uint64(h.CompressedSize64)
+ }
+ if needO {
+ eb.uint64(h.offset)
+ }
+ h.Extra = append(h.Extra, buf[:4+size]...)
}
b.uint16(uint16(len(h.Name)))
@@ -126,7 +153,7 @@
b.uint16(uint16(len(h.Comment)))
b = b[4:] // skip disk number start and internal file attr (2x uint16)
b.uint32(h.ExternalAttrs)
- if h.offset > uint32max {
+ if needO {
b.uint32(uint32max)
} else {
b.uint32(uint32(h.offset))
Change information
Files:
- A src/archive/zip/issue33116_test.go
- M src/archive/zip/writer.go
Change size: M
Delta: 2 files changed, 196 insertions(+), 15 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Liao (Gerrit)
5:43 PM (6 hours ago) 5:43 PM
to Cyber Hamza, goph...@pubsubhelper.golang.org, Joseph Tsai, Brad Fitzpatrick, Gopher Robot, golang-co...@googlegroups.com
Sean Liao abandoned this change.
Abandoned
issue fixed with https://go-review.googlesource.com/c/go/+/725161
Sean Liao abandoned this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages