From 0f74f980bb19749a038fab81c96c4b443c2580a9 Mon Sep 17 00:00:00 2001 From: Yash Kakade Date: Sat, 31 Oct 2026 16:05:11 -0400 Subject: [PATCH 1/5] Add random-access ZIP and conda constructors --- README.md | 33 ++++++ browse.go | 9 ++ random_access.go | 175 +++++++++++++++++++++++++++ random_access_test.go | 269 ++++++++++++++++++++++++++++++++++++++++++ stream_zip.go | 13 +- 5 files changed, 493 insertions(+), 6 deletions(-) create mode 100644 random_access.go create mode 100644 random_access_test.go diff --git a/README.md b/README.md index a8b89ad..ce7d540 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,39 @@ buffers that input within `MaxInputBytes`; `OpenStreamBytes(name, data, options) reuses an existing byte slice without copying it. Keep that slice unchanged until `Close`. TAR and gem can consume a forward-only input directly. +For an existing file or another `io.ReaderAt`, use +`OpenStreamReaderAt(name, input, size, options)` or +`OpenBrowserReaderAt(name, input, size, browseOptions)` to avoid copying ZIP +and conda input. Each has a `Context` variant. The size is the complete compressed +archive size and must fit `MaxInputBytes`. Keep the source open and unchanged +until browsing and all extracted files finish. Closing a stream or extracted +file does not close the source. + +```go +file, err := os.Open("package.zip") +if err != nil { + return err +} +defer file.Close() +info, err := file.Stat() +if err != nil { + return err +} +browser, err := archives.OpenBrowserReaderAt("package.zip", file, info.Size(), archives.BrowseOptions{}) +if err != nil { + return err +} +files, err := browser.List() +``` + +The random-access APIs check the central directory before allocating ZIP entry +metadata. They skip unread ZIP bodies without decoding or checking their CRCs. +Declared sizes of skipped entries still count against expansion limits. +Read a member through EOF to check its CRC. These APIs return `Stream` or +`Browser`, not the `Reader` interface. For whole-artifact hashing, hash the +original source separately. The existing forward-only and byte-slice APIs keep +their behavior. + When content detection finds gzip, bzip2, xz or zstd, the decompressed prefix determines whether to read TAR entries or expose one member named `content`. The single member has `Size: -1` in streams and browser listings because its size diff --git a/browse.go b/browse.go index a58dcd8..d174d9a 100644 --- a/browse.go +++ b/browse.go @@ -32,6 +32,8 @@ type Browser struct { reopen func() (io.ReadCloser, error) first io.Reader data []byte + random io.ReaderAt + size int64 } // OpenBrowser prepares an archive for browsing. The caller owns content and @@ -75,6 +77,13 @@ func (b *Browser) openStream() (*Stream, func() error, error) { if err := b.ctx.Err(); err != nil { return nil, nil, err } + if b.random != nil { + stream, err := OpenStreamReaderAtContext(b.ctx, b.filename, b.random, b.size, b.options.Limits) + if err != nil { + return nil, nil, err + } + return stream, stream.Close, nil + } if b.data != nil { stream, err := OpenStreamBytesContext(b.ctx, b.filename, b.data, b.options.Limits) if err != nil { diff --git a/random_access.go b/random_access.go new file mode 100644 index 0000000..9c2656a --- /dev/null +++ b/random_access.go @@ -0,0 +1,175 @@ +package archives + +import ( + "archive/zip" + "context" + "encoding/binary" + "errors" + "fmt" + "io" +) + +// OpenStreamReaderAt opens ZIP or conda input without buffering the archive. +// size is the complete compressed input size. The caller owns content and must +// keep it open and unchanged until the stream is closed. Unread ZIP entries are +// skipped without decoding; their declared sizes still count against limits. +func OpenStreamReaderAt(filename string, content io.ReaderAt, size int64, options StreamOptions) (*Stream, error) { + return OpenStreamReaderAtContext(context.Background(), filename, content, size, options) +} + +// OpenStreamReaderAtContext is OpenStreamReaderAt with cancellation before and +// after each source read. Cancellation does not close or interrupt the input. +func OpenStreamReaderAtContext(ctx context.Context, filename string, content io.ReaderAt, size int64, options StreamOptions) (*Stream, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + options, err := options.defaults() + if err != nil { + return nil, err + } + if size < 0 { + return nil, fmt.Errorf("negative archive size: %d", size) + } + if size > options.MaxInputBytes { + return nil, ErrInputLimit + } + input := contextReaderAt{ctx: ctx, reader: content} + format, _, err := streamFormat(filename, io.NewSectionReader(input, 0, size)) + if err != nil { + return nil, err + } + if format != formatZIP && format != formatConda { + return nil, fmt.Errorf("%w: random-access archive format: %s", errors.ErrUnsupported, filename) + } + if err := checkZipReaderAt(input, size, options.MaxEntries); err != nil { + return nil, err + } + reader, err := zip.NewReader(input, size) + if err != nil { + return nil, fmt.Errorf("opening zip: %w", err) + } + if len(reader.File) > options.MaxEntries { + return nil, ErrEntryLimit + } + reader.RegisterDecompressor(zip.Deflate, newFlateReader) + var source streamSource = &zipStream{ctx: ctx, reader: reader, skipUnread: true} + if format == formatConda { + source = &condaStream{outer: &Stream{ctx: ctx, source: source, options: containerOptions(options)}} + } + return &Stream{ctx: ctx, source: source, options: options}, nil +} + +// OpenBrowserReaderAt prepares ZIP or conda input for repeated browsing without +// copying it. The caller owns content and must keep it open and unchanged until +// browsing and all extracted files finish. Limits apply to each pass. This API +// does not implement Reader or whole-artifact hashing; hash the source separately. +func OpenBrowserReaderAt(filename string, content io.ReaderAt, size int64, options BrowseOptions) (*Browser, error) { + return OpenBrowserReaderAtContext(context.Background(), filename, content, size, options) +} + +// OpenBrowserReaderAtContext is OpenBrowserReaderAt with cancellation across +// every pass, including reads from extracted files. +func OpenBrowserReaderAtContext(ctx context.Context, filename string, content io.ReaderAt, size int64, options BrowseOptions) (*Browser, error) { + stream, err := OpenStreamReaderAtContext(ctx, filename, content, size, options.Limits) + if err != nil { + return nil, err + } + if err := stream.Close(); err != nil { + return nil, err + } + return &Browser{ctx: ctx, filename: filename, options: options, random: content, size: size}, nil +} + +// Check both the advertised count and the actual headers before archive/zip +// allocates its file slice. Read only the end records and central headers. +func checkZipReaderAt(input io.ReaderAt, size int64, limit int) error { + tailSize := min(size, int64(zipDirectoryEndSearchWindow)) + tail := make([]byte, int(tailSize)) + if _, err := input.ReadAt(tail, size-tailSize); err != nil { + return err + } + for offset := len(tail) - zipDirectoryEndLen; offset >= 0; offset-- { + if binary.LittleEndian.Uint32(tail[offset:]) != zipDirectoryEndSignature { + continue + } + end := tail[offset:] + if offset+zipDirectoryEndLen+int(binary.LittleEndian.Uint16(end[20:])) > len(tail) { + continue + } + directoryEnd := size - tailSize + int64(offset) + count := uint64(binary.LittleEndian.Uint16(end[10:])) + length := uint64(binary.LittleEndian.Uint32(end[12:])) + start := uint64(binary.LittleEndian.Uint32(end[16:])) + if count == zipDirectoryRecords64Marker || length == zipDirectorySizeOffsetMarker || start == zipDirectorySizeOffsetMarker { + var locator [zipDirectory64LocatorLen]byte + if directoryEnd < int64(len(locator)) { + return zip.ErrFormat + } + if _, err := input.ReadAt(locator[:], directoryEnd-int64(len(locator))); err != nil { + return err + } + if binary.LittleEndian.Uint32(locator[:]) != zipDirectory64LocSignature { + return zip.ErrFormat + } + recordOffset := binary.LittleEndian.Uint64(locator[8:]) + var record [zipDirectory64EndLen]byte + if size < int64(len(record)) || recordOffset > uint64(size-int64(len(record))) { + return zip.ErrFormat + } + if _, err := input.ReadAt(record[:], int64(recordOffset)); err != nil { + return err + } + if binary.LittleEndian.Uint32(record[:]) != zipDirectory64EndSignature { + return zip.ErrFormat + } + directoryEnd = int64(recordOffset) + count = binary.LittleEndian.Uint64(record[32:]) + length = binary.LittleEndian.Uint64(record[40:]) + start = binary.LittleEndian.Uint64(record[48:]) + } + if count > uint64(limit) { + return ErrEntryLimit + } + if length > uint64(directoryEnd) || start > uint64(directoryEnd)-length { + return zip.ErrFormat + } + // ZIPs can have an executable prefix whose length is absent from offsets. + actualStart := directoryEnd - int64(length) + if start < uint64(actualStart) { + var signature [4]byte + if _, err := input.ReadAt(signature[:], int64(start)); err != nil { + return err + } + if binary.LittleEndian.Uint32(signature[:]) == zipDirectoryHeaderSignature { + actualStart = int64(start) + } + } + actualCount := 0 + for pos := actualStart; pos < directoryEnd; { + var header [zipDirectoryHeaderLen]byte + if directoryEnd-pos < int64(len(header)) { + return zip.ErrFormat + } + if _, err := input.ReadAt(header[:], pos); err != nil { + return err + } + if binary.LittleEndian.Uint32(header[:]) != zipDirectoryHeaderSignature { + return zip.ErrFormat + } + recordLen := int64(zipDirectoryHeaderLen) + int64(binary.LittleEndian.Uint16(header[28:])) + int64(binary.LittleEndian.Uint16(header[30:])) + int64(binary.LittleEndian.Uint16(header[32:])) + if recordLen > directoryEnd-pos { + return zip.ErrFormat + } + actualCount++ + if actualCount > limit { + return ErrEntryLimit + } + pos += recordLen + } + if uint64(actualCount) != count { + return zip.ErrFormat + } + return nil + } + return zip.ErrFormat +} diff --git a/random_access_test.go b/random_access_test.go new file mode 100644 index 0000000..e39f4a8 --- /dev/null +++ b/random_access_test.go @@ -0,0 +1,269 @@ +package archives + +import ( + "archive/zip" + "bytes" + "context" + "encoding/binary" + "errors" + "io" + "os" + "path/filepath" + "testing" +) + +func randomAccessZIP(t testing.TB, payloadSize int) []byte { + t.Helper() + var buf bytes.Buffer + writer := zip.NewWriter(&buf) + for _, entry := range []struct { + name string + data []byte + }{{"root/large", make([]byte, payloadSize)}, {"root/small", []byte("hello")}} { + out, err := writer.CreateHeader(&zip.FileHeader{Name: entry.name, Method: zip.Store}) + if err != nil { + t.Fatal(err) + } + if _, err := out.Write(entry.data); err != nil { + t.Fatal(err) + } + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +type countedReaderAt struct { + *bytes.Reader + reads, calls, total int +} + +func (r *countedReaderAt) Read(p []byte) (int, error) { r.reads++; return r.Reader.Read(p) } +func (r *countedReaderAt) ReadAt(p []byte, off int64) (int, error) { + r.calls++ + n, err := r.Reader.ReadAt(p, off) + r.total += n + return n, err +} + +func TestBrowserReaderAt(t *testing.T) { + data := randomAccessZIP(t, 2<<20) + input := &countedReaderAt{Reader: bytes.NewReader(data)} + browser, err := OpenBrowserReaderAt("test.zip", input, int64(len(data)), BrowseOptions{DetectRoot: true}) + if err != nil { + t.Fatal(err) + } + for range 2 { + files, err := browser.List() + if err != nil || len(files) != 2 || files[0].Path != "large" { + t.Fatalf("List = %+v, %v", files, err) + } + assertBrowsedBody(t, browser, "small", "hello") + } + if input.reads != 0 || input.calls == 0 || input.total >= len(data)/2 { + t.Fatalf("Read=%d ReadAt=%d bytes=%d archive=%d", input.reads, input.calls, input.total, len(data)) + } +} + +func TestBrowserReaderAtFile(t *testing.T) { + data := randomAccessZIP(t, 128) + name := filepath.Join(t.TempDir(), "test.zip") + if err := os.WriteFile(name, data, 0o600); err != nil { + t.Fatal(err) + } + input, err := os.Open(name) + if err != nil { + t.Fatal(err) + } + defer func() { _ = input.Close() }() + browser, err := OpenBrowserReaderAt("test.zip", input, int64(len(data)), BrowseOptions{}) + if err != nil { + t.Fatal(err) + } + assertBrowsedBody(t, browser, "root/small", "hello") + if _, err := input.Stat(); err != nil { + t.Fatalf("caller input closed: %v", err) + } +} + +func TestStreamReaderAtFormats(t *testing.T) { + for _, name := range []string{"test.zip", "test.conda", "renamed"} { + t.Run(name, func(t *testing.T) { + format := name + if name == "renamed" { + format = "test.zip" + } + data := streamTestArchive(t, format, browseTar(t, "root/a", "root/b")) + stream, err := OpenStreamReaderAt(name, bytes.NewReader(data), int64(len(data)), StreamOptions{}) + if err != nil { + t.Fatal(err) + } + defer func() { _ = stream.Close() }() + count := 0 + for { + _, err := stream.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatal(err) + } + if _, err := io.Copy(io.Discard, stream); err != nil { + t.Fatal(err) + } + count++ + } + if count != 2 { + t.Fatalf("entries = %d", count) + } + }) + } +} + +func TestReaderAtLimitsAndCancellation(t *testing.T) { + data := randomAccessZIP(t, 128) + for _, tc := range []struct { + name string + options StreamOptions + want error + }{ + {"input", StreamOptions{MaxInputBytes: int64(len(data) - 1)}, ErrInputLimit}, + {"entries", StreamOptions{MaxEntries: 1}, ErrEntryLimit}, + {"entry bytes", StreamOptions{MaxEntryBytes: 127}, ErrEntrySizeLimit}, + {"expanded", StreamOptions{MaxExpandedBytes: 128}, ErrDecompressLimit}, + } { + t.Run(tc.name, func(t *testing.T) { + b, err := OpenBrowserReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), BrowseOptions{Limits: tc.options}) + if err == nil { + _, err = b.List() + } + if !errors.Is(err, tc.want) { + t.Fatalf("error = %v, want %v", err, tc.want) + } + }) + } + ctx, cancel := context.WithCancel(context.Background()) + b, err := OpenBrowserReaderAtContext(ctx, "test.zip", bytes.NewReader(data), int64(len(data)), BrowseOptions{}) + if err != nil { + t.Fatal(err) + } + body, err := b.Extract("root/small") + if err != nil { + t.Fatal(err) + } + defer func() { _ = body.Close() }() + cancel() + if _, err = body.Read(make([]byte, 1)); !errors.Is(err, context.Canceled) { + t.Fatalf("read error = %v", err) + } + if _, err = b.List(); !errors.Is(err, context.Canceled) { + t.Fatalf("List error = %v", err) + } + if _, err = OpenStreamReaderAtContext(ctx, "test.zip", bytes.NewReader(data), int64(len(data)), StreamOptions{}); !errors.Is(err, context.Canceled) { + t.Fatalf("open error = %v", err) + } +} + +func TestReaderAtMalformedDirectory(t *testing.T) { + valid := randomAccessZIP(t, 16) + for _, tc := range []struct { + name string + change func([]byte) + want error + }{ + {"underreported count", func(data []byte) { binary.LittleEndian.PutUint16(data[len(data)-12:], 1) }, zip.ErrFormat}, + {"advertised count", func(data []byte) { binary.LittleEndian.PutUint16(data[len(data)-12:], 40000) }, ErrEntryLimit}, + {"header length", func(data []byte) { + start, _, _ := zipCentralDirectoryBounds(data) + binary.LittleEndian.PutUint16(data[start+28:], 65535) + }, zip.ErrFormat}, + {"signature", func(data []byte) { start, _, _ := zipCentralDirectoryBounds(data); data[start] = 0 }, zip.ErrFormat}, + } { + t.Run(tc.name, func(t *testing.T) { + data := bytes.Clone(valid) + tc.change(data) + if _, err := OpenStreamReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), StreamOptions{MaxEntries: 2}); !errors.Is(err, tc.want) { + t.Fatalf("error = %v, want %v", err, tc.want) + } + }) + } + for _, size := range []int64{-1, 0, 12} { + if _, err := OpenStreamReaderAt("test.zip", bytes.NewReader(valid), size, StreamOptions{}); err == nil { + t.Fatalf("accepted size %d", size) + } + } + if _, err := OpenStreamReaderAt("test.zip", bytes.NewReader(valid), int64(len(valid)), StreamOptions{MaxEntries: -1}); err == nil { + t.Fatal("accepted negative limit") + } + if _, err := OpenStreamReaderAt("test.tar", bytes.NewReader(valid), int64(len(valid)), StreamOptions{}); !errors.Is(err, errors.ErrUnsupported) { + t.Fatalf("format error = %v", err) + } +} + +func BenchmarkBrowserReaderAt(b *testing.B) { + data := randomAccessZIP(b, 16<<20) + for _, random := range []bool{false, true} { + name := "buffered" + if random { + name = "reader-at" + } + b.Run(name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + var browser *Browser + var err error + if random { + browser, err = OpenBrowserReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), BrowseOptions{}) + } else { + browser, err = OpenBrowser("test.zip", bytes.NewReader(data), nil, BrowseOptions{}) + } + if err != nil { + b.Fatal(err) + } + if _, err = browser.List(); err != nil { + b.Fatal(err) + } + file, err := browser.Extract("root/small") + if err != nil { + b.Fatal(err) + } + if _, err = io.Copy(io.Discard, file); err != nil { + b.Fatal(err) + } + if err = file.Close(); err != nil { + b.Fatal(err) + } + } + }) + } +} + +func TestReaderAtZIP64AndPrefix(t *testing.T) { + original := randomAccessZIP(t, 32) + for _, prefix := range []bool{false, true} { + data := bytes.Clone(original) + if prefix { + data = append([]byte("executable prefix"), data...) + } else { + end := len(data) - zipDirectoryEndLen + record := make([]byte, zipDirectory64EndLen+zipDirectory64LocatorLen) + binary.LittleEndian.PutUint32(record, zipDirectory64EndSignature) + binary.LittleEndian.PutUint64(record[4:], 44) + binary.LittleEndian.PutUint64(record[24:], 2) + binary.LittleEndian.PutUint64(record[32:], 2) + binary.LittleEndian.PutUint64(record[40:], uint64(binary.LittleEndian.Uint32(data[end+12:]))) + binary.LittleEndian.PutUint64(record[48:], uint64(binary.LittleEndian.Uint32(data[end+16:]))) + binary.LittleEndian.PutUint32(record[56:], zipDirectory64LocSignature) + binary.LittleEndian.PutUint64(record[64:], uint64(end)) + binary.LittleEndian.PutUint32(record[72:], 1) + data = append(append(data[:end:end], record...), data[end:]...) + binary.LittleEndian.PutUint16(data[len(data)-12:], zipDirectoryRecords64Marker) + } + b, err := OpenBrowserReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), BrowseOptions{}) + if err != nil { + t.Fatal(err) + } + assertBrowsedBody(t, b, "root/small", "hello") + } +} diff --git a/stream_zip.go b/stream_zip.go index 126f6f2..0972866 100644 --- a/stream_zip.go +++ b/stream_zip.go @@ -10,11 +10,12 @@ import ( ) type zipStream struct { - ctx context.Context - reader *zip.Reader - index int - file *zip.File - current io.ReadCloser + ctx context.Context + reader *zip.Reader + index int + file *zip.File + current io.ReadCloser + skipUnread bool } //nolint:ireturn // archive formats share a sequential source @@ -38,7 +39,7 @@ func newZipStream(ctx context.Context, raw []byte, options StreamOptions, conda } func (z *zipStream) Next() (*StreamEntry, error) { - if z.file != nil { + if z.file != nil && (!z.skipUnread || z.current != nil) { if _, err := io.Copy(io.Discard, z); err != nil { return nil, err } From 5ba6b238b14dd6b2a2a0b077cf563bd277603961 Mon Sep 17 00:00:00 2001 From: Yash Kakade Date: Mon, 2 Nov 2026 19:47:51 -0500 Subject: [PATCH 2/5] Share one ZIP central directory parser The random-access path had its own copy of the end record and central header scan. It rejected archives that the byte-slice path opens, such as a non-ZIP64 archive whose 16-bit count has wrapped. Parse from an io.ReaderAt in one place and call it through bytes.NewReader on the byte-slice path. Read headers through a bufio.Reader so that a large directory does not cost one ReadAt per header. Add a test that runs malformed directories through both paths and compares the results. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- random_access.go | 97 +--------------------- random_access_test.go | 185 +++++++++++++++++++++++++++++++++++++----- stream_zip.go | 2 +- zip.go | 153 ++++++++++++++++++++++------------ 5 files changed, 271 insertions(+), 170 deletions(-) diff --git a/README.md b/README.md index ce7d540..8ed52be 100644 --- a/README.md +++ b/README.md @@ -213,8 +213,8 @@ if err != nil { files, err := browser.List() ``` -The random-access APIs check the central directory before allocating ZIP entry -metadata. They skip unread ZIP bodies without decoding or checking their CRCs. +The random-access APIs apply the same central directory checks as the byte-slice +APIs and accept the same archives. They skip unread ZIP bodies without decoding or checking their CRCs. Declared sizes of skipped entries still count against expansion limits. Read a member through EOF to check its CRC. These APIs return `Stream` or `Browser`, not the `Reader` interface. For whole-artifact hashing, hash the diff --git a/random_access.go b/random_access.go index 9c2656a..a3bce8f 100644 --- a/random_access.go +++ b/random_access.go @@ -3,7 +3,6 @@ package archives import ( "archive/zip" "context" - "encoding/binary" "errors" "fmt" "io" @@ -41,7 +40,7 @@ func OpenStreamReaderAtContext(ctx context.Context, filename string, content io. if format != formatZIP && format != formatConda { return nil, fmt.Errorf("%w: random-access archive format: %s", errors.ErrUnsupported, filename) } - if err := checkZipReaderAt(input, size, options.MaxEntries); err != nil { + if err := checkZipEntryCountLimitContext(ctx, input, size, options.MaxEntries); err != nil { return nil, err } reader, err := zip.NewReader(input, size) @@ -79,97 +78,3 @@ func OpenBrowserReaderAtContext(ctx context.Context, filename string, content io } return &Browser{ctx: ctx, filename: filename, options: options, random: content, size: size}, nil } - -// Check both the advertised count and the actual headers before archive/zip -// allocates its file slice. Read only the end records and central headers. -func checkZipReaderAt(input io.ReaderAt, size int64, limit int) error { - tailSize := min(size, int64(zipDirectoryEndSearchWindow)) - tail := make([]byte, int(tailSize)) - if _, err := input.ReadAt(tail, size-tailSize); err != nil { - return err - } - for offset := len(tail) - zipDirectoryEndLen; offset >= 0; offset-- { - if binary.LittleEndian.Uint32(tail[offset:]) != zipDirectoryEndSignature { - continue - } - end := tail[offset:] - if offset+zipDirectoryEndLen+int(binary.LittleEndian.Uint16(end[20:])) > len(tail) { - continue - } - directoryEnd := size - tailSize + int64(offset) - count := uint64(binary.LittleEndian.Uint16(end[10:])) - length := uint64(binary.LittleEndian.Uint32(end[12:])) - start := uint64(binary.LittleEndian.Uint32(end[16:])) - if count == zipDirectoryRecords64Marker || length == zipDirectorySizeOffsetMarker || start == zipDirectorySizeOffsetMarker { - var locator [zipDirectory64LocatorLen]byte - if directoryEnd < int64(len(locator)) { - return zip.ErrFormat - } - if _, err := input.ReadAt(locator[:], directoryEnd-int64(len(locator))); err != nil { - return err - } - if binary.LittleEndian.Uint32(locator[:]) != zipDirectory64LocSignature { - return zip.ErrFormat - } - recordOffset := binary.LittleEndian.Uint64(locator[8:]) - var record [zipDirectory64EndLen]byte - if size < int64(len(record)) || recordOffset > uint64(size-int64(len(record))) { - return zip.ErrFormat - } - if _, err := input.ReadAt(record[:], int64(recordOffset)); err != nil { - return err - } - if binary.LittleEndian.Uint32(record[:]) != zipDirectory64EndSignature { - return zip.ErrFormat - } - directoryEnd = int64(recordOffset) - count = binary.LittleEndian.Uint64(record[32:]) - length = binary.LittleEndian.Uint64(record[40:]) - start = binary.LittleEndian.Uint64(record[48:]) - } - if count > uint64(limit) { - return ErrEntryLimit - } - if length > uint64(directoryEnd) || start > uint64(directoryEnd)-length { - return zip.ErrFormat - } - // ZIPs can have an executable prefix whose length is absent from offsets. - actualStart := directoryEnd - int64(length) - if start < uint64(actualStart) { - var signature [4]byte - if _, err := input.ReadAt(signature[:], int64(start)); err != nil { - return err - } - if binary.LittleEndian.Uint32(signature[:]) == zipDirectoryHeaderSignature { - actualStart = int64(start) - } - } - actualCount := 0 - for pos := actualStart; pos < directoryEnd; { - var header [zipDirectoryHeaderLen]byte - if directoryEnd-pos < int64(len(header)) { - return zip.ErrFormat - } - if _, err := input.ReadAt(header[:], pos); err != nil { - return err - } - if binary.LittleEndian.Uint32(header[:]) != zipDirectoryHeaderSignature { - return zip.ErrFormat - } - recordLen := int64(zipDirectoryHeaderLen) + int64(binary.LittleEndian.Uint16(header[28:])) + int64(binary.LittleEndian.Uint16(header[30:])) + int64(binary.LittleEndian.Uint16(header[32:])) - if recordLen > directoryEnd-pos { - return zip.ErrFormat - } - actualCount++ - if actualCount > limit { - return ErrEntryLimit - } - pos += recordLen - } - if uint64(actualCount) != count { - return zip.ErrFormat - } - return nil - } - return zip.ErrFormat -} diff --git a/random_access_test.go b/random_access_test.go index e39f4a8..2e1860b 100644 --- a/random_access_test.go +++ b/random_access_test.go @@ -4,11 +4,15 @@ import ( "archive/zip" "bytes" "context" + "crypto/sha256" "encoding/binary" "errors" + "fmt" "io" "os" "path/filepath" + "strconv" + "strings" "testing" ) @@ -167,27 +171,6 @@ func TestReaderAtLimitsAndCancellation(t *testing.T) { func TestReaderAtMalformedDirectory(t *testing.T) { valid := randomAccessZIP(t, 16) - for _, tc := range []struct { - name string - change func([]byte) - want error - }{ - {"underreported count", func(data []byte) { binary.LittleEndian.PutUint16(data[len(data)-12:], 1) }, zip.ErrFormat}, - {"advertised count", func(data []byte) { binary.LittleEndian.PutUint16(data[len(data)-12:], 40000) }, ErrEntryLimit}, - {"header length", func(data []byte) { - start, _, _ := zipCentralDirectoryBounds(data) - binary.LittleEndian.PutUint16(data[start+28:], 65535) - }, zip.ErrFormat}, - {"signature", func(data []byte) { start, _, _ := zipCentralDirectoryBounds(data); data[start] = 0 }, zip.ErrFormat}, - } { - t.Run(tc.name, func(t *testing.T) { - data := bytes.Clone(valid) - tc.change(data) - if _, err := OpenStreamReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), StreamOptions{MaxEntries: 2}); !errors.Is(err, tc.want) { - t.Fatalf("error = %v, want %v", err, tc.want) - } - }) - } for _, size := range []int64{-1, 0, 12} { if _, err := OpenStreamReaderAt("test.zip", bytes.NewReader(valid), size, StreamOptions{}); err == nil { t.Fatalf("accepted size %d", size) @@ -267,3 +250,163 @@ func TestReaderAtZIP64AndPrefix(t *testing.T) { assertBrowsedBody(t, b, "root/small", "hello") } } + +// zipDirectoryStartForTest returns the central directory offset of data. +func zipDirectoryStartForTest(t *testing.T, data []byte) int { + t.Helper() + start, _, ok, err := zipCentralDirectoryBounds(bytes.NewReader(data), int64(len(data))) + if err != nil || !ok { + t.Fatalf("directory bounds: ok=%v err=%v", ok, err) + } + return int(start) +} + +// wrappedCountZIP has a classic end record whose 16-bit count has wrapped. +func wrappedCountZIP(t *testing.T, entries int) []byte { + t.Helper() + var buf bytes.Buffer + writer := zip.NewWriter(&buf) + for i := range entries { + if _, err := writer.CreateHeader(&zip.FileHeader{Name: strconv.Itoa(i), Method: zip.Store}); err != nil { + t.Fatal(err) + } + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + data := buf.Bytes() + start, end, ok, err := zipCentralDirectoryBounds(bytes.NewReader(data), int64(len(data))) + if err != nil || !ok { + t.Fatalf("directory bounds: ok=%v err=%v", ok, err) + } + record := make([]byte, zipDirectoryEndLen) + binary.LittleEndian.PutUint32(record, zipDirectoryEndSignature) + binary.LittleEndian.PutUint16(record[8:], uint16(entries)) + binary.LittleEndian.PutUint16(record[10:], uint16(entries)) + binary.LittleEndian.PutUint32(record[12:], uint32(end-start)) + binary.LittleEndian.PutUint32(record[16:], uint32(start)) + return append(data[:end:end], record...) +} + +func streamContents(stream *Stream, openErr error) (string, error) { + if openErr != nil { + return "", openErr + } + defer func() { _ = stream.Close() }() + var out strings.Builder + for { + entry, err := stream.Next() + if err == io.EOF { + return out.String(), nil + } + if err != nil { + return out.String(), err + } + body, err := io.ReadAll(stream) + if err != nil { + return out.String(), err + } + fmt.Fprintf(&out, "%s=%x\n", entry.Path, sha256.Sum256(body)) + } +} + +func TestZIPParserParity(t *testing.T) { + valid := randomAccessZIP(t, 16) + edit := func(change func([]byte)) func(*testing.T) []byte { + return func(t *testing.T) []byte { + t.Helper() + data := bytes.Clone(valid) + change(data) + return data + } + } + endCount := func(count uint16) func(*testing.T) []byte { + return edit(func(data []byte) { binary.LittleEndian.PutUint16(data[len(data)-12:], count) }) + } + for _, tc := range []struct { + name string + build func(*testing.T) []byte + maxEntries int + wantErr error + wantOpen bool + }{ + {name: "valid", build: edit(func([]byte) {}), wantOpen: true}, + {name: "wrapped count", build: func(t *testing.T) []byte { t.Helper(); return wrappedCountZIP(t, 70_000) }, wantOpen: true}, + {name: "wrapped count over limit", build: func(t *testing.T) []byte { t.Helper(); return wrappedCountZIP(t, 70_000) }, maxEntries: 69_999, wantErr: ErrEntryLimit}, + {name: "headers over limit", build: edit(func([]byte) {}), maxEntries: 1, wantErr: ErrEntryLimit}, + {name: "underreported count", build: endCount(1), maxEntries: 2}, + {name: "advertised count over limit", build: endCount(40000), maxEntries: 2}, + {name: "header length", build: func(t *testing.T) []byte { + t.Helper() + data := bytes.Clone(valid) + binary.LittleEndian.PutUint16(data[zipDirectoryStartForTest(t, data)+28:], 65535) + return data + }}, + {name: "header signature", build: func(t *testing.T) []byte { + t.Helper() + data := bytes.Clone(valid) + data[zipDirectoryStartForTest(t, data)] = 0 + return data + }}, + {name: "second header signature", build: func(t *testing.T) []byte { + t.Helper() + data := bytes.Clone(valid) + start := zipDirectoryStartForTest(t, data) + first := zipDirectoryHeaderLen + int(binary.LittleEndian.Uint16(data[start+28:])) + data[start+first] = 0 + return data + }, maxEntries: 1}, + {name: "no end record", build: edit(func(data []byte) { data[len(data)-zipDirectoryEndLen] = 0 })}, + {name: "missing zip64 locator", build: endCount(zipDirectoryRecords64Marker)}, + {name: "directory size past start", build: edit(func(data []byte) { + binary.LittleEndian.PutUint32(data[len(data)-10:], uint32(len(data))) + })}, + {name: "truncated", build: func(*testing.T) []byte { return valid[:12] }}, + {name: "empty", build: func(*testing.T) []byte { return nil }}, + {name: "executable prefix", build: func(*testing.T) []byte { return append([]byte("executable prefix"), valid...) }, wantOpen: true}, + } { + t.Run(tc.name, func(t *testing.T) { + data := tc.build(t) + options := StreamOptions{MaxEntries: tc.maxEntries} + bytesStream, bytesOpenErr := OpenStreamBytes("test.zip", data, options) + bytesOut, bytesErr := streamContents(bytesStream, bytesOpenErr) + randomStream, randomOpenErr := OpenStreamReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), options) + randomOut, randomErr := streamContents(randomStream, randomOpenErr) + + if (bytesOpenErr == nil) != (randomOpenErr == nil) { + t.Fatalf("open errors differ: bytes=%v reader-at=%v", bytesOpenErr, randomOpenErr) + } + if fmt.Sprint(bytesErr) != fmt.Sprint(randomErr) { + t.Fatalf("errors differ: bytes=%v reader-at=%v", bytesErr, randomErr) + } + if bytesOut != randomOut { + t.Fatalf("entries differ:\nbytes:\n%s\nreader-at:\n%s", bytesOut, randomOut) + } + if tc.wantErr != nil && !errors.Is(randomErr, tc.wantErr) { + t.Fatalf("error = %v, want %v", randomErr, tc.wantErr) + } + if tc.wantOpen && randomErr != nil { + t.Fatalf("error = %v, want success", randomErr) + } + }) + } +} + +func TestZIPEntryCountBuffersDirectoryReads(t *testing.T) { + const entries = 5000 + data := wrappedCountZIP(t, entries) + input := &countedReaderAt{Reader: bytes.NewReader(data)} + if err := checkZipEntryCountLimitContext(context.Background(), input, int64(len(data)), entries); err != nil { + t.Fatal(err) + } + if input.calls >= entries/10 { + t.Fatalf("ReadAt calls = %d for %d entries", input.calls, entries) + } +} + +func TestReaderAtSourceShorterThanSize(t *testing.T) { + data := randomAccessZIP(t, 16) + if _, err := OpenStreamReaderAt("test.zip", bytes.NewReader(data), int64(len(data))+1, StreamOptions{}); !errors.Is(err, io.ErrUnexpectedEOF) { + t.Fatalf("error = %v, want %v", err, io.ErrUnexpectedEOF) + } +} diff --git a/stream_zip.go b/stream_zip.go index 0972866..95f7e06 100644 --- a/stream_zip.go +++ b/stream_zip.go @@ -20,7 +20,7 @@ type zipStream struct { //nolint:ireturn // archive formats share a sequential source func newZipStream(ctx context.Context, raw []byte, options StreamOptions, conda bool) (streamSource, error) { - if err := checkZipEntryCountLimitContext(ctx, raw, options.MaxEntries); err != nil { + if err := checkZipEntryCountLimitContext(ctx, bytes.NewReader(raw), int64(len(raw)), options.MaxEntries); err != nil { return nil, err } reader, err := zip.NewReader(contextReaderAt{ctx: ctx, reader: bytes.NewReader(raw)}, int64(len(raw))) diff --git a/zip.go b/zip.go index ad2c387..590c511 100644 --- a/zip.go +++ b/zip.go @@ -73,32 +73,36 @@ func openZip(raw []byte) (*zipReader, error) { } func checkZipEntryCount(raw []byte) error { - return checkZipEntryCountLimit(raw, maxArchiveEntries) + return checkZipEntryCountLimitContext(context.Background(), bytes.NewReader(raw), int64(len(raw)), maxArchiveEntries) } -func checkZipEntryCountLimit(raw []byte, limit int) error { - return checkZipEntryCountLimitContext(context.Background(), raw, limit) -} - -func checkZipEntryCountLimitContext(ctx context.Context, raw []byte, limit int) error { - start, end, ok := zipCentralDirectoryBounds(raw) - if !ok { - return nil +// checkZipEntryCountLimitContext counts central directory headers before +// archive/zip builds its file list. Only the limit is enforced here. Malformed +// directories are left for archive/zip to accept or reject. +func checkZipEntryCountLimitContext(ctx context.Context, input io.ReaderAt, size int64, limit int) error { + start, end, ok, err := zipCentralDirectoryBounds(input, size) + if err != nil || !ok { + return err } + headers := bufio.NewReader(io.NewSectionReader(input, start, end-start)) + var header [zipDirectoryHeaderLen]byte count := 0 - for offset := start; offset+zipDirectoryHeaderLen <= end; { + for remaining := end - start; remaining >= zipDirectoryHeaderLen; { if err := ctx.Err(); err != nil { return err } - if binary.LittleEndian.Uint32(raw[offset:]) != zipDirectoryHeaderSignature { + if _, err := io.ReadFull(headers, header[:]); err != nil { + return zipSourceError(err) + } + if binary.LittleEndian.Uint32(header[:]) != zipDirectoryHeaderSignature { break } - nameLen := int(binary.LittleEndian.Uint16(raw[offset+28:])) - extraLen := int(binary.LittleEndian.Uint16(raw[offset+30:])) - commentLen := int(binary.LittleEndian.Uint16(raw[offset+32:])) - recordLen := zipDirectoryHeaderLen + nameLen + extraLen + commentLen - if recordLen > end-offset { + nameLen := int(binary.LittleEndian.Uint16(header[28:])) + extraLen := int(binary.LittleEndian.Uint16(header[30:])) + commentLen := int(binary.LittleEndian.Uint16(header[32:])) + recordLen := int64(zipDirectoryHeaderLen + nameLen + extraLen + commentLen) + if recordLen > remaining { return nil } @@ -106,63 +110,112 @@ func checkZipEntryCountLimitContext(ctx context.Context, raw []byte, limit int) if count > limit { return fmt.Errorf("%w: count %d exceeds %d", ErrEntryLimit, count, limit) } - offset += recordLen + if _, err := headers.Discard(nameLen + extraLen + commentLen); err != nil { + return zipSourceError(err) + } + remaining -= recordLen } return nil } -func zipCentralDirectoryBounds(raw []byte) (int, int, bool) { - searchStart := len(raw) - zipDirectoryEndSearchWindow - if searchStart < 0 { - searchStart = 0 +func zipCentralDirectoryBounds(input io.ReaderAt, size int64) (int64, int64, bool, error) { + tailStart := max(size-zipDirectoryEndSearchWindow, 0) + tail := make([]byte, size-tailStart) + if err := readZipSource(input, tail, tailStart); err != nil { + return 0, 0, false, err } - for offset := len(raw) - zipDirectoryEndLen; offset >= searchStart; offset-- { - if binary.LittleEndian.Uint32(raw[offset:]) != zipDirectoryEndSignature { + for offset := len(tail) - zipDirectoryEndLen; offset >= 0; offset-- { + if binary.LittleEndian.Uint32(tail[offset:]) != zipDirectoryEndSignature { continue } - commentLen := int(binary.LittleEndian.Uint16(raw[offset+20:])) - if offset+zipDirectoryEndLen+commentLen > len(raw) { + commentLen := int(binary.LittleEndian.Uint16(tail[offset+20:])) + if offset+zipDirectoryEndLen+commentLen > len(tail) { continue } - directoryEnd := offset - directoryRecords := binary.LittleEndian.Uint16(raw[offset+10:]) - directorySize := uint64(binary.LittleEndian.Uint32(raw[offset+12:])) - directoryOffset := uint64(binary.LittleEndian.Uint32(raw[offset+16:])) + recordOffset := tailStart + int64(offset) + directoryEnd := recordOffset + directoryRecords := binary.LittleEndian.Uint16(tail[offset+10:]) + directorySize := uint64(binary.LittleEndian.Uint32(tail[offset+12:])) + directoryOffset := uint64(binary.LittleEndian.Uint32(tail[offset+16:])) if directoryRecords == zipDirectoryRecords64Marker || directorySize == zipDirectorySizeOffsetMarker || directoryOffset == zipDirectorySizeOffsetMarker { - locatorOffset := offset - zipDirectory64LocatorLen - if locatorOffset < 0 || binary.LittleEndian.Uint32(raw[locatorOffset:]) != zipDirectory64LocSignature { - return 0, 0, false - } - zip64Offset := binary.LittleEndian.Uint64(raw[locatorOffset+8:]) - if len(raw) < zipDirectory64EndLen || zip64Offset > uint64(len(raw)-zipDirectory64EndLen) { - return 0, 0, false + var ok bool + var err error + directoryEnd, directorySize, directoryOffset, ok, err = zip64DirectoryRecord(input, size, recordOffset) + if err != nil || !ok { + return 0, 0, false, err } - directoryEnd = int(zip64Offset) - if binary.LittleEndian.Uint32(raw[directoryEnd:]) != zipDirectory64EndSignature { - return 0, 0, false - } - directorySize = binary.LittleEndian.Uint64(raw[directoryEnd+40:]) - directoryOffset = binary.LittleEndian.Uint64(raw[directoryEnd+48:]) } if directorySize > uint64(directoryEnd) { - return 0, 0, false + return 0, 0, false, nil } - start := directoryEnd - int(directorySize) - if directoryOffset < uint64(directoryEnd)-directorySize && - directoryOffset <= uint64(len(raw)-zipSignatureLen) && - binary.LittleEndian.Uint32(raw[int(directoryOffset):]) == zipDirectoryHeaderSignature { - start = int(directoryOffset) + start := directoryEnd - int64(directorySize) + if directoryOffset < uint64(start) { + // ZIPs can have an executable prefix whose length is absent from offsets. + var signature [zipSignatureLen]byte + if err := readZipSource(input, signature[:], int64(directoryOffset)); err != nil { + return 0, 0, false, err + } + if binary.LittleEndian.Uint32(signature[:]) == zipDirectoryHeaderSignature { + start = int64(directoryOffset) + } } - return start, directoryEnd, true + return start, directoryEnd, true, nil + } + + return 0, 0, false, nil +} + +func zip64DirectoryRecord(input io.ReaderAt, size, endOffset int64) (int64, uint64, uint64, bool, error) { + if endOffset < zipDirectory64LocatorLen { + return 0, 0, 0, false, nil } + var locator [zipDirectory64LocatorLen]byte + if err := readZipSource(input, locator[:], endOffset-zipDirectory64LocatorLen); err != nil { + return 0, 0, 0, false, err + } + if binary.LittleEndian.Uint32(locator[:]) != zipDirectory64LocSignature { + return 0, 0, 0, false, nil + } + recordOffset := binary.LittleEndian.Uint64(locator[8:]) + if size < zipDirectory64EndLen || recordOffset > uint64(size-zipDirectory64EndLen) { + return 0, 0, 0, false, nil + } + var record [zipDirectory64EndLen]byte + if err := readZipSource(input, record[:], int64(recordOffset)); err != nil { + return 0, 0, 0, false, err + } + if binary.LittleEndian.Uint32(record[:]) != zipDirectory64EndSignature { + return 0, 0, 0, false, nil + } + directorySize := binary.LittleEndian.Uint64(record[40:]) + directoryOffset := binary.LittleEndian.Uint64(record[48:]) + return int64(recordOffset), directorySize, directoryOffset, true, nil +} + +func readZipSource(input io.ReaderAt, p []byte, offset int64) error { + n, err := input.ReadAt(p, offset) + if err == io.EOF && n == len(p) { + return nil + } + if err != nil { + return zipSourceError(err) + } + return nil +} - return 0, 0, false +// The size is trusted to bound every read, so EOF means the source is shorter +// than the caller declared. +func zipSourceError(err error) error { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + return fmt.Errorf("reading zip central directory: %w", err) } func (z *zipReader) List() ([]FileInfo, error) { From 64ba90fc7f2db114aa75a34be091c4fb501e2f3e Mon Sep 17 00:00:00 2001 From: Yash Kakade Date: Tue, 3 Nov 2026 11:26:49 -0500 Subject: [PATCH 3/5] Search a 1 KiB ZIP tail before the full window The end record is at size-22 when the archive has no comment. Match archive/zip: read the last 1 KiB first and read 65 KiB only when the signature is not in it. This brings the bytes path back below its allocation before the parsers were unified. Pin the underreported and over-limit count cases as archives that open, and add a comment case that needs the wider search. Co-Authored-By: Claude Opus 5.5 --- random_access_test.go | 46 ++++++++++++++------- zip.go | 96 +++++++++++++++++++++++++------------------ 2 files changed, 86 insertions(+), 56 deletions(-) diff --git a/random_access_test.go b/random_access_test.go index 2e1860b..32d9ed3 100644 --- a/random_access_test.go +++ b/random_access_test.go @@ -229,19 +229,7 @@ func TestReaderAtZIP64AndPrefix(t *testing.T) { if prefix { data = append([]byte("executable prefix"), data...) } else { - end := len(data) - zipDirectoryEndLen - record := make([]byte, zipDirectory64EndLen+zipDirectory64LocatorLen) - binary.LittleEndian.PutUint32(record, zipDirectory64EndSignature) - binary.LittleEndian.PutUint64(record[4:], 44) - binary.LittleEndian.PutUint64(record[24:], 2) - binary.LittleEndian.PutUint64(record[32:], 2) - binary.LittleEndian.PutUint64(record[40:], uint64(binary.LittleEndian.Uint32(data[end+12:]))) - binary.LittleEndian.PutUint64(record[48:], uint64(binary.LittleEndian.Uint32(data[end+16:]))) - binary.LittleEndian.PutUint32(record[56:], zipDirectory64LocSignature) - binary.LittleEndian.PutUint64(record[64:], uint64(end)) - binary.LittleEndian.PutUint32(record[72:], 1) - data = append(append(data[:end:end], record...), data[end:]...) - binary.LittleEndian.PutUint16(data[len(data)-12:], zipDirectoryRecords64Marker) + data = withZIP64Count(data, 2) } b, err := OpenBrowserReaderAt("test.zip", bytes.NewReader(data), int64(len(data)), BrowseOptions{}) if err != nil { @@ -251,6 +239,25 @@ func TestReaderAtZIP64AndPrefix(t *testing.T) { } } +// withZIP64Count adds a ZIP64 end record and locator to a ZIP that has no +// archive comment. The ZIP64 record advertises records entries. +func withZIP64Count(data []byte, records uint64) []byte { + end := len(data) - zipDirectoryEndLen + record := make([]byte, zipDirectory64EndLen+zipDirectory64LocatorLen) + binary.LittleEndian.PutUint32(record, zipDirectory64EndSignature) + binary.LittleEndian.PutUint64(record[4:], 44) + binary.LittleEndian.PutUint64(record[24:], records) + binary.LittleEndian.PutUint64(record[32:], records) + binary.LittleEndian.PutUint64(record[40:], uint64(binary.LittleEndian.Uint32(data[end+12:]))) + binary.LittleEndian.PutUint64(record[48:], uint64(binary.LittleEndian.Uint32(data[end+16:]))) + binary.LittleEndian.PutUint32(record[56:], zipDirectory64LocSignature) + binary.LittleEndian.PutUint64(record[64:], uint64(end)) + binary.LittleEndian.PutUint32(record[72:], 1) + out := append(append(data[:end:end], record...), data[end:]...) + binary.LittleEndian.PutUint16(out[len(out)-12:], zipDirectoryRecords64Marker) + return out +} + // zipDirectoryStartForTest returns the central directory offset of data. func zipDirectoryStartForTest(t *testing.T, data []byte) int { t.Helper() @@ -334,8 +341,10 @@ func TestZIPParserParity(t *testing.T) { {name: "wrapped count", build: func(t *testing.T) []byte { t.Helper(); return wrappedCountZIP(t, 70_000) }, wantOpen: true}, {name: "wrapped count over limit", build: func(t *testing.T) []byte { t.Helper(); return wrappedCountZIP(t, 70_000) }, maxEntries: 69_999, wantErr: ErrEntryLimit}, {name: "headers over limit", build: edit(func([]byte) {}), maxEntries: 1, wantErr: ErrEntryLimit}, - {name: "underreported count", build: endCount(1), maxEntries: 2}, - {name: "advertised count over limit", build: endCount(40000), maxEntries: 2}, + // archive/zip compares the advertised count modulo 65536, so these + // fixtures disagree with the headers but archive/zip still opens them. + {name: "underreported count", build: func(t *testing.T) []byte { t.Helper(); return wrappedCountZIP(t, 65_537) }, maxEntries: 65_537, wantOpen: true}, + {name: "advertised count over limit", build: func(*testing.T) []byte { return withZIP64Count(bytes.Clone(valid), 2+65_536) }, maxEntries: 2, wantOpen: true}, {name: "header length", build: func(t *testing.T) []byte { t.Helper() data := bytes.Clone(valid) @@ -364,6 +373,13 @@ func TestZIPParserParity(t *testing.T) { {name: "truncated", build: func(*testing.T) []byte { return valid[:12] }}, {name: "empty", build: func(*testing.T) []byte { return nil }}, {name: "executable prefix", build: func(*testing.T) []byte { return append([]byte("executable prefix"), valid...) }, wantOpen: true}, + {name: "comment past short window", build: func(t *testing.T) []byte { + t.Helper() + data := bytes.Clone(valid) + comment := bytes.Repeat([]byte("c"), 4*zipDirectoryEndShortWindow) + binary.LittleEndian.PutUint16(data[len(data)-2:], uint16(len(comment))) + return append(data, comment...) + }, wantOpen: true}, } { t.Run(tc.name, func(t *testing.T) { data := tc.build(t) diff --git a/zip.go b/zip.go index 590c511..3d51d0e 100644 --- a/zip.go +++ b/zip.go @@ -34,6 +34,7 @@ const ( zipDirectoryEndSignature = 0x06054b50 zipDirectory64EndSignature = 0x06064b50 zipDirectory64LocSignature = 0x07064b50 + zipDirectoryEndShortWindow = 1024 zipDirectoryEndSearchWindow = 65 * 1024 zipDirectoryRecords64Marker = 0xffff zipDirectorySizeOffsetMarker = 0xffffffff @@ -119,56 +120,69 @@ func checkZipEntryCountLimitContext(ctx context.Context, input io.ReaderAt, size return nil } +// zipDirectoryEndRecord finds the end record the same way archive/zip does. +// It searches a short tail first, because the record is at size-22 when the +// archive has no comment. It widens the search only when that tail has no +// record. +func zipDirectoryEndRecord(input io.ReaderAt, size int64) ([]byte, int64, bool, error) { + for _, window := range [...]int64{zipDirectoryEndShortWindow, zipDirectoryEndSearchWindow} { + tailStart := max(size-window, 0) + tail := make([]byte, size-tailStart) + if err := readZipSource(input, tail, tailStart); err != nil { + return nil, 0, false, err + } + for offset := len(tail) - zipDirectoryEndLen; offset >= 0; offset-- { + if binary.LittleEndian.Uint32(tail[offset:]) != zipDirectoryEndSignature { + continue + } + commentLen := int(binary.LittleEndian.Uint16(tail[offset+20:])) + if offset+zipDirectoryEndLen+commentLen > len(tail) { + continue + } + return tail[offset : offset+zipDirectoryEndLen], tailStart + int64(offset), true, nil + } + if tailStart == 0 { + break + } + } + return nil, 0, false, nil +} + func zipCentralDirectoryBounds(input io.ReaderAt, size int64) (int64, int64, bool, error) { - tailStart := max(size-zipDirectoryEndSearchWindow, 0) - tail := make([]byte, size-tailStart) - if err := readZipSource(input, tail, tailStart); err != nil { + record, recordOffset, found, err := zipDirectoryEndRecord(input, size) + if err != nil || !found { return 0, 0, false, err } - for offset := len(tail) - zipDirectoryEndLen; offset >= 0; offset-- { - if binary.LittleEndian.Uint32(tail[offset:]) != zipDirectoryEndSignature { - continue - } - commentLen := int(binary.LittleEndian.Uint16(tail[offset+20:])) - if offset+zipDirectoryEndLen+commentLen > len(tail) { - continue - } - - recordOffset := tailStart + int64(offset) - directoryEnd := recordOffset - directoryRecords := binary.LittleEndian.Uint16(tail[offset+10:]) - directorySize := uint64(binary.LittleEndian.Uint32(tail[offset+12:])) - directoryOffset := uint64(binary.LittleEndian.Uint32(tail[offset+16:])) - if directoryRecords == zipDirectoryRecords64Marker || - directorySize == zipDirectorySizeOffsetMarker || - directoryOffset == zipDirectorySizeOffsetMarker { - var ok bool - var err error - directoryEnd, directorySize, directoryOffset, ok, err = zip64DirectoryRecord(input, size, recordOffset) - if err != nil || !ok { - return 0, 0, false, err - } + directoryEnd := recordOffset + directoryRecords := binary.LittleEndian.Uint16(record[10:]) + directorySize := uint64(binary.LittleEndian.Uint32(record[12:])) + directoryOffset := uint64(binary.LittleEndian.Uint32(record[16:])) + if directoryRecords == zipDirectoryRecords64Marker || + directorySize == zipDirectorySizeOffsetMarker || + directoryOffset == zipDirectorySizeOffsetMarker { + var ok bool + directoryEnd, directorySize, directoryOffset, ok, err = zip64DirectoryRecord(input, size, recordOffset) + if err != nil || !ok { + return 0, 0, false, err } + } - if directorySize > uint64(directoryEnd) { - return 0, 0, false, nil + if directorySize > uint64(directoryEnd) { + return 0, 0, false, nil + } + start := directoryEnd - int64(directorySize) + if directoryOffset < uint64(start) { + // ZIPs can have an executable prefix whose length is absent from offsets. + var signature [zipSignatureLen]byte + if err := readZipSource(input, signature[:], int64(directoryOffset)); err != nil { + return 0, 0, false, err } - start := directoryEnd - int64(directorySize) - if directoryOffset < uint64(start) { - // ZIPs can have an executable prefix whose length is absent from offsets. - var signature [zipSignatureLen]byte - if err := readZipSource(input, signature[:], int64(directoryOffset)); err != nil { - return 0, 0, false, err - } - if binary.LittleEndian.Uint32(signature[:]) == zipDirectoryHeaderSignature { - start = int64(directoryOffset) - } + if binary.LittleEndian.Uint32(signature[:]) == zipDirectoryHeaderSignature { + start = int64(directoryOffset) } - return start, directoryEnd, true, nil } - - return 0, 0, false, nil + return start, directoryEnd, true, nil } func zip64DirectoryRecord(input io.ReaderAt, size, endOffset int64) (int64, uint64, uint64, bool, error) { From a59701fc2fa7162ddcfd50d0fb0014c17cbc492f Mon Sep 17 00:00:00 2001 From: Yash Kakade Date: Wed, 4 Nov 2026 11:00:07 -0500 Subject: [PATCH 4/5] Test the wide ZIP end-record search directly The open path also rejects len(reader.File) over the limit, so the parity case passes without the 65 KiB search. Call the parser directly to pin it, and shorten the zipDirectoryEndRecord comment. Co-Authored-By: Claude Opus 5.5 --- random_access_test.go | 26 +++++++++++++++++++------- zip.go | 5 +---- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/random_access_test.go b/random_access_test.go index 32d9ed3..130ee1a 100644 --- a/random_access_test.go +++ b/random_access_test.go @@ -373,13 +373,9 @@ func TestZIPParserParity(t *testing.T) { {name: "truncated", build: func(*testing.T) []byte { return valid[:12] }}, {name: "empty", build: func(*testing.T) []byte { return nil }}, {name: "executable prefix", build: func(*testing.T) []byte { return append([]byte("executable prefix"), valid...) }, wantOpen: true}, - {name: "comment past short window", build: func(t *testing.T) []byte { - t.Helper() - data := bytes.Clone(valid) - comment := bytes.Repeat([]byte("c"), 4*zipDirectoryEndShortWindow) - binary.LittleEndian.PutUint16(data[len(data)-2:], uint16(len(comment))) - return append(data, comment...) - }, wantOpen: true}, + {name: "comment past short window", build: func(*testing.T) []byte { + return withZIPComment(valid, 4*zipDirectoryEndShortWindow) + }, maxEntries: 1, wantErr: ErrEntryLimit}, } { t.Run(tc.name, func(t *testing.T) { data := tc.build(t) @@ -408,6 +404,22 @@ func TestZIPParserParity(t *testing.T) { } } +func withZIPComment(data []byte, length int) []byte { + data = bytes.Clone(data) + binary.LittleEndian.PutUint16(data[len(data)-2:], uint16(length)) + return append(data, bytes.Repeat([]byte("c"), length)...) +} + +// The open path also checks len(reader.File), so only a direct parser call +// shows that the wide search found the end record. +func TestZIPEntryCountSearchesPastShortWindow(t *testing.T) { + data := withZIPComment(randomAccessZIP(t, 16), 4*zipDirectoryEndShortWindow) + err := checkZipEntryCountLimitContext(context.Background(), bytes.NewReader(data), int64(len(data)), 1) + if !errors.Is(err, ErrEntryLimit) { + t.Fatalf("error = %v, want %v", err, ErrEntryLimit) + } +} + func TestZIPEntryCountBuffersDirectoryReads(t *testing.T) { const entries = 5000 data := wrappedCountZIP(t, entries) diff --git a/zip.go b/zip.go index 3d51d0e..31a57df 100644 --- a/zip.go +++ b/zip.go @@ -120,10 +120,7 @@ func checkZipEntryCountLimitContext(ctx context.Context, input io.ReaderAt, size return nil } -// zipDirectoryEndRecord finds the end record the same way archive/zip does. -// It searches a short tail first, because the record is at size-22 when the -// archive has no comment. It widens the search only when that tail has no -// record. +// Search 1 KiB before 65 KiB, as archive/zip does; most archives have no comment. func zipDirectoryEndRecord(input io.ReaderAt, size int64) ([]byte, int64, bool, error) { for _, window := range [...]int64{zipDirectoryEndShortWindow, zipDirectoryEndSearchWindow} { tailStart := max(size-window, 0) From 05e44da752286b13155f9f0d8e918cb50951787b Mon Sep 17 00:00:00 2001 From: Yash Kakade Date: Wed, 4 Nov 2026 11:22:16 -0500 Subject: [PATCH 5/5] Count ZIP headers when no ZIP64 locator exists archive/zip keeps the classic end-record values when a ZIP64 marker has no valid locator before the end record. The parser gave up in that case, so a wrapped count of 0xffff skipped the entry limit until archive/zip had built its file list. Accept the same locators as archive/zip and fall back to the classic values otherwise. Co-Authored-By: Claude Opus 5.5 --- random_access_test.go | 11 +++++++++++ zip.go | 32 +++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/random_access_test.go b/random_access_test.go index 130ee1a..a19175d 100644 --- a/random_access_test.go +++ b/random_access_test.go @@ -367,6 +367,7 @@ func TestZIPParserParity(t *testing.T) { }, maxEntries: 1}, {name: "no end record", build: edit(func(data []byte) { data[len(data)-zipDirectoryEndLen] = 0 })}, {name: "missing zip64 locator", build: endCount(zipDirectoryRecords64Marker)}, + {name: "marker count without locator over limit", build: func(t *testing.T) []byte { t.Helper(); return wrappedCountZIP(t, 131_071) }, maxEntries: 131_070, wantErr: ErrEntryLimit}, {name: "directory size past start", build: edit(func(data []byte) { binary.LittleEndian.PutUint32(data[len(data)-10:], uint32(len(data))) })}, @@ -420,6 +421,16 @@ func TestZIPEntryCountSearchesPastShortWindow(t *testing.T) { } } +// archive/zip keeps the classic end record when no ZIP64 locator precedes it, +// so the parser must still count headers when the count is the ZIP64 marker. +func TestZIPEntryCountWithoutZIP64Locator(t *testing.T) { + data := wrappedCountZIP(t, 131_071) + err := checkZipEntryCountLimitContext(context.Background(), bytes.NewReader(data), int64(len(data)), 131_070) + if !errors.Is(err, ErrEntryLimit) { + t.Fatalf("error = %v, want %v", err, ErrEntryLimit) + } +} + func TestZIPEntryCountBuffersDirectoryReads(t *testing.T) { const entries = 5000 data := wrappedCountZIP(t, entries) diff --git a/zip.go b/zip.go index 31a57df..143e531 100644 --- a/zip.go +++ b/zip.go @@ -158,11 +158,18 @@ func zipCentralDirectoryBounds(input io.ReaderAt, size int64) (int64, int64, boo if directoryRecords == zipDirectoryRecords64Marker || directorySize == zipDirectorySizeOffsetMarker || directoryOffset == zipDirectorySizeOffsetMarker { - var ok bool - directoryEnd, directorySize, directoryOffset, ok, err = zip64DirectoryRecord(input, size, recordOffset) - if err != nil || !ok { + recordOffset64, located, err := zip64LocatorOffset(input, recordOffset) + if err != nil { return 0, 0, false, err } + // Without a locator, archive/zip keeps the classic values, so count with them too. + if located { + var ok bool + directoryEnd, directorySize, directoryOffset, ok, err = zip64DirectoryRecord(input, size, recordOffset64) + if err != nil || !ok { + return 0, 0, false, err + } + } } if directorySize > uint64(directoryEnd) { @@ -182,18 +189,25 @@ func zipCentralDirectoryBounds(input io.ReaderAt, size int64) (int64, int64, boo return start, directoryEnd, true, nil } -func zip64DirectoryRecord(input io.ReaderAt, size, endOffset int64) (int64, uint64, uint64, bool, error) { +// zip64LocatorOffset accepts the same locators as archive/zip: one disk, with +// the ZIP64 end record on disk 0. +func zip64LocatorOffset(input io.ReaderAt, endOffset int64) (uint64, bool, error) { if endOffset < zipDirectory64LocatorLen { - return 0, 0, 0, false, nil + return 0, false, nil } var locator [zipDirectory64LocatorLen]byte if err := readZipSource(input, locator[:], endOffset-zipDirectory64LocatorLen); err != nil { - return 0, 0, 0, false, err + return 0, false, err } - if binary.LittleEndian.Uint32(locator[:]) != zipDirectory64LocSignature { - return 0, 0, 0, false, nil + if binary.LittleEndian.Uint32(locator[:]) != zipDirectory64LocSignature || + binary.LittleEndian.Uint32(locator[4:]) != 0 || + binary.LittleEndian.Uint32(locator[16:]) != 1 { + return 0, false, nil } - recordOffset := binary.LittleEndian.Uint64(locator[8:]) + return binary.LittleEndian.Uint64(locator[8:]), true, nil +} + +func zip64DirectoryRecord(input io.ReaderAt, size int64, recordOffset uint64) (int64, uint64, uint64, bool, error) { if size < zipDirectory64EndLen || recordOffset > uint64(size-zipDirectory64EndLen) { return 0, 0, 0, false, nil }