refactor: consistent range

Author: alanshaw

DESIGN_NOTES.md

@@ -102,7 +102,7 @@ but unwired. The full trace is the
 
 **Encrypted blobs** (the FilOne encryption design's read side) decrypt inside
 the per-blob open, leaving every other read-path value plaintext: the
-manifest's `BlobRef.Offset/Length`, `Body.Size`, ETag and Content-Length are
+manifest's `BlobRef.Start/End`, `Body.Size`, ETag and Content-Length are
 plaintext coordinates, while `BlobRef.Digest` names the stored FEE envelope.
 A `blob_encryption_params` row marks a blob encrypted and carries what its
 decryptor needs; the read unwraps the region-wrapped CEK through

blockstore/cache.go

@@ -70,11 +70,11 @@ func (c *Cached) OpenBlob(ctx context.Context, space did.DID, digest mh.Multihas
 	return nil, ErrNotFound
 }
 
-// OpenBlobRange streams stored bytes [off, off+n) of a body blob straight
+// OpenBlobRange streams stored bytes [start, end] (inclusive) of a body blob straight
 // from the base reader, bypassing the LRU exactly like OpenBlob.
-func (c *Cached) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, error) {
+func (c *Cached) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, error) {
 	if br, ok := c.base.(BlobReader); ok {
-		return OpenBlobRangeOf(ctx, br, space, digest, off, n)
+		return OpenBlobRangeOf(ctx, br, space, digest, start, end)
 	}
 	return nil, ErrNotFound
 }

blockstore/forge.go

