Skip to content

fix: guard sliceDirect against out-of-bounds offsets#2329

Merged
djeebus merged 3 commits intomainfrom
joe/cache-slice-direct-bounds
Apr 8, 2026
Merged

fix: guard sliceDirect against out-of-bounds offsets#2329
djeebus merged 3 commits intomainfrom
joe/cache-slice-direct-bounds

Conversation

@joe-lombrozo-s-bot
Copy link
Copy Markdown
Contributor

Resolves the review comment on #2306 about missing an out-of-bounds guard.

Changes:

  • return when or
  • avoid mmap slice panics for invalid offsets
  • add a focused regression test covering out-of-range offsets

Branch was created from fresh after pulling latest.

Add the missing bounds check in sliceDirect so off >= c.size (or negative)
returns BytesNotAvailableError instead of panicking on the mmap slice.
Also add a focused regression test.
@djeebus djeebus marked this pull request as ready for review April 8, 2026 18:34
Copy link
Copy Markdown
Contributor

@levb levb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending the tests

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — simple, focused bounds guard with a regression test.

Extended reasoning...

Overview

Adds a 4-line out-of-bounds guard to sliceDirect in cache.go, returning BytesNotAvailableError{} when off < 0 || off >= c.size. Paired with a targeted regression test in cache_test.go.

Security risks

None. The change is purely defensive — it prevents a potential mmap slice panic for callers that pass an out-of-range offset, and it matches the error type already used elsewhere in the file.

Level of scrutiny

Low. This is a minimal, single-concern fix that follows the established pattern used by the Slice method (which guards via isCached). The logic is trivial and the test directly validates both the boundary case (off == size) and a beyond-boundary case (off > size).

Other factors

No bugs found by the bug hunting system. No outstanding reviewer comments. The fix resolves a previously noted review comment on #2306 and the branch is up to date with main.

@djeebus
Copy link
Copy Markdown
Contributor

djeebus commented Apr 8, 2026

The failure is a race condition introduced by another PR. fied in #2330

@djeebus djeebus merged commit 393d0af into main Apr 8, 2026
43 checks passed
@djeebus djeebus deleted the joe/cache-slice-direct-bounds branch April 8, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants