engine/fetch/chunk: reject peer block indices that overflow signed range geometry (fixes #52)

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<start return nil |

docs/engine.md

@@ -160,6 +160,18 @@ block it does not hold, it fetches the block via its own tree parent/origin
 makes intermediate tree nodes offload the owner. A store-only node can use
 `peer.StoreSource` instead (no relay).
 
+**Malformed block indices are declined, not relayed (issue #52).** The wire
+carries the block index as an unrestricted `uint64`, while block geometry is
+int64 arithmetic. An index above `chunk.Config.MaxBlockIndex()` — above
+`MaxInt64`, or large enough that `index*BlockSize` would overflow int64 — can
+never come from a legitimate same-config peer, and computing with it would wrap
+(wrapped-to-0 start silently serves block 0's bytes; a wrapped-negative start
+drops the Range header and pulls the whole object). `PeerSource` and
+`PeerStreamSource` decline such requests (`held=false`, recorded as a refused
+relay) **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.
+
 ### 3.6 `func (e *Engine) PeerStreamSource() peer.StreamSource`
 
 The **cut-through** counterpart of `PeerSource`, for a `peer.StreamServer`:
@@ -170,6 +182,9 @@ The **cut-through** counterpart of `PeerSource`, for a `peer.StreamServer`:
   requester **as they arrive** while tee-ing them into the local cache;
 - the tree root (no parent) fetches from origin, caches, then streams.
 
+The malformed-index decline of §3.5 applies identically here: nothing is
+streamed, the sizer is not called, and no origin request is made.
+
 This is what `cmd/dart` mounts, so a multi-hop chain pipelines: the tail node
 starts receiving after roughly one block-transfer time instead of
 depth x block-transfer time.
@@ -305,6 +320,12 @@ requests; their bytes are counted as both `client` and `origin_in` wire bytes
    An empty object is a valid `200` with `Content-Length: 0`; a Range request
    on it is `416` (`bytes */0`), suffix ranges included.
 4. **Range semantics**: standard `200`/`206`/`416` with correct `Content-Range`.
+5. **No wrapped geometry on the relay path**: a peer-supplied block index that
+   cannot be represented in int64 range arithmetic
+   (`> 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
 

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
 

docs/peer.md

@@ -28,7 +28,13 @@ X-DART-Hop:    <n>              (relay depth, for loop safety)
 ```
 
 - `<chunkKey-hex>` is `store.BlockKey.Chunk` in base-16; `<blockIndex>` 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

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.

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)
+			}
+		}
+	}
+}

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 {

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")
+	}
+}