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

Unmarshalled Strings Unsafely Share Byte Slices #25

Closed
swdunlop opened this issue Jul 10, 2021 · 1 comment
Closed

Unmarshalled Strings Unsafely Share Byte Slices #25

swdunlop opened this issue Jul 10, 2021 · 1 comment

Comments

@swdunlop
Copy link

If the bytes in the byte slice passed to msgpack.Unmarshal change after msgpack.Unmarshal returns, any of the unmarshaled strings may change their contents, violating the expectation that Go strings are immutable. This is probably due to the use of unsafe.Pointer in decoder.asString. This is surprising behavior which can really screw things up in Go if the original byte slice was a buffer whose contents are replaced on each message.

See https://play.golang.org/p/YQyb8ie8Qqd for a simple repro of the issue.

The simplest fix I can see is to document this behavior and warn not to reuse the underlying byte array until any strings are expunged. Discontinuing this behavior would likely have a dramatic negative impact on performance. The most complex fix would be to make the behavior of asString configurable, which would let the users decide if they want to sacrifice sanity for efficiency. (And this is an insane behavior -- consider what happens if the resulting string was used as a map key by an unsuspecting user.)

@shamaton
Copy link
Owner

shamaton commented Aug 5, 2021

@swdunlop
Sorry for delay.
Thank you for proposing this issue and sharing sample code!!
I understood how this behavior.

Performance is important, but being safe to use is more important.
So, I updated behavior by this PR. #27

Performance is less, but all users can use this package safe completely.

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

No branches or pull requests

2 participants