Skip to content

fix(internal/utils): keep byteReader position after failed seeks - #1005

Merged
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/byte-reader-position
Jul 27, 2026
Merged

fix(internal/utils): keep byteReader position after failed seeks#1005
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/byte-reader-position

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

byteReader updated its cached position even when the underlying seek failed, which could make Peek and Read disagree about where the reader was. Discard also returned io.EOF when it consumed exactly the remaining bytes.

This only updates the cached position after a successful seek and treats exact exhaustion as a successful discard. Added focused regressions for both behaviors.

Tests: go test ./internal/utils

@fallintoplace
fallintoplace requested a review from zeroshade as a code owner July 24, 2026 23:32
@fallintoplace fallintoplace changed the title fix(internal/utils): preserve byte reader position fix(internal/utils): keep byteReader state consistent on seek and discard Jul 25, 2026
@fallintoplace fallintoplace changed the title fix(internal/utils): keep byteReader state consistent on seek and discard fix(internal/utils): preserve byteReader state after failed seeks Jul 25, 2026
@fallintoplace fallintoplace changed the title fix(internal/utils): preserve byteReader state after failed seeks fix(internal/utils): keep byteReader position after failed seeks Jul 25, 2026

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verified both fixes against current main:

  1. Seek unconditionally did r.pos = int(pos). Since bytes.Reader.Seek returns pos = 0 on failure (invalid whence or negative resulting position), a failed seek corrupted the tracked position to 0 while the underlying reader stayed put — leaving Peek and Read disagreeing about the location. Guarding on err == nil is correct.

  2. Discard used newPos >= len(r.buf), so discarding exactly the remaining bytes reported io.EOF even though all n bytes were successfully discarded. > is right and matches both stdlib bufio.Reader.Discard semantics and the sibling bufferedReader.Discard in this same file.

No regression risk on the EOF change: the only caller (page_reader.go) ignores Discard's return values. Both tests distinguish pre- from post-fix behavior. LGTM.

@zeroshade
zeroshade merged commit d8b6170 into apache:main Jul 27, 2026
23 checks passed
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.

2 participants