Merge remote-tracking branch 'upstream/main' into add-nested-archive-cataloging

internal/file/reader_size.go

@@ -0,0 +1,66 @@
+package file
+
+import "io"
+
+// ReaderSize reports the number of bytes behind r. The bool is false when that cannot be determined, and
+// callers must branch on it rather than treating the zero as a size.
+//
+// Binary parsers read sizes out of the files they are parsing and allocate against them. Those sizes are
+// 32- and 64-bit fields under the control of whoever produced the file, so a truncated or hostile input
+// can declare gigabytes that are not there. The read would fail afterwards either way; what matters is
+// refusing before the allocation, and that needs a real byte count to weigh the claim against.
+//
+// The bool is the whole point of the signature. An earlier version returned 0 for every failure mode, and
+// a caller that forgot to special-case it got a bound of "nothing is allowed" or, more often, silently
+// fell back to an unbounded path. Making the caller name the failure keeps that from happening by
+// omission.
+//
+// Both *bytes.Reader and *io.SectionReader answer directly. A file handle only has Seek, so fall back to
+// a save-and-restore seek, which leaves the caller's cursor where it found it. That fallback is not
+// atomic: it moves the cursor and puts it back, so a reader being read concurrently can observe the
+// intermediate position.
+//
+// Neither answer is taken on faith, because a reader can report a length it cannot deliver: an
+// *io.SectionReader answers with the length it was constructed with, and debug/elf and friends construct
+// theirs with a nominal one, so io.NewSectionReader(r, 0, 1<<63-1) would otherwise report a bound that
+// permits everything as if it had been measured. The last byte is read back to confirm the count, which is
+// one ReadAt whatever the size, and the read is what the caller was going to weigh anyway.
+//
+// A wrapper that embeds io.ReaderAt as an interface promotes only ReadAt, so it answers no size here
+// however sizable the reader underneath it is. A type meant to be bounded needs to forward Size() itself.
+func ReaderSize(r io.ReaderAt) (int64, bool) {
+	size, ok := reportedSize(r)
+	if !ok {
+		return 0, false
+	}
+	// ReadAt may return io.EOF alongside a full read, so the count is what says the byte is there
+	var last [1]byte
+	if n, _ := r.ReadAt(last[:], size-1); n != len(last) {
+		return 0, false
+	}
+	return size, true
+}
+
+// reportedSize is what the reader says about itself, before ReaderSize checks whether it can back it up.
+func reportedSize(r io.ReaderAt) (int64, bool) {
+	if sr, ok := r.(interface{ Size() int64 }); ok {
+		size := sr.Size()
+		return size, size > 0
+	}
+	s, ok := r.(io.Seeker)
+	if !ok {
+		return 0, false
+	}
+	cur, err := s.Seek(0, io.SeekCurrent)
+	if err != nil {
+		return 0, false
+	}
+	size, err := s.Seek(0, io.SeekEnd)
+	if err != nil {
+		return 0, false
+	}
+	if _, err := s.Seek(cur, io.SeekStart); err != nil {
+		return 0, false
+	}
+	return size, size > 0
+}

internal/file/reader_size_test.go

