Reposition the temp file when seeking to offset zero - #363
Open
youdie006 wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On the
osbackend,Seek(0, io.SeekStart)followed byWriteappends instead of overwriting fromthe start.
memgets it right.Offset 6 is the case the conformance suite already covers, which is why this has gone unnoticed.
Cause
copyToLocalTempReaderbuffers writes into a temp file. When the file exists andSeek/Readranfirst, it copies the original in with
io.Copy— leaving the temp file's cursor at EOF — and thenrepositions, but only when the cursor is past the start (
backend/os/file.go:512):For
cursorPos == 0the reposition is skipped, so the write lands at EOF.Change
Drop the
> 0guard — the seek is needed for offset zero precisely becauseio.Copymoved thecursor.
Tests
Two, both red with only
backend/os/file.goreverted:DefaultIOTestCases, somemdefines the expected value rather than measserting it. Only
osfails:mempasses it before and after.osFileTestsuite covering offset 0 and offset 6, since theconformance run is behind the
vfsintegrationtag and would not count toward the coverage gate.go test ./backend/os/ ./backend/mem/ ./backend/testsuite/is green, as isgo test -tags vfsintegration -run TestIOConformance ./backend/os/ ./backend/mem/.gofmt -llistsnone of the changed files. CHANGELOG entry added under Unreleased.
Scope
sftpandazuredelegateSeekstraight through and may share this shape, but I could notexercise them without credentials, so this PR is limited to
os.Separately and not included:
os,azureandsftpdo not validatewhence(Linux accepts3for
SEEK_DATA) whilemem,s3,gsandftpreturnvfs.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.