fix(sdk): DSPX-4590 zip64 conformance and per-segment size defaults

sdk/internal/zipstream/fuzz_test.go

@@ -5,6 +5,7 @@ package zipstream
 import (
 	"bytes"
 	"encoding/base64"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -130,6 +131,24 @@ func FuzzReader(f *testing.F) {
 		"AAAAAAAAAwLnBheWxvYWRQSwECLQAtAAgAAAB9dS8xAuiuLAIE///tBQAADwAAAAAAAAAAA" +
 		"AAAAABWAAAAMC5tYW5pZmVzdC5qc29uUEsFBgAAAAACAAIAdAAAAJUFAAAAAA=="))
 
+	// A central directory file comment: the entry after it only parses if
+	// nextCD accounts for FileCommentLength.
+	f.Add(buildRawZip(f, []rawZipEntry{
+		{name: "0.payload", data: []byte("payload bytes"), comment: "central directory file comment"},
+		{name: "0.manifest.json", data: []byte(`{"m":1}`)},
+	}, false))
+
+	// Filename plus extra field plus header size sums to 65646, which wraps
+	// to 110 if the addition happens at uint16 width.
+	f.Add(buildRawZip(f, []rawZipEntry{
+		{
+			name:        strings.Repeat("n", 65000),
+			data:        []byte("payload bytes"),
+			extraPrefix: bytes.Repeat([]byte{0}, 600),
+		},
+		{name: "0.manifest.json", data: []byte(`{"m":1}`)},
+	}, false))
+
 	f.Fuzz(func(t *testing.T, data []byte) {
 		reader, err := NewReader(bytes.NewReader(data))
 		if err != nil {

sdk/internal/zipstream/reader.go

@@ -72,15 +72,19 @@ func NewReader(readSeeker io.ReadSeeker) (Reader, error) {
 	}
 
 	// check if zip is zip64 or zip32 format
+	//
+	// Any of the three EOCD fields that ZIP64 can overflow may carry the
+	// sentinel independently: the entry count (two bytes wide, so its
+	// sentinel is 0xFFFF), the central directory size, and the central
+	// directory offset. An archive with more than 65534 entries needs ZIP64
+	// for the count alone while its central directory still starts below
+	// 4 GiB, so keying off the offset by itself misses it.
 	var entryCount uint64
 	var centralDirectoryStart uint64
-	isZip64 := false
-	if endOfCDRecord.CentralDirectoryOffset != zip64MagicVal { //nolint:nestif // pkzip is complicated
+	if !eocdNeedsZip64(endOfCDRecord) { //nolint:nestif // pkzip is complicated
 		entryCount = uint64(endOfCDRecord.NumberOfCDRecordEntries)
 		centralDirectoryStart = uint64(endOfCDRecord.CentralDirectoryOffset)
 	} else {
-		isZip64 = true
-
 		// read zip64 end of central directory locator
 		_, err := readSeeker.Seek(-(endOfCDRecordSize + zip64EndOfCDRecordLocatorSize), io.SeekEnd)
 		if err != nil {
@@ -144,52 +148,10 @@ func NewReader(readSeeker io.ReadSeeker) (Reader, error) {
 			return reader, fmt.Errorf("binary.Read failed: %w", err)
 		}
 
-		offset := uint64(cdFileHeader.LocalHeaderOffset)
-		bytesToRead := uint64(cdFileHeader.CompressedSize)
-
-		if isZip64 { //nolint:nestif // pkzip is complicated
-			// read Zip64 Extended Information extra field id
-			headerTag := uint16(0)
-			err = binary.Read(readSeeker, binary.LittleEndian, &headerTag)
-			if err != nil {
-				return reader, fmt.Errorf("binary.Read failed: %w", err)
-			}
-
-			// read Zip64 Extended Information Extra Field Block Size
-			blockSize := uint16(0)
-			err = binary.Read(readSeeker, binary.LittleEndian, &blockSize)
-			if err != nil {
-				return reader, fmt.Errorf("binary.Read failed: %w", err)
-			}
-
-			if headerTag == zip64ExternalID {
-				if cdFileHeader.CompressedSize == zip64MagicVal {
-					compressedSize := uint64(0)
-					err = binary.Read(readSeeker, binary.LittleEndian, &compressedSize)
-					if err != nil {
-						return reader, fmt.Errorf("binary.Read failed: %w", err)
-					}
-
-					bytesToRead = compressedSize
-				}
-
-				if cdFileHeader.UncompressedSize == zip64MagicVal {
-					uncompressedSize := uint64(0)
-					err = binary.Read(readSeeker, binary.LittleEndian, &uncompressedSize)
-					if err != nil {
-						return reader, fmt.Errorf("binary.Read failed: %w", err)
-					}
-				}
-
-				if cdFileHeader.LocalHeaderOffset == zip64MagicVal {
-					localHeaderOffset := uint64(0)
-					err = binary.Read(readSeeker, binary.LittleEndian, &localHeaderOffset)
-					if err != nil {
-						return reader, fmt.Errorf("binary.Read failed: %w", err)
-					}
-					offset = localHeaderOffset
-				}
-			}
+		// readSeeker is now positioned at this entry's extra-field area.
+		offset, bytesToRead, err := resolveEntryLocation(readSeeker, cdFileHeader)
+		if err != nil {
+			return reader, err
 		}
 
 		// Read each file
@@ -214,12 +176,156 @@ func NewReader(readSeeker io.ReadSeeker) (Reader, error) {
 
 		reader.fileEntries[string(fileNameByteArray)] = zipFileEntry
 
-		nextCD += uint64(cdFileHeader.ExtraFieldLength + cdFileHeader.FilenameLength + cdFileHeaderSize)
+		// Widen every term before summing: all three header lengths are
+		// uint16, so adding them at their declared width wraps at 65536 and
+		// lands the next seek inside the current entry. The file comment is
+		// part of the record too -- omitting it desyncs every entry after
+		// the first one that carries a comment.
+		nextCD += uint64(cdFileHeaderSize) +
+			uint64(cdFileHeader.FilenameLength) +
+			uint64(cdFileHeader.ExtraFieldLength) +
+			uint64(cdFileHeader.FileCommentLength)
 	}
 
 	return reader, nil
 }
 
+// resolveEntryLocation returns the local header offset and the number of
+// stored bytes for a central directory entry, reading the ZIP64 extended
+// information extra field when the 32-bit fields carry the sentinel.
+//
+// That field is a property of the entry, not of the archive: APPNOTE permits
+// one on an entry whose EOCD is not ZIP64, so the lookup is driven off this
+// entry's own sentinel values rather than an archive-wide flag. The reader
+// must be positioned at the start of the entry's extra-field area, and the
+// area is only consumed when the entry declares one -- reading
+// unconditionally would eat the bytes of whatever record follows.
+func resolveEntryLocation(readSeeker io.Reader, cdFileHeader CDFileHeader) (uint64, uint64, error) {
+	offset := uint64(cdFileHeader.LocalHeaderOffset)
+	bytesToRead := uint64(cdFileHeader.CompressedSize)
+
+	if cdFileHeader.ExtraFieldLength == 0 || !cdHeaderHasZip64Sentinel(cdFileHeader) {
+		return offset, bytesToRead, nil
+	}
+
+	extraFields := make([]byte, cdFileHeader.ExtraFieldLength)
+	if _, err := io.ReadFull(readSeeker, extraFields); err != nil {
+		return 0, 0, fmt.Errorf("io.ReadFull failed: %w", err)
+	}
+
+	zip64, err := parseZip64ExtraField(extraFields, cdFileHeader)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	if zip64.found {
+		if cdFileHeader.CompressedSize == zip64MagicVal {
+			bytesToRead = zip64.compressedSize
+		}
+		if cdFileHeader.LocalHeaderOffset == zip64MagicVal {
+			offset = zip64.localHeaderOffset
+		}
+	}
+
+	return offset, bytesToRead, nil
+}
+
+// eocdNeedsZip64 reports whether the end of central directory record defers
+// any of its fields to the ZIP64 end of central directory record. Note the
+// entry count is two bytes wide, so it uses a 16-bit sentinel.
+func eocdNeedsZip64(eocd EndOfCDRecord) bool {
+	return eocd.CentralDirectoryOffset == zip64MagicVal ||
+		eocd.SizeOfCentralDirectory == zip64MagicVal ||
+		eocd.NumberOfCDRecordEntries == zip64MagicVal16
+}
+
+// cdHeaderHasZip64Sentinel reports whether any central directory field of
+// this entry defers its value to a ZIP64 extended information extra field.
+func cdHeaderHasZip64Sentinel(h CDFileHeader) bool {
+	return h.CompressedSize == zip64MagicVal ||
+		h.UncompressedSize == zip64MagicVal ||
+		h.LocalHeaderOffset == zip64MagicVal
+}
+
+// zip64ExtraValues holds the values a ZIP64 Extended Information extra
+// field supplies for one central directory entry. Only the fields whose
+// central directory counterpart carried the sentinel are populated.
+type zip64ExtraValues struct {
+	found             bool
+	compressedSize    uint64
+	localHeaderOffset uint64
+}
+
+// parseZip64ExtraField walks the whole extra-field area of a central
+// directory entry looking for the ZIP64 Extended Information field
+// (0x0001). The field is not required to come first -- a Unix timestamp or
+// NTFS field frequently precedes it -- so the area has to be iterated
+// rather than probed at its head.
+//
+// Within the field the values appear in APPNOTE 4.5.3 order: original
+// (uncompressed) size, compressed size, then local header offset. Each is
+// present only when the matching central directory field holds the
+// 0xFFFFFFFF sentinel, so the uncompressed size has to be stepped over even
+// though nothing here consumes it -- reading the compressed size first
+// would hand back the wrong value for any entry where the two differ.
+func parseZip64ExtraField(extraFields []byte, h CDFileHeader) (zip64ExtraValues, error) {
+	var values zip64ExtraValues
+
+	for pos := 0; pos+extraFieldHeaderSize <= len(extraFields); {
+		tag := binary.LittleEndian.Uint16(extraFields[pos:])
+		size := int(binary.LittleEndian.Uint16(extraFields[pos+2:]))
+		pos += extraFieldHeaderSize
+
+		if size > len(extraFields)-pos {
+			// A field claiming to run past the end of the area is
+			// malformed; there is nothing sane to resync to.
+			return values, errZipFormat
+		}
+
+		if tag != zip64ExternalID {
+			pos += size
+			continue
+		}
+
+		body := extraFields[pos : pos+size]
+		bodyPos := 0
+		read := func() (uint64, bool) {
+			const uint64Size = 8
+			if bodyPos+uint64Size > len(body) {
+				return 0, false
+			}
+			v := binary.LittleEndian.Uint64(body[bodyPos:])
+			bodyPos += uint64Size
+			return v, true
+		}
+
+		if h.UncompressedSize == zip64MagicVal {
+			if _, ok := read(); !ok {
+				return values, errZipFormat
+			}
+		}
+		if h.CompressedSize == zip64MagicVal {
+			v, ok := read()
+			if !ok {
+				return values, errZipFormat
+			}
+			values.compressedSize = v
+		}
+		if h.LocalHeaderOffset == zip64MagicVal {
+			v, ok := read()
+			if !ok {
+				return values, errZipFormat
+			}
+			values.localHeaderOffset = v
+		}
+
+		values.found = true
+		return values, nil
+	}
+
+	return values, nil
+}
+
 // ReadFileData Read data from file of given length of size.
 func (reader Reader) ReadFileData(filename string, index int64, length int64) ([]byte, error) {
 	fileNameEntry, ok := reader.fileEntries[filename]

sdk/internal/zipstream/segment_writer.go

@@ -33,10 +33,13 @@ func NewSegmentTDFWriter(expectedSegments int, opts ...Option) SegmentWriter {
 
 	base := newBaseWriter(cfg)
 
+	centralDir := NewCentralDirectory()
+	centralDir.MaxNonZip64Value = cfg.MaxNonZip64Value
+
 	return &segmentWriter{
 		baseWriter: base,
 		metadata:   NewSegmentMetadata(expectedSegments, cfg.Now),
-		centralDir: NewCentralDirectory(),
+		centralDir: centralDir,
 		payloadEntry: &FileEntry{
 			Name:        TDFPayloadFileName,
 			Offset:      0,
@@ -191,11 +194,12 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
 	// Total payload size = header + all data (no data descriptor in this calculation)
 	totalPayloadSize := headerSize + sw.payloadEntry.CompressedSize
 
-	// Decide whether payload descriptor must be ZIP64
-	const max32 = ^uint32(0)
+	// Decide whether payload descriptor must be ZIP64. The switch point is
+	// 2 GiB rather than 4 GiB: see maxNonZip64Value.
+	maxNonZip64 := sw.config.MaxNonZip64Value
 	needZip64ForPayload := sw.config.Zip64 == Zip64Always ||
-		sw.payloadEntry.Size > uint64(max32) ||
-		sw.payloadEntry.CompressedSize > uint64(max32)
+		sw.payloadEntry.Size > maxNonZip64 ||
+		sw.payloadEntry.CompressedSize > maxNonZip64
 
 	// 1. Write data descriptor for payload (fail if Zip64Never but required)
 	if sw.config.Zip64 == Zip64Never && needZip64ForPayload {
@@ -230,7 +234,10 @@ func (sw *segmentWriter) Finalize(ctx context.Context, manifest []byte) ([]byte,
 	// 5. Write central directory
 	sw.centralDir.Offset = totalPayloadSize + uint64(buffer.Len())
 	// Decide if ZIP64 is needed for central directory/EOCD based on offset or forced mode
-	needZip64ForCD := needZip64ForPayload || sw.config.Zip64 == Zip64Always || sw.centralDir.Offset > uint64(max32) || len(sw.centralDir.Entries) > int(^uint16(0))
+	needZip64ForCD := needZip64ForPayload ||
+		sw.config.Zip64 == Zip64Always ||
+		sw.centralDir.Offset > maxNonZip64 ||
+		len(sw.centralDir.Entries) >= zip64MagicVal16
 	if sw.config.Zip64 == Zip64Never && needZip64ForCD {
 		return nil, &Error{Op: "finalize", Type: "segment", Err: ErrZip64Required}
 	}

sdk/internal/zipstream/writer.go

@@ -68,6 +68,7 @@ var (
 	ErrNoSegmentZero    = errors.New("segment 0 missing; it carries the payload local file header")
 	ErrInvalidSize      = errors.New("invalid size")
 	ErrZip64Required    = errors.New("ZIP64 required but disabled (Zip64Never)")
+	ErrFieldOverflow    = errors.New("value too large for zip field")
 )
 
 // Config holds configuration options for writers
@@ -79,6 +80,10 @@ type Config struct {
 	// Defaults to time.Now; tests inject a pinned clock for
 	// deterministic ZIP output.
 	Now func() time.Time
+	// MaxNonZip64Value is the largest size or offset written into a 32-bit
+	// zip field before the archive switches to ZIP64. Zero means
+	// maxNonZip64Value (2 GiB - 1). See WithMaxNonZip64Value.
+	MaxNonZip64Value uint64
 }
 
 // Option is a functional option for configuring writers
@@ -130,13 +135,27 @@ func WithClock(now func() time.Time) Option {
 	}
 }
 
+// WithMaxNonZip64Value lowers the point at which the writer switches to
+// ZIP64. This exists as a test seam -- mirroring java-sdk's injectable
+// MAX_NON_ZIP64_VALUE -- so the ZIP64 path can be exercised without
+// materializing a 2 GiB payload. Production callers should leave it alone;
+// zero or a value above the default is ignored.
+func WithMaxNonZip64Value(maxValue uint64) Option {
+	return func(c *Config) {
+		if maxValue > 0 && maxValue <= maxNonZip64Value {
+			c.MaxNonZip64Value = maxValue
+		}
+	}
+}
+
 // defaultConfig returns default configuration
 func defaultConfig() *Config {
 	return &Config{
-		Zip64:         Zip64Auto,
-		MaxSegments:   defaultMaxSegments,
-		EnableLogging: false,
-		Now:           time.Now,
+		Zip64:            Zip64Auto,
+		MaxSegments:      defaultMaxSegments,
+		EnableLogging:    false,
+		Now:              time.Now,
+		MaxNonZip64Value: maxNonZip64Value,
 	}
 }
 
@@ -154,6 +173,12 @@ func applyOptions(opts []Option) *Config {
 	if cfg.Now == nil {
 		cfg.Now = time.Now
 	}
+	// Same defence for the ZIP64 switch point: an option is free to zero
+	// the exported field, and zero would mean "switch to ZIP64 for
+	// everything" to the comparisons in Finalize.
+	if cfg.MaxNonZip64Value == 0 {
+		cfg.MaxNonZip64Value = maxNonZip64Value
+	}
 	return cfg
 }