@@ -0,0 +1,140 @@
+package file
+
+import (
+	"bytes"
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestReaderSize(t *testing.T) {
+	tests := []struct {
+		name   string
+		r      io.ReaderAt
+		want   int64
+		wantOK bool
+	}{
+		{
+			name:   "bytes.Reader answers directly",
+			r:      bytes.NewReader(make([]byte, 1234)),
+			want:   1234,
+			wantOK: true,
+		},
+		{
+			// an empty reader is not a size to bound against, so it reports not-ok along with everything
+			// else that cannot be measured
+			name: "empty bytes.Reader",
+			r:    bytes.NewReader(nil),
+			want: 0,
+		},
+		{
+			name:   "SectionReader over a real length",
+			r:      io.NewSectionReader(bytes.NewReader(make([]byte, 500)), 100, 300),
+			want:   300,
+			wantOK: true,
+		},
+		{
+			name:   "seek-only reader falls back to seeking",
+			r:      seekOnly{bytes.NewReader(make([]byte, 4096))},
+			want:   4096,
+			wantOK: true,
+		},
+		{
+			name: "reader with neither Size nor Seek cannot be measured",
+			r:    readAtOnly{bytes.NewReader(make([]byte, 4096))},
+			want: 0,
+		},
+		{
+			// the shape debug/elf, debug/pe and debug/macho build internally. Taking this at its word hands
+			// back a bound that permits everything, reported as if it had been measured.
+			name: "SectionReader over a nominal length reports what it can deliver",
+			r:    io.NewSectionReader(bytes.NewReader(make([]byte, 500)), 0, 1<<63-1),
+			want: 0,
+		},
+		{
+			// a Size() a type simply got wrong is the same failure: the bytes are what decides
+			name: "a size the reader cannot back up is refused",
+			r:    overstatedSize{ReaderAt: bytes.NewReader(make([]byte, 10)), size: 1 << 30},
+			want: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			size, ok := ReaderSize(tt.r)
+			assert.Equal(t, tt.want, size)
+			assert.Equal(t, tt.wantOK, ok)
+		})
+	}
+}
+
+func TestReaderSize_FileHandle(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "f.bin")
+	require.NoError(t, os.WriteFile(path, make([]byte, 7777), 0o600))
+	f, err := os.Open(path)
+	require.NoError(t, err)
+	defer f.Close()
+
+	// the cursor has to come back where it was found: the caller may be mid-read, and an *os.File is the
+	// one input here that carries a shared position
+	_, err = f.Seek(101, io.SeekStart)
+	require.NoError(t, err)
+
+	size, ok := ReaderSize(f)
+	assert.True(t, ok)
+	assert.Equal(t, int64(7777), size)
+
+	at, err := f.Seek(0, io.SeekCurrent)
+	require.NoError(t, err)
+	assert.Equal(t, int64(101), at, "measuring the file must not move the caller's cursor")
+}
+
+// TestReaderSize_WrapperMustForwardSize pins the shape that made every bound expressed against this
+// helper go silently inert. A type that embeds io.ReaderAt as an interface promotes only ReadAt, so it
+// answers 0 here however sizable the reader underneath is, and a caller that treats 0 as "fall back to
+// unbounded" then stops bounding anything. Forwarding Size() is what fixes it, and this is the assertion
+// that a new wrapper has to satisfy.
+func TestReaderSize_WrapperMustForwardSize(t *testing.T) {
+	inner := bytes.NewReader(make([]byte, 2048))
+
+	size, ok := ReaderSize(embedsReaderAt{inner})
+	assert.False(t, ok, "a wrapper that only embeds the interface cannot be measured")
+	assert.Equal(t, int64(0), size)
+
+	size, ok = ReaderSize(forwardsSize{inner})
+	assert.True(t, ok, "a wrapper meant to be bounded has to forward Size itself")
+	assert.Equal(t, int64(2048), size)
+}
+
+type seekOnly struct{ inner *bytes.Reader }
+
+func (s seekOnly) ReadAt(p []byte, off int64) (int, error) { return s.inner.ReadAt(p, off) }
+func (s seekOnly) Seek(off int64, whence int) (int64, error) {
+	return s.inner.Seek(off, whence)
+}
+
+type readAtOnly struct{ inner *bytes.Reader }
+
+func (r readAtOnly) ReadAt(p []byte, off int64) (int, error) { return r.inner.ReadAt(p, off) }
+
+type embedsReaderAt struct{ io.ReaderAt }
+
+type forwardsSize struct{ io.ReaderAt }
+
+func (f forwardsSize) Size() int64 {
+	size, _ := ReaderSize(f.ReaderAt)
+	return size
+}
+
+// overstatedSize answers with a size larger than the bytes behind it, the way a wrapper that returns a
+// declared length rather than a measured one would.
+type overstatedSize struct {
+	io.ReaderAt
+	size int64
+}
+
+func (o overstatedSize) Size() int64 { return o.size }