fix(y): shall always return empty slice rather than nil#2245
fix(y): shall always return empty slice rather than nil#2245matthewmcneely merged 4 commits intodgraph-io:mainfrom
Conversation
In practice, keys processed will always have data, but echo `ParseTs`'s logic to prevent panics as SafeCopy does now produce zero length slices and operates on Entry.Key
This was causing an intermittent cgo segmentation problem on OSX
| // ParseKey parses the actual key from the key bytes. | ||
| func ParseKey(key []byte) []byte { | ||
| if key == nil { | ||
| if len(key) < 8 { |
There was a problem hiding this comment.
@kooltuoehias I added this sanity check here. In practice, keys processed anywhere in badger will always have byte data, but because SafeCopy does now produce zero length slices and Entry.Key values are sent to that function in publisher.go, better safe than sorry. This check mimics ParseTs's logic to prevent panics by ensuring the slice has the capacity that it's operating on.
There was a problem hiding this comment.
@kooltuoehias I added this sanity check here. In practice, keys processed anywhere in badger will always have byte data, but because SafeCopy does now produce zero length slices and
Entry.Keyvalues are sent to that function in publisher.go, better safe than sorry. This check mimicsParseTs's logic to prevent panics by ensuring the slice has the capacity that it's operating on.
Many thanks ! Things goes really quick here !
| # run tests in sequence | ||
| #root | ||
| #stream | ||
| root |
There was a problem hiding this comment.
These mistakenly got commented out in a prior merge.
There was a problem hiding this comment.
These mistakenly got commented out in a prior merge.
Thanks for catching the edge case with Entry.Key and the quick merge! Happy to contribute.
2 participants
Fixes #2067
Description
This PR fixes an issue where Go's
appendreturnsnilwhen appending an empty slice to a nil slice.I added a check in
SafeCopyto ensure we always return[]byte{}in this case, along with a new unit test.I also verified it using the test offered in the issue
Checklist
CHANGELOG.mdfile describing and linking tothis PR
Instructions
syntax, leading with
fix:,feat:,chore:,ci:, etc.link to the bug.
[x]syntax.back and check the box later.
Instructionsline and everything below it, to indicate you have read and arefollowing these instructions. 🙂
Thank you for your contribution to Badger!