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

v5 encoding.TextMarshaler and round tripping #367

Open
razor-1 opened this issue Jan 21, 2024 · 1 comment
Open

v5 encoding.TextMarshaler and round tripping #367

razor-1 opened this issue Jan 21, 2024 · 1 comment

Comments

@razor-1
Copy link

razor-1 commented Jan 21, 2024

In trying to move from v4 to v5, I noticed that v5 checks for the implementation of encoding.TextMarshaler, and if present, encodes its output as bytes.

Since encoding.TextMarshaler is intended to provide a textual, UTF-8 representation of an object, it often does not include the necessary information to fully round-trip all fields.

Expected Behavior

Using a type such as String found here or in the many similar packages used for working with relational databases, a msgpack.Marshal() msgpack.Unmarshal() sequence should result in equivalent values for all struct fields.

Current Behavior

None of the original information is preserved; an error like this is returned:

msgpack: invalid code=c4 decoding map length

Possible Solution

Removing the check for the implementation of encoding.TextMarshaler would be a breaking change in v5, and in other use cases it may very well be valuable; in fact, it must have been added for this reason. But it would be nice to be able to configure a msgpack decoder with an option not to check for the encoding.TextMarshaler implementation, since by its definition in the encoding package, TextMarshaler will often produce output that does not fully encode the object.

Steps to Reproduce

import (
  "github.com/vmihailenco/msgpack/v5"
  "github.com/volatiletech/null/v9"
)

testVal := null.StringFrom("test")
packed, err := msgpack.Marshal(&testVal)
var unpacked null.String
err = msgpack.Unmarshal(packed, &unpacked)

Context (Environment)

Since v4 didn't do this, and types that implement TextMarshaler are in widespread use throughout my code base, it appears that an easy migration to v5 is not possible

Detailed Description

I'd like a way to opt out of using TextMarshaler and TextUnmarshaler.

Possible Implementation

@razor-1
Copy link
Author

razor-1 commented Jan 21, 2024

Also I should note that if this idea of allowing an opt-out is agreeable I am happy to work on a PR to implement it.

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

1 participant