@@ -1,7 +1,6 @@
 package blockstore
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -146,19 +145,19 @@ func NewForge(cfg ForgeConfig) (*Forge, error) {
 // empty 200 when it can't resolve a did:plc principal in the proof chain)
 // otherwise surfaces only as a bare client-side EOF mid-body, with nothing
 // attributable in ingot's own logs.
-// off/n select a sub-range of the located bytes: stored bytes [off, off+n)
-// of the blob. n < 0 retrieves the locator's whole range (the entire blob,
+// start/end select an inclusive sub-range of the located bytes (HTTP Range
+// semantics). end < 0 retrieves the locator's whole range (the entire blob,
 // for a body blob).
-func (f *Forge) retrieve(ctx context.Context, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, int64, error) {
-	rc, got, err := f.doRetrieve(ctx, space, digest, off, n)
+func (f *Forge) retrieve(ctx context.Context, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, int64, error) {
+	rc, got, err := f.doRetrieve(ctx, space, digest, start, end)
 	if err != nil && !errors.Is(err, ErrNotFound) {
 		f.logger.Warn("forge: retrieve failed",
 			zap.Stringer("space", space), zap.String("digest", digestutil.Format(digest)), zap.Error(err))
 	}
 	return rc, got, err
 }
 
-func (f *Forge) doRetrieve(ctx context.Context, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, int64, error) {
+func (f *Forge) doRetrieve(ctx context.Context, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, int64, error) {
 	locations, err := f.locator.Locate(ctx, []did.DID{space}, digest)
 	if err != nil {
 		var nf locator.NotFoundError
@@ -178,21 +177,21 @@ func (f *Forge) doRetrieve(ctx context.Context, space did.DID, digest mh.Multiha
 	}
 	target := cm.Location[0]
 
-	// Narrow the located range to the caller's sub-range. loc.Range is
-	// inclusive and, for a whole blob, starts at 0 — so the sub-range is
-	// [Start+off, Start+off+n-1], clamped to the located end. The decrypting
-	// read path uses this to fetch only the ciphertext chunks it needs.
-	if n >= 0 {
-		start := loc.Range.Start + off
-		if start > loc.Range.End || n == 0 {
-			// Nothing to fetch; an empty body keeps the contract simple.
-			return io.NopCloser(bytes.NewReader(nil)), 0, nil
+	// Narrow the located range to the caller's inclusive sub-range. loc.Range
+	// is inclusive too and, for a whole blob, starts at 0 — so the sub-range
+	// maps to [Range.Start+start, Range.Start+end], with the end clamped to
+	// the located end (HTTP Range semantics). The decrypting read path uses
+	// this to fetch only the ciphertext chunks it needs.
+	if end >= 0 {
+		first := loc.Range.Start + start
+		if start < 0 || end < start || first > loc.Range.End {
+			return nil, 0, fmt.Errorf("forge: range [%d,%d] of %s is unsatisfiable", start, end, digestutil.Format(digest))
 		}
-		end := start + n - 1
-		if end > loc.Range.End {
-			end = loc.Range.End
+		last := loc.Range.Start + end
+		if last > loc.Range.End {
+			last = loc.Range.End
 		}
-		loc.Range = blobindex.Range{Start: start, End: end}
+		loc.Range = blobindex.Range{Start: first, End: last}
 	}
 
 	// The commitment's space scopes the retrieve capability; fall back to
@@ -286,11 +285,12 @@ func (f *Forge) OpenBlob(ctx context.Context, space did.DID, digest mh.Multihash
 	return rc, err
 }
 
-// OpenBlobRange streams stored bytes [off, off+n) of an object-body blob from
-// piri — a ranged /content/retrieve, so the decrypting read path fetches only
-// the ciphertext chunks a plaintext range needs rather than the whole blob.
-func (f *Forge) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, error) {
-	rc, _, err := f.retrieve(ctx, space, digest, off, n)
+// OpenBlobRange streams stored bytes [start, end] (inclusive) of an
+// object-body blob from piri — a ranged /content/retrieve, so the decrypting
+// read path fetches only the ciphertext chunks a plaintext range needs
+// rather than the whole blob.
+func (f *Forge) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, error) {
+	rc, _, err := f.retrieve(ctx, space, digest, start, end)
 	return rc, err
 }
 

blockstore/layered.go

@@ -94,13 +94,13 @@ func (l *Layered) OpenBlob(ctx context.Context, space did.DID, digest mh.Multiha
 	return nil, ErrNotFound
 }
 
-// OpenBlobRange streams stored bytes [off, off+n) of a body blob with the
+// OpenBlobRange streams stored bytes [start, end] (inclusive) of a body blob with the
 // same tiering as OpenBlob: spool first, then the network base. A tier that
 // implements only BlobReader is served through OpenBlob with the prefix
 // discarded (OpenBlobRangeOf).
-func (l *Layered) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, error) {
+func (l *Layered) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, error) {
 	if br, ok := l.spool.(BlobReader); ok {
-		rc, err := OpenBlobRangeOf(ctx, br, space, digest, off, n)
+		rc, err := OpenBlobRangeOf(ctx, br, space, digest, start, end)
 		if err == nil {
 			return rc, nil
 		}
@@ -109,7 +109,7 @@ func (l *Layered) OpenBlobRange(ctx context.Context, space did.DID, digest mh.Mu
 		}
 	}
 	if br, ok := l.base.(BlobReader); ok {
-		return OpenBlobRangeOf(ctx, br, space, digest, off, n)
+		return OpenBlobRangeOf(ctx, br, space, digest, start, end)
 	}
 	return nil, ErrNotFound
 }

blockstore/openblob_test.go

@@ -80,8 +80,9 @@ func TestCached_OpenBlob_BypassesCache(t *testing.T) {
 	}
 }
 
-// TestSpool_OpenBlobRange: the section reader serves exactly [off, off+n) of
-// the spooled file, and a range past the end yields a shorter stream.
+// TestSpool_OpenBlobRange: the section reader serves exactly the inclusive
+// [start, end] of the spooled file, and an end past the file's end yields a
+// shorter stream.
 func TestSpool_OpenBlobRange(t *testing.T) {
 	ctx := context.Background()
 	sp, err := NewSpool(t.TempDir())
@@ -95,18 +96,18 @@ func TestSpool_OpenBlobRange(t *testing.T) {
 	}
 
 	cases := []struct {
-		name   string
-		off, n int64
-		want   string
+		name       string
+		start, end int64
+		want       string
 	}{
-		{"interior", 4, 6, "456789"},
-		{"prefix", 0, 3, "012"},
-		{"suffix past end clamps", 12, 100, "cdef"},
-		{"empty", 5, 0, ""},
+		{"interior", 4, 9, "456789"},
+		{"prefix", 0, 2, "012"},
+		{"single byte", 15, 15, "f"},
+		{"end past file clamps", 12, 100, "cdef"},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			rc, err := sp.OpenBlobRange(ctx, did.Undef, digest, c.off, c.n)
+			rc, err := sp.OpenBlobRange(ctx, did.Undef, digest, c.start, c.end)
 			if err != nil {
 				t.Fatalf("OpenBlobRange: %v", err)
 			}
@@ -116,7 +117,7 @@ func TestSpool_OpenBlobRange(t *testing.T) {
 				t.Fatalf("read: %v", err)
 			}
 			if string(got) != c.want {
-				t.Fatalf("range [%d,+%d) = %q, want %q", c.off, c.n, got, c.want)
+				t.Fatalf("range [%d,%d] = %q, want %q", c.start, c.end, got, c.want)
 			}
 		})
 	}
@@ -132,7 +133,7 @@ func TestOpenBlobRangeOf_Fallback(t *testing.T) {
 	ctx := context.Background()
 	tier := fakeBlobTier{data: []byte("0123456789abcdef")}
 
-	rc, err := OpenBlobRangeOf(ctx, tier, did.Undef, nil, 4, 6)
+	rc, err := OpenBlobRangeOf(ctx, tier, did.Undef, nil, 4, 9)
 	if err != nil {
 		t.Fatalf("OpenBlobRangeOf: %v", err)
 	}

blockstore/spool.go

@@ -116,22 +116,19 @@ func (s *Spool) OpenBlob(_ context.Context, _ did.DID, digest mh.Multihash) (io.
 	return f, nil
 }
 
-// OpenBlobRange returns a reader over stored bytes [off, off+n) of the
-// spooled blob, or ErrNotFound — OpenBlob restricted to a section, for the
-// decrypting read path, which fetches only the ciphertext span a plaintext
-// range needs. A range past the file's end yields a shorter stream.
-func (s *Spool) OpenBlobRange(_ context.Context, _ did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, error) {
-	if off < 0 || n < 0 {
-		return nil, fmt.Errorf("blockstore: invalid blob range [%d, +%d)", off, n)
-	}
+// OpenBlobRange returns a reader over stored bytes [start, end] (inclusive)
+// of the spooled blob, or ErrNotFound — OpenBlob restricted to a section,
+// for the decrypting read path, which fetches only the ciphertext span a
+// plaintext range needs. An end past the file's end yields a shorter stream.
+func (s *Spool) OpenBlobRange(_ context.Context, _ did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, error) {
 	f, err := os.Open(s.Path(digest))
 	if errors.Is(err, os.ErrNotExist) {
 		return nil, ErrNotFound
 	}
 	if err != nil {
 		return nil, fmt.Errorf("blockstore: spool open %s: %w", digest.B58String(), err)
 	}
-	return readerCloser{Reader: io.NewSectionReader(f, off, n), Closer: f}, nil
+	return readerCloser{Reader: io.NewSectionReader(f, start, end-start+1), Closer: f}, nil
 }
 
 // GetBlock returns the blob stored under c's multihash, or ErrNotFound. A miss

blockstore/store.go

@@ -106,43 +106,42 @@ type BlobReader interface {
 }
 
 // BlobRangeReader is the optional ranged counterpart of BlobReader: it
-// streams only the stored bytes [off, off+n) of a blob. The decrypting read
-// path uses it to fetch exactly the ciphertext chunks a plaintext range
-// needs instead of the whole blob. Returns ErrNotFound if the blob is not in
-// this tier; a range past the stored bytes yields a shorter stream, which
-// the caller detects. A tier without this capability is served through
-// OpenBlob with the prefix discarded (see OpenBlobRangeOf).
+// streams only the stored bytes [start, end] (inclusive, HTTP Range
+// semantics) of a blob. The decrypting read path uses it to fetch exactly
+// the ciphertext chunks a plaintext range needs instead of the whole blob.
+// Returns ErrNotFound if the blob is not in this tier; an end past the
+// stored bytes yields a shorter stream, which the caller detects. A tier
+// without this capability is served through OpenBlob with the prefix
+// discarded (see OpenBlobRangeOf).
 type BlobRangeReader interface {
-	OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, error)
+	OpenBlobRange(ctx context.Context, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, error)
 }
 
-// OpenBlobRangeOf opens stored bytes [off, off+n) of a blob through br,
-// using its BlobRangeReader capability when present and falling back to
-// OpenBlob with the prefix read-and-discarded (or Seek'd, for a spool file)
-// when not. It is how range-consuming callers stay correct over any tier.
-func OpenBlobRangeOf(ctx context.Context, br BlobReader, space did.DID, digest mh.Multihash, off, n int64) (io.ReadCloser, error) {
-	if off < 0 || n < 0 {
-		return nil, fmt.Errorf("blockstore: invalid blob range [%d, +%d)", off, n)
-	}
+// OpenBlobRangeOf opens stored bytes [start, end] (inclusive) of a blob
+// through br, using its BlobRangeReader capability when present and falling
+// back to OpenBlob with the prefix read-and-discarded (or Seek'd, for a
+// spool file) when not. It is how range-consuming callers stay correct over
+// any tier.
+func OpenBlobRangeOf(ctx context.Context, br BlobReader, space did.DID, digest mh.Multihash, start, end int64) (io.ReadCloser, error) {
 	if rr, ok := br.(BlobRangeReader); ok {
-		return rr.OpenBlobRange(ctx, space, digest, off, n)
+		return rr.OpenBlobRange(ctx, space, digest, start, end)
 	}
 	rc, err := br.OpenBlob(ctx, space, digest)
 	if err != nil {
 		return nil, err
 	}
-	if off > 0 {
+	if start > 0 {
 		if seeker, ok := rc.(io.Seeker); ok {
-			if _, err := seeker.Seek(off, io.SeekStart); err != nil {
+			if _, err := seeker.Seek(start, io.SeekStart); err != nil {
 				_ = rc.Close()
-				return nil, fmt.Errorf("blockstore: seek blob to %d: %w", off, err)
+				return nil, fmt.Errorf("blockstore: seek blob to %d: %w", start, err)
 			}
-		} else if _, err := io.CopyN(io.Discard, rc, off); err != nil {
+		} else if _, err := io.CopyN(io.Discard, rc, start); err != nil {
 			_ = rc.Close()
-			return nil, fmt.Errorf("blockstore: skip into blob to %d: %w", off, err)
+			return nil, fmt.Errorf("blockstore: skip into blob to %d: %w", start, err)
 		}
 	}
-	return readerCloser{Reader: io.LimitReader(rc, n), Closer: rc}, nil
+	return readerCloser{Reader: io.LimitReader(rc, end-start+1), Closer: rc}, nil
 }
 
 // readerCloser pairs a wrapped reader with the closer releasing its