Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix slice encoding #64

Merged
merged 1 commit into from
Jul 3, 2024
Merged

Conversation

chrisroberts
Copy link
Contributor

Encoding a field that is a slice will currently panic due to Elem() being called on a slice in util.ReflectValue. reflect.Value only allows for Elem() to be called on interface or pointer values and will panic otherwise. So this just removes slice from being included in the check to unwrap and adds in a test to validate that encoding the slice is successful.

@coveralls
Copy link

coveralls commented Jul 2, 2024

Pull Request Test Coverage Report for Build 9752982979

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.897%

Totals Coverage Status
Change from base Build 9298275274: 0.0%
Covered Lines: 406
Relevant Lines: 419

💛 - Coveralls

@pilagod
Copy link
Owner

pilagod commented Jul 2, 2024

@chrisroberts Nice catch! Could you help me to add an integration test in cursor/encoding_test.go, by encoding and then decoding to ensure they work expectedly?

You can take other cases as examples:

/* struct */
type structModel struct {
Value structValue
ValuePtr *structValue
}
type structValue struct {
Value []byte
}
func (s *encodingSuite) TestStruct() {
c, _ := s.encodeValue(structModel{
Value: structValue{Value: []byte("123")},
})
v, _ := s.decodeValue(structModel{}, c)
s.Equal(structValue{Value: []byte("123")}, v)
}
func (s *encodingSuite) TestStructPtr() {
sv := structValue{Value: []byte("123")}
c, _ := s.encodeValuePtr(structModel{ValuePtr: &sv})
v, _ := s.decodeValuePtr(structModel{}, c)
s.Equal(sv, *(v.(*structValue)))
}

But I am wondering that []byte should be also a slice, right? I am not sure why the []byte case could work perfectly, do you have any idea?

Updates the util.ReflectValue function to not call Elem() on
a reflect.Value that is a slice as it is only allowed on
an interface or pointer and will panic otherwise.
@chrisroberts
Copy link
Contributor Author

@pilagod Yeah, no problem. Added in the extra tests.

As for the []byte case, it works because the value on the struct is not the byte slice directly, rather the value is the struct that contains the byte slice. So when the value is reflected, it's not seen as a slice.

@coveralls
Copy link

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9765782179

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.897%

Totals Coverage Status
Change from base Build 9298275274: 0.0%
Covered Lines: 406
Relevant Lines: 419

💛 - Coveralls

@pilagod
Copy link
Owner

pilagod commented Jul 3, 2024

You're right, I totally missed it is under another struct.

Thank you so much for this patch, it looks pretty good to me. I will notify you once there is a new release including this fix 💪

@pilagod pilagod merged commit 272eb3d into pilagod:master Jul 3, 2024
3 checks passed
@pilagod
Copy link
Owner

pilagod commented Jul 3, 2024

I just released v2.6.1 to include this patch. Thanks again for the contribution. 🙌

@chrisroberts chrisroberts deleted the slice-encoding branch July 3, 2024 15:21
@chrisroberts
Copy link
Contributor Author

Awesome, thank you so much @pilagod!

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.

3 participants