Reject negative offsets in MemMapFs - #654
Open
youdie006 wants to merge 1 commit into
Open
Conversation
Seek stored a negative position without complaint and ReadAt/WriteAt sliced with it, so the standard read-the-last-N-bytes idiom panicked with a slice bounds error on a file shorter than N. OsFs returns an error for all three.
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.
MemMapFsaccepts a negative seek offset and then panics on the next read, whereOsFsreturns anerror.
Reading the last N bytes of a file shorter than N is the usual way to hit it:
Same three calls through both backends of the shared suite:
OsFsis the correct half.io.Seekersays "Seeking to an offset before the start of the file isan error", and
os.FilereturnsEINVALfor the seek andnegative offsetforReadAt.Cause
mem/file.go:275stores whatever the arithmetic produces:and
mem/file.go:233then slices with it:ReadAtandWriteAthave the same gap for a caller-supplied negativeoff.Change
A negative-offset guard in
Seek,ReadAtandWriteAt, returning theErrOutOfRangesentinelthis file already uses for
Truncate(-1)(mem/file.go:261).Seeknow computes into a local andvalidates before storing, which also makes an unknown
whencean error rather than a silent no-op —os.Filerejects that too.Test
Added to
afero_test.go, which runs overFss = []Fs{&MemMapFs{}, &OsFs{}}, soOsFsdefines theexpected behaviour rather than me asserting it. With only
mem/file.goreverted,OsFspasses andMemMapFsfails on the seeks and then panics on theReadAt:With the change both backends pass.
go test ./...andgo test -race ./ ./memare green, andgofmt -llists neither file.Disclosure: prepared with AI assistance; I verified the backend comparison and the red/green runs
myself.