Skip to content

Reposition the temp file when seeking to offset zero - #363

Open
youdie006 wants to merge 1 commit into
C2FO:mainfrom
youdie006:os-seek-start-overwrite
Open

Reposition the temp file when seeking to offset zero#363
youdie006 wants to merge 1 commit into
C2FO:mainfrom
youdie006:os-seek-start-overwrite

Conversation

@youdie006

Copy link
Copy Markdown

On the os backend, Seek(0, io.SeekStart) followed by Write appends instead of overwriting from
the start. mem gets it right.

os   Seek(0,0)+Write("HELLO") -> "hello worldHELLO"   want "HELLO world"   FAIL
os   Seek(6,0)+Write("there") -> "hello there"        want "hello there"   PASS

mem  Seek(0,0)+Write("HELLO") -> "HELLO world"
mem  Seek(6,0)+Write("there") -> "hello there"

Offset 6 is the case the conformance suite already covers, which is why this has gone unnoticed.

Cause

copyToLocalTempReader buffers writes into a temp file. When the file exists and Seek/Read ran
first, it copies the original in with io.Copy — leaving the temp file's cursor at EOF — and then
repositions, but only when the cursor is past the start (backend/os/file.go:512):

if f.cursorPos > 0 {
    if _, err := tmpFile.Seek(f.cursorPos, 0); err != nil {

For cursorPos == 0 the reposition is skipped, so the write lands at EOF.

Change

Drop the > 0 guard — the seek is needed for offset zero precisely because io.Copy moved the
cursor.

Tests

Two, both red with only backend/os/file.go reverted:

  • A case in the shared DefaultIOTestCases, so mem defines the expected value rather than me
    asserting it. Only os fails:
    --- FAIL: TestIOConformance/Seek_to_start,_Write,_Close,_file_exists
        Seek to start, Write, Close, file exists: expected results that text but got some textthat
    
    mem passes it before and after.
  • A table-driven case in the osFileTest suite covering offset 0 and offset 6, since the
    conformance run is behind the vfsintegration tag and would not count toward the coverage gate.

go test ./backend/os/ ./backend/mem/ ./backend/testsuite/ is green, as is
go test -tags vfsintegration -run TestIOConformance ./backend/os/ ./backend/mem/. gofmt -l lists
none of the changed files. CHANGELOG entry added under Unreleased.

Scope

sftp and azure delegate Seek straight through and may share this shape, but I could not
exercise them without credentials, so this PR is limited to os.

Separately and not included: os, azure and sftp do not validate whence (Linux accepts 3
for SEEK_DATA) while mem, s3, gs and ftp return vfs.ErrSeekInvalidWhence
(errors.go:25). Happy to send that separately if you want it.


Disclosure: prepared with AI assistance; I verified the backend comparison and both red/green runs
myself.

copyToLocalTempReader copies the existing file in with io.Copy, which
leaves the temp file at EOF, then seeks back only when cursorPos > 0. So
Seek(0, io.SeekStart) followed by Write appended instead of overwriting
from the start. The mem backend already behaves correctly.
@c2fo-cibot c2fo-cibot Bot added the size/M Denotes a PR that changes 30-99 lines label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant