From 29897187d2dc385425caa781372c4dbb784f2315 Mon Sep 17 00:00:00 2001 From: Coldwings Date: Wed, 26 Aug 2026 12:30:10 +0800 Subject: [PATCH] engine/fetch/chunk: reject peer block indices that overflow signed range geometry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The peer wire carries the block index as an unrestricted uint64, but relay geometry is int64 arithmetic: engine.PeerSource/PeerStreamSource converted with int64(req.Key.Block), and fetch.FetchBlock derived the byte window with signed multiplication. An index above MaxInt64 wrapped negative on conversion, and a smaller index whose start index*BlockSize exceeded MaxInt64 wrapped in the multiplication. A wrapped start either recycled to 0 — a 2^63 index was silently served block 0's bytes and cached under a garbage key — or went negative, where HTTPFetcher.Fetch omits the Range header and a one-block fetch degraded into a whole-object GET (the amplification shape of #15, re-entering via the peer path). Fix at two layers: - chunk.Config.MaxBlockIndex pins the largest index whose whole window [i*BlockSize, i*BlockSize+BlockSize-1] fits int64 (golden-tested against arbitrary-precision values). - The engine's relay sources decline req.Key.Block > MaxBlockIndex before cache lookup, relay selection, size probe, or origin I/O. Declined, not errored: the malformed index is the requester's fault, and a 500 would charge this healthy relay on the requester's circuit breaker. - fetch.FetchBlock rejects negative indices and overflowing windows before invoking the fetcher, so range arithmetic cannot wrap even if a future caller skips the boundary check. Regression tests pin 2^63, 2^64-1, and the first window-overflowing index on both the buffered and streaming relay paths (asserting zero origin contact), the FetchBlock guard with a capture fetcher, and the exact golden window for the largest representable index. Verified to fail on the pre-fix code. Fixes #52 --- docs/chunk.md | 14 ++++ docs/engine.md | 23 ++++++ docs/fetch.md | 13 +++ docs/peer.md | 8 +- internal/chunk/chunk.go | 17 ++++ internal/chunk/issue52_test.go | 56 +++++++++++++ internal/engine/engine.go | 23 ++++++ internal/engine/issue52_test.go | 142 ++++++++++++++++++++++++++++++++ internal/engine/stream.go | 4 + internal/fetch/fetch.go | 15 ++++ internal/fetch/issue52_test.go | 90 ++++++++++++++++++++ 11 files changed, 404 insertions(+), 1 deletion(-) create mode 100644 internal/chunk/issue52_test.go create mode 100644 internal/engine/issue52_test.go create mode 100644 internal/fetch/issue52_test.go diff --git a/docs/chunk.md b/docs/chunk.md index 617c4b4..be01114 100644 --- a/docs/chunk.md +++ b/docs/chunk.md @@ -53,10 +53,23 @@ func (c Config) ChunkIndex(offset int64) int64 // chunk containing offset func (c Config) BlockIndex(offset int64) int64 // global block containing offset func (c Config) ChunkOfBlock(blockIndex int64) int64 func (c Config) BlockStart(blockIndex int64) int64 // absolute byte offset of block start +func (c Config) MaxBlockIndex() int64 // largest index whose window fits int64 ``` All are O(1) integer arithmetic; `offset`/`blockIndex` must be ≥ 0. +`MaxBlockIndex` is `(MaxInt64-BlockSize+1)/BlockSize`: the largest block index +whose whole window `[i*BlockSize, i*BlockSize+BlockSize-1]` stays inside int64. +Above it the start multiplication (or the end addition) wraps — a wrapped start +recycles to 0 (silently addressing block 0) or goes negative (where an HTTP +range fetcher reads `start < 0` as "no Range header" and pulls the whole +object). Indices derived from clamped client ranges can never exceed it (an +offset is at most `MaxInt64`, so `BlockIndex(offset) <= MaxInt64/BlockSize`, +which equals the bound for the power-of-two block sizes DART uses), but the +peer wire carries the index as an unrestricted `uint64`, so relay code must +reject `Block > uint64(MaxBlockIndex())` before doing signed geometry with it +(see docs/engine.md §3.5, issue #52). Requires `BlockSize > 0`. + ### 3.3 `func (c Config) Segments(start, end int64) []Segment` Decomposes the **inclusive** absolute range `[start, end]` into the ordered @@ -211,6 +224,7 @@ go test ./internal/chunk/ -cover -count=1 | `TestChunkKeyAgainstPythonReference` | CI diffs the ChunkKey goldens against the tracked Python script | | `TestConfigValidate` | valid/invalid configs; `BlocksPerChunk` | | `TestGridMath` | ChunkIndex/BlockIndex/ChunkOfBlock/BlockStart across boundaries | +| **`TestMaxBlockIndexGolden`** | **`MaxBlockIndex` pinned against arbitrary-precision goldens; the bound's window fits int64 and the next index does not (issue #52)** | | `TestSegmentsSingleBlock` | a small read maps to exactly one block/sub-range | | `TestSegmentsCrossBlockAndChunk` | a range crossing block and chunk boundaries splits correctly | | `TestSegmentsInvalidRange` | negative start / end chunk.Config.MaxBlockIndex()`) is declined before any cache lookup, + relay selection, size probe, or origin I/O — never served, never cached, + never turned into a whole-object origin GET (issue #52; the fetch-layer + arithmetic guard is in docs/fetch.md §3.4). ## 5. Concurrency & Call Permissions @@ -403,6 +424,8 @@ go test ./internal/engine/ -cover -count=1 | `TestPassthroughCountsMetrics` | `dart_passthrough_total` increments; bytes count as client+origin wire bytes, not as any block source | | `TestPassthroughUnavailableWithoutOpener` | a non-streaming fetcher fails the passthrough cleanly with 502 | | `TestRelayDeclinesRangeBlindOrigin` | a relay declines (held=false) a block of a Range-unsupported object | +| **`TestPeerSourceRejectsMalformedBlockIndex`** | **wire indices above `MaxBlockIndex` (2^63, 2^64-1, first window-overflowing index) are declined before cache lookup/size probe/origin I/O; nothing cached; legitimate control still relays (issue #52)** | +| **`TestPeerStreamSourceRejectsMalformedBlockIndex`** | **same decline on the cut-through path: nothing streamed, sizer not called, zero origin requests; legitimate control still streams** | ## 8. Limitations & TODO diff --git a/docs/fetch.md b/docs/fetch.md index f4e4967..2a2f572 100644 --- a/docs/fetch.md +++ b/docs/fetch.md @@ -102,6 +102,17 @@ RFC-clamped 206 on the tail block (`Content-Range` shows `end = total-1` below the requested end, body length matching) is accepted, so the unknown-size flow covers the final block too. Any other length mismatch stays a hard error. +**Geometry that cannot wrap (issue #52).** `blockIndex` must be non-negative +with the whole window representable in int64, i.e. `blockSize > 0` and +`blockIndex <= (MaxInt64-blockSize+1)/blockSize` (the same bound +`chunk.Config.MaxBlockIndex` states in grid terms). Anything else — a negative +index (what `int64()` makes of a peer-wire `uint64` above `MaxInt64`), or an +index whose start multiplication or end addition would overflow — is an +**error before the fetcher is invoked**: a wrapped start would otherwise +recycle to 0 (silently fetching block 0's bytes) or go negative, which +`HTTPFetcher.Fetch` reads as "no Range header" — a one-block fetch degrading +into a whole-object GET. + ### 3.5 `type Coalescing` ```go @@ -281,6 +292,8 @@ go test ./internal/fetch/ -cover -count=1 | `TestUncontendedFetchNotMarkedCoalesced` | an uncontended fetch is not marked coalesced | | `TestFetchBlockPastEndErrors` | a block wholly past the object (or an inverted range) errors without contacting origin | | `TestFetchBlockUnknownSizeTailBlock` | an RFC-clamped 206 on the tail block is accepted and reveals Total | +| **`TestFetchBlockRejectsOverflowGeometry`** | **negative / int64-overflowing block indices (and non-positive block size) error before the fetcher is invoked, size known or unknown (issue #52)** | +| **`TestFetchBlockMaxInt64Window`** | **the largest representable index reaches the fetcher with the exact golden window `[9223372036854771712, MaxInt64]`; ordinary tail clamping unaffected** | ## 8. Limitations & TODO diff --git a/docs/peer.md b/docs/peer.md index 8d31556..c7597a5 100644 --- a/docs/peer.md +++ b/docs/peer.md @@ -28,7 +28,13 @@ X-DART-Hop: (relay depth, for loop safety) ``` - `` is `store.BlockKey.Chunk` in base-16; `` is - `store.BlockKey.Block` in base-10. + `store.BlockKey.Block` in base-10. The index is an **unrestricted uint64 on + the wire**: transport (`parseBlockPath`) accepts any uint64, and it is the + relay-capable Source's job to reject indices that cannot be represented in + signed range geometry before doing arithmetic with them — the engine's + sources decline anything above `chunk.Config.MaxBlockIndex()` with a `404` + (issue #52, see docs/engine.md §3.5). `StoreSource` never does range + arithmetic with the index, so it needs no such check. - `X-DART-Origin` lets a relay-capable Source fetch a block it does not hold (via its own parent/origin); `X-DART-Hop` bounds relay recursion. - Responses: `200` + block bytes (with `X-DART-Node`; `Content-Length` when diff --git a/internal/chunk/chunk.go b/internal/chunk/chunk.go index 4a328fc..38722a5 100644 --- a/internal/chunk/chunk.go +++ b/internal/chunk/chunk.go @@ -18,6 +18,7 @@ package chunk import ( "encoding/binary" "errors" + "math" "net/url" "strings" ) @@ -67,6 +68,22 @@ func (c Config) ChunkOfBlock(blockIndex int64) int64 { return blockIndex / c.Blo // BlockStart returns the absolute byte offset at which the given block begins. func (c Config) BlockStart(blockIndex int64) int64 { return blockIndex * c.BlockSize } +// MaxBlockIndex returns the largest block index whose whole byte range +// [index*BlockSize, index*BlockSize+BlockSize-1] is representable in int64 +// arithmetic. Indices above it wrap: the start-offset multiplication (or the +// end-offset addition) overflows int64, and a wrapped start can silently +// recycle to 0 (fetching the wrong block) or go negative (where an HTTP +// fetcher reads start < 0 as "no Range header", degrading a one-block fetch +// into a whole-object GET — see issue #52). +// +// Block indices arrive from two places: byte offsets of a clamped client +// range (BlockIndex(offset), always <= MaxInt64/BlockSize, which is <= this +// bound for the power-of-two block sizes DART uses) and the peer wire, which +// carries an unrestricted uint64. Callers accepting a peer-supplied index +// must reject anything above this bound before doing range geometry with it. +// Requires BlockSize > 0. +func (c Config) MaxBlockIndex() int64 { return (math.MaxInt64 - c.BlockSize + 1) / c.BlockSize } + // Segment is the intersection of a requested byte range with a single block: // which chunk and block it belongs to, and the absolute inclusive byte range // [From, To] to actually read/serve from that block. diff --git a/internal/chunk/issue52_test.go b/internal/chunk/issue52_test.go new file mode 100644 index 0000000..3841f18 --- /dev/null +++ b/internal/chunk/issue52_test.go @@ -0,0 +1,56 @@ +package chunk + +// Regression test for issue #52: the peer wire carries block indices as an +// unrestricted uint64, and the relay boundary declines anything above +// MaxBlockIndex. Pin the bound against independently computed (arbitrary +// precision) values so it neither re-admits overflow geometry nor rejects +// legitimate tail blocks. + +import ( + "math" + "testing" +) + +// TestMaxBlockIndexGolden pins Config.MaxBlockIndex: the largest block index +// whose whole window [i*BlockSize, i*BlockSize+BlockSize-1] fits in int64. +// Golden values were computed with arbitrary-precision arithmetic (Python): +// +// bs=1 9223372036854775807*1 + 0 = MaxInt64 +// bs=3 3074457345618258601*3 = ...805, + 2 = ...805 <= MaxInt64; next start ...806, end ...808 > MaxInt64 +// bs=16 576460752303423487*16 = ...792, + 15 = MaxInt64 exactly +// bs=4096 2251799813685247*4096 = ...712, + 4095 = MaxInt64 exactly +// bs=4MiB 2199023255551*4194304 = ...504, + 4095*1024-1 = MaxInt64 exactly +// +// In every case the next index's window end exceeds MaxInt64. +func TestMaxBlockIndexGolden(t *testing.T) { + cases := []struct { + blockSize int64 + want int64 + }{ + {1, 9223372036854775807}, + {3, 3074457345618258601}, + {16, 576460752303423487}, + {4096, 2251799813685247}, + {4 * MiB, 2199023255551}, + } + for _, tc := range cases { + c := Config{ChunkSize: tc.blockSize * 2, BlockSize: tc.blockSize} + if got := c.MaxBlockIndex(); got != tc.want { + t.Errorf("BlockSize %d: MaxBlockIndex = %d, want %d", tc.blockSize, got, tc.want) + } + // The pinned index's window must end within int64 (computed in the + // subtraction form so the check itself cannot wrap). + if start := tc.want * tc.blockSize; start > math.MaxInt64-tc.blockSize+1 { + t.Errorf("BlockSize %d: window of the pinned MaxBlockIndex does not fit int64", tc.blockSize) + } + // The next index must NOT fit: its start alone exceeds the largest + // start whose window end stays in range. (When the bound is MaxInt64 + // the next index is not even representable — trivially "does not + // fit"; computing it would wrap this very check.) + if tc.want != math.MaxInt64 { + if next := tc.want + 1; next <= (math.MaxInt64-tc.blockSize+1)/tc.blockSize { + t.Errorf("BlockSize %d: index %d above the pinned bound still fits — the bound is too low", tc.blockSize, next) + } + } + } +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 3323586..9d1c005 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -287,6 +287,25 @@ func (e *Engine) Serve(ctx context.Context, w io.Writer, url string, start, end // maxHop bounds relay recursion depth (loop safety under membership skew). const maxHop = 64 +// relayableBlockIndex reports whether a peer-supplied block index can be +// represented in signed range geometry. The wire carries the index as an +// unrestricted uint64, but block byte offsets are int64 arithmetic +// (fetch.FetchBlock): an index above MaxInt64 wraps negative on conversion, +// and a smaller index whose window start index*BlockSize would exceed MaxInt64 +// wraps in the multiplication. A wrapped start silently recycles to 0 +// (serving the wrong block) or goes negative (where the fetcher omits the +// Range header and a one-block fetch degrades into a whole-object GET — +// issue #52). A legitimate same-config peer never sends such an index: a real +// block of a real object always satisfies index <= chunk.Config.MaxBlockIndex, +// so anything above it is malformed and must be declined before cache lookup, +// relay selection, or origin I/O. +// +// Declined, not errored: the malformed index is the requester's fault, and a +// 500 would charge this healthy relay on the requester's circuit breaker. +func (e *Engine) relayableBlockIndex(b uint64) bool { + return b <= uint64(e.cfg.MaxBlockIndex()) +} + // block returns the bytes of one block: from the local cache, else (with P2P) // from its parent in the distribution tree, else from origin. Fetched bytes are // cached. hop is the relay depth (0 when serving a local client). @@ -429,6 +448,10 @@ func (e *Engine) fileKey(objectID string) uint64 { // P2P node; a store-only node can use peer.StoreSource instead. func (e *Engine) PeerSource() peer.Source { return func(ctx context.Context, req peer.BlockRequest) ([]byte, bool, error) { + if !e.relayableBlockIndex(req.Key.Block) { + e.mx.recordRelay(false) + return nil, false, nil // malformed wire index: decline cheaply + } if data, ok, err := e.store.Get(req.Key); err != nil { return nil, false, err } else if ok { diff --git a/internal/engine/issue52_test.go b/internal/engine/issue52_test.go new file mode 100644 index 0000000..a3e4b5f --- /dev/null +++ b/internal/engine/issue52_test.go @@ -0,0 +1,142 @@ +package engine + +// Regression tests for issue #52: the peer wire carries the block index as an +// unrestricted uint64, but relay geometry is int64 arithmetic. Indices that +// cannot be represented — above MaxInt64, or whose window start +// index*BlockSize would overflow int64 — must be declined at the relay +// boundary, before cache lookup, relay selection, size probe, or origin I/O. +// Before the guard the wrapped start either recycled to 0 (a 2^63 index was +// served block 0's bytes, held=true) or went negative (the fetcher omitted +// the Range header and pulled the whole object — the amplification shape of +// issue #15, re-entering via the peer path). + +import ( + "bytes" + "context" + "math" + "sync/atomic" + "testing" + + "github.com/data-accelerator/dart/internal/peer" + "github.com/data-accelerator/dart/internal/store" +) + +// malformedWireIndices cannot be represented in signed range geometry under +// testCfg() (BlockSize 16, chunk.MaxBlockIndex 576460752303423487). The last +// entry is the first index whose window overflows: 576460752303423488*16 = +// 2^63 (independently computed, see chunk.TestMaxBlockIndexGolden). +var malformedWireIndices = []struct { + name string + block uint64 +}{ + {"2^63 wraps to MinInt64 on int64 conversion", 1 << 63}, + {"2^64-1 wraps to -1", math.MaxUint64}, + {"first index whose byte window overflows int64", 576460752303423488}, +} + +// TestPeerSourceRejectsMalformedBlockIndex: the buffered relay source must +// decline a malformed wire index cheaply — no error (the fault is the +// requester's; a 500 would charge this healthy relay on the requester's +// circuit breaker), no origin contact (not even the size probe), no cache +// pollution. +func TestPeerSourceRejectsMalformedBlockIndex(t *testing.T) { + content := blob(100) + var reqs int64 + origin := countingOrigin(t, content, &reqs) + + st := openStoreAt(t) + e, err := New(Options{Chunk: testCfg(), Store: st, Fetcher: newFetcher()}) + if err != nil { + t.Fatalf("New: %v", err) + } + src := e.PeerSource() + + for _, tc := range malformedWireIndices { + atomic.StoreInt64(&reqs, 0) + key := store.BlockKey{Chunk: 0, Block: tc.block} + data, held, err := src(context.Background(), + peer.BlockRequest{Key: key, URL: origin.URL, Hop: 1}) + if err != nil || held || data != nil { + t.Errorf("%s: data=%d bytes held=%v err=%v, want a cheap decline", tc.name, len(data), held, err) + } + if got := atomic.LoadInt64(&reqs); got != 0 { + t.Errorf("%s: origin contacted %d times for a malformed index (size probe or fetch)", tc.name, got) + } + if st.Has(key) { + t.Errorf("%s: a malformed-index block entered the cache", tc.name) + } + } + + // Control: a legitimate relay request still fetches and serves block 1. + atomic.StoreInt64(&reqs, 0) + key := store.BlockKey{Chunk: blockKeyFor(origin.URL).Chunk, Block: 1} + data, held, err := src(context.Background(), + peer.BlockRequest{Key: key, URL: origin.URL, Hop: 1}) + if err != nil || !held { + t.Fatalf("legitimate relay: held=%v err=%v", held, err) + } + if !bytes.Equal(data, content[16:32]) { + t.Fatalf("legitimate relay served %d wrong bytes", len(data)) + } + if atomic.LoadInt64(&reqs) == 0 { + t.Fatal("control: the legitimate relay must actually reach origin") + } +} + +// TestPeerStreamSourceRejectsMalformedBlockIndex: same contract on the +// cut-through path — decline, nothing streamed, no size announced, no origin +// contact. +func TestPeerStreamSourceRejectsMalformedBlockIndex(t *testing.T) { + content := blob(100) + var reqs int64 + origin := countingOrigin(t, content, &reqs) + + st := openStoreAt(t) + e, err := New(Options{Chunk: testCfg(), Store: st, Fetcher: newFetcher()}) + if err != nil { + t.Fatalf("New: %v", err) + } + src := e.PeerStreamSource() + + for _, tc := range malformedWireIndices { + atomic.StoreInt64(&reqs, 0) + key := store.BlockKey{Chunk: 0, Block: tc.block} + var buf bytes.Buffer + var sized int64 + n, ok, err := src(context.Background(), + peer.BlockRequest{Key: key, URL: origin.URL, Hop: 1}, + &buf, func(m int64) { sized = m }) + if err != nil || ok || n != 0 { + t.Errorf("%s: n=%d ok=%v err=%v, want a cheap decline", tc.name, n, ok, err) + } + if buf.Len() != 0 { + t.Errorf("%s: %d bytes streamed for a malformed index", tc.name, buf.Len()) + } + if sized != 0 { + t.Errorf("%s: sizer called with %d for a malformed index", tc.name, sized) + } + if got := atomic.LoadInt64(&reqs); got != 0 { + t.Errorf("%s: origin contacted %d times for a malformed index (size probe or fetch)", tc.name, got) + } + if st.Has(key) { + t.Errorf("%s: a malformed-index block entered the cache", tc.name) + } + } + + // Control: a legitimate relay request still streams block 1. + atomic.StoreInt64(&reqs, 0) + key := store.BlockKey{Chunk: blockKeyFor(origin.URL).Chunk, Block: 1} + var buf bytes.Buffer + n, ok, err := src(context.Background(), + peer.BlockRequest{Key: key, URL: origin.URL, Hop: 1}, + &buf, func(int64) {}) + if err != nil || !ok { + t.Fatalf("legitimate stream relay: ok=%v err=%v", ok, err) + } + if n != 16 || !bytes.Equal(buf.Bytes(), content[16:32]) { + t.Fatalf("legitimate stream relay wrote %d wrong bytes", n) + } + if atomic.LoadInt64(&reqs) == 0 { + t.Fatal("control: the legitimate relay must actually reach origin") + } +} diff --git a/internal/engine/stream.go b/internal/engine/stream.go index 41093e3..a2ba6b4 100644 --- a/internal/engine/stream.go +++ b/internal/engine/stream.go @@ -54,6 +54,10 @@ func asRelayError(err error) error { // rather than depth × block-transfer time. func (e *Engine) PeerStreamSource() peer.StreamSource { return func(ctx context.Context, req peer.BlockRequest, w io.Writer, sizer func(int64)) (int64, bool, error) { + if !e.relayableBlockIndex(req.Key.Block) { + e.mx.recordRelay(false) + return 0, false, nil // malformed wire index: decline cheaply + } // Local hit: serve from the store. Get copies the block out under the // store lock, so a concurrent eviction that reuses the slot cannot tear the // bytes while they stream to the peer (a slot-backed GetReader could). The diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index df709bb..fede29b 100644 --- a/internal/fetch/fetch.go +++ b/internal/fetch/fetch.go @@ -15,6 +15,7 @@ import ( "errors" "fmt" "io" + "math" "net/http" "net/url" "strconv" @@ -308,7 +309,21 @@ func startFromContentRange(v string) (int64, bool) { // > 0 the final (tail) block's end is clamped to size-1; when size <= 0 the // natural block range is requested and the returned Range.Total may reveal the // size. +// +// blockIndex must be non-negative and small enough that the whole block window +// [blockIndex*blockSize, blockIndex*blockSize+blockSize-1] is representable in +// int64 (the same bound chunk.Config.MaxBlockIndex states in grid terms). +// Anything else is a caller bug — on the peer relay path it is a malformed +// wire index, since the protocol carries the index as an unrestricted uint64: +// the int64 conversion or the start-offset multiplication would wrap, and a +// wrapped start either recycles to 0 (silently fetching the wrong block) or +// goes negative, where Fetch reads start < 0 as "no Range header" and a +// one-block fetch degrades into a whole-object GET (issue #52). Such geometry +// is rejected here, before any origin I/O. func FetchBlock(ctx context.Context, f Fetcher, url string, blockSize, blockIndex, size int64) (Range, error) { + if blockSize <= 0 || blockIndex < 0 || blockIndex > (math.MaxInt64-blockSize+1)/blockSize { + return Range{}, fmt.Errorf("fetch: %s: block index %d out of range for block size %d", Redact(url), blockIndex, blockSize) + } start := blockIndex * blockSize end := start + blockSize - 1 if size > 0 { diff --git a/internal/fetch/issue52_test.go b/internal/fetch/issue52_test.go new file mode 100644 index 0000000..864e9c6 --- /dev/null +++ b/internal/fetch/issue52_test.go @@ -0,0 +1,90 @@ +package fetch + +// Regression tests for issue #52: block geometry that cannot be represented +// in int64 must be rejected before the fetcher is invoked. Before the guard, +// a peer-wire index above MaxInt64 (converted with int64(req.Key.Block)) or +// whose start-offset multiplication overflowed produced a wrapped start that +// either recycled to 0 — silently fetching block 0's bytes for a nonsensical +// index — or went negative, which HTTPFetcher.Fetch reads as "no Range +// header": a one-block fetch degrading into a whole-object GET. + +import ( + "context" + "math" + "strings" + "testing" +) + +// captureFetcher records the requested window instead of performing I/O, so a +// test can prove the overflow guard fires before any origin contact. +type captureFetcher struct { + calls int + start, end int64 +} + +func (f *captureFetcher) Fetch(_ context.Context, _ string, start, end int64) (Range, error) { + f.calls++ + f.start, f.end = start, end + return Range{Data: make([]byte, end-start+1), Total: -1}, nil +} + +// TestFetchBlockRejectsOverflowGeometry: negative indices (the result of +// int64() on a wire value above MaxInt64) and positive indices whose window +// overflows int64 must error, with size both known and unknown, and must +// never reach the fetcher. The boundary values are the independently computed +// MaxBlockIndex+1 for each block size (see chunk.TestMaxBlockIndexGolden). +func TestFetchBlockRejectsOverflowGeometry(t *testing.T) { + cases := []struct { + name string + blockSize, index int64 + }{ + {"uint64 2^63 converts to MinInt64", 4096, math.MinInt64}, + {"uint64 2^64-1 converts to -1", 4096, -1}, + {"MaxInt64: start multiplication wraps negative", 4096, math.MaxInt64}, + {"2^52 * 4096 wraps to 0", 4096, 1 << 52}, + {"first index past the int64 window, block size 4096", 4096, 2251799813685248}, + {"first index past the int64 window, block size 16", 16, 576460752303423488}, + {"non-positive block size", 0, 5}, + } + for _, tc := range cases { + for _, size := range []int64{4500, -1} { // size probed / unknown + f := &captureFetcher{} + _, err := FetchBlock(context.Background(), f, "http://origin/blob", tc.blockSize, tc.index, size) + if err == nil { + t.Errorf("%s (size %d): want an error, got none", tc.name, size) + } else if !strings.Contains(err.Error(), "out of range") { + t.Errorf("%s (size %d): error %q does not identify the cause", tc.name, size, err) + } + if f.calls != 0 { + t.Errorf("%s (size %d): fetcher invoked %d times; overflow geometry must not reach origin", tc.name, size, f.calls) + } + } + } +} + +// TestFetchBlockMaxInt64Window pins the valid side of the boundary: the +// largest representable index must reach the fetcher with the exact window +// [index*blockSize, MaxInt64] — no wrap, no spurious error. Golden values +// (arbitrary precision): 2251799813685247*4096 = 9223372036854771712, and +// +4095 = math.MaxInt64 exactly. +func TestFetchBlockMaxInt64Window(t *testing.T) { + f := &captureFetcher{} + if _, err := FetchBlock(context.Background(), f, "http://origin/blob", 4096, 2251799813685247, -1 /* size unknown: no tail clamp */); err != nil { + t.Fatalf("largest representable block: %v", err) + } + if f.calls != 1 { + t.Fatalf("fetcher calls = %d, want 1", f.calls) + } + if f.start != 9223372036854771712 || f.end != math.MaxInt64 { + t.Fatalf("window = [%d, %d], want [9223372036854771712, %d]", f.start, f.end, int64(math.MaxInt64)) + } + + // Control: ordinary tail-block clamping is unaffected by the guard. + f2 := &captureFetcher{} + if _, err := FetchBlock(context.Background(), f2, "http://origin/blob", 4096, 1, 4500); err != nil { + t.Fatalf("tail block: %v", err) + } + if f2.calls != 1 || f2.start != 4096 || f2.end != 4499 { + t.Fatalf("tail window = [%d, %d] (calls %d), want [4096, 4499] (calls 1)", f2.start, f2.end, f2.calls) + } +}