From a3131b085fcf5a25f6b068637d32cfb5d78797e4 Mon Sep 17 00:00:00 2001 From: alliasgher Date: Tue, 14 Apr 2026 03:29:22 +0500 Subject: [PATCH 1/3] decode_number: fix typo in float64 error message The float64 fallback error path said "decoding float32" instead of "decoding float64". Fix the message to match the function. Fixes #352 Signed-off-by: alliasgher --- decode_number.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decode_number.go b/decode_number.go index 45d6a74..56301ab 100644 --- a/decode_number.go +++ b/decode_number.go @@ -213,7 +213,7 @@ func (d *Decoder) float64(c byte) (float64, error) { n, err := d.int(c) if err != nil { - return 0, fmt.Errorf("msgpack: invalid code=%x decoding float32", c) + return 0, fmt.Errorf("msgpack: invalid code=%x decoding float64", c) } return float64(n), nil } From 96a9023626a1bd0f3fdbdd6df20fbb48ffa18059 Mon Sep 17 00:00:00 2001 From: alliasgher Date: Tue, 14 Apr 2026 03:31:36 +0500 Subject: [PATCH 2/3] types: use reflect.Value.IsZero for struct omitempty check The isEmptyValue method used structs.Fields to inspect only exported fields when deciding whether a struct value tagged omitempty should be omitted. If a struct had zero-valued exported fields but non-zero unexported fields, it was incorrectly treated as empty and omitted. Replace the exported-field scan with reflect.Value.IsZero, which checks all fields (exported and unexported) recursively. A struct is now omitted only when it equals the zero value of its type, which is the semantically correct behaviour for an "omit if empty" tag. Fixes #378 Signed-off-by: alliasgher --- msgpack_test.go | 30 ++++++++++++++++++++++++++++++ types.go | 9 ++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/msgpack_test.go b/msgpack_test.go index c3872c0..5a83fec 100644 --- a/msgpack_test.go +++ b/msgpack_test.go @@ -553,3 +553,33 @@ func TestPtrValueDecode(t *testing.T) { require.NotNil(t, foo.Bar) require.Equal(t, *foo.Bar, bar2) } + +// TestOmitEmptyStructWithNonZeroUnexportedField is a regression test for +// https://github.com/vmihailenco/msgpack/issues/378. +// A struct field tagged omitempty must not be omitted when the struct value +// contains a non-zero unexported field, even if all exported fields are zero. +func TestOmitEmptyStructWithNonZeroUnexportedField(t *testing.T) { + type inner struct { + n int // unexported, may be non-zero + Str string + } + + type outer struct { + Field inner `msgpack:",omitempty"` + } + + // Sanity check: when both n and Str are zero the struct should be omitted. + raw, err := msgpack.Marshal(outer{Field: inner{n: 0, Str: ""}}) + require.Nil(t, err) + var o1 outer + require.Nil(t, msgpack.Unmarshal(raw, &o1)) + require.Equal(t, "", o1.Field.Str, "zero struct: exported field should remain empty") + + // When n is non-zero the struct should NOT be omitted, and the exported + // field Str should survive the round-trip. + raw, err = msgpack.Marshal(outer{Field: inner{n: 1, Str: "str"}}) + require.Nil(t, err) + var o2 outer + require.Nil(t, msgpack.Unmarshal(raw, &o2)) + require.Equal(t, "str", o2.Field.Str, "non-zero struct: exported field must not be lost") +} diff --git a/types.go b/types.go index d212e09..bef5138 100644 --- a/types.go +++ b/types.go @@ -338,9 +338,12 @@ func (e *Encoder) isEmptyValue(v reflect.Value) bool { case reflect.Array, reflect.Map, reflect.Slice, reflect.String: return v.Len() == 0 case reflect.Struct: - structFields := structs.Fields(v.Type(), e.structTag) - fields := structFields.OmitEmpty(e, v) - return len(fields) == 0 + // Use reflect.Value.IsZero so that structs with non-zero unexported + // fields are correctly treated as non-empty. The previous approach + // only inspected exported fields and would incorrectly omit a struct + // whose exported fields were all zero but whose unexported fields + // were not (see https://github.com/vmihailenco/msgpack/issues/378). + return v.IsZero() case reflect.Bool: return !v.Bool() case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: From 688e2af537c8cbafe8156a028945a69c6d5ed8a5 Mon Sep 17 00:00:00 2001 From: alliasgher Date: Tue, 14 Apr 2026 16:15:10 +0500 Subject: [PATCH 3/3] types: prefer TextUnmarshaler over BinaryUnmarshaler for str format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a type implements both encoding.BinaryUnmarshaler and encoding.TextUnmarshaler, the decoder previously always picked BinaryUnmarshaler regardless of the wire format. This is wrong for types like uuid.UUID that are stored as msgpack str (e.g. after a JSON round-trip): the BinaryUnmarshaler expects raw bytes while a TextUnmarshaler expects the text representation. Introduce unmarshalBinaryOrTextValue that peeks at the next code: - msgpack str (any variant) → use TextUnmarshaler - everything else (bin, nil, ...) → use BinaryUnmarshaler Fixes #370 Signed-off-by: alliasgher --- decode_value.go | 24 +++++++++++++++++++ types_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/decode_value.go b/decode_value.go index c44a674..323a97b 100644 --- a/decode_value.go +++ b/decode_value.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "reflect" + + "github.com/vmihailenco/msgpack/v5/msgpcode" ) var ( @@ -71,6 +73,10 @@ func _getDecoder(typ reflect.Type) decoderFunc { return nilAwareDecoder(typ, unmarshalValue) } if typ.Implements(binaryUnmarshalerType) { + if typ.Implements(textUnmarshalerType) { + // Type implements both: pick the decoder based on the wire format. + return nilAwareDecoder(typ, unmarshalBinaryOrTextValue) + } return nilAwareDecoder(typ, unmarshalBinaryValue) } if typ.Implements(textUnmarshalerType) { @@ -87,6 +93,9 @@ func _getDecoder(typ reflect.Type) decoderFunc { return addrDecoder(nilAwareDecoder(typ, unmarshalValue)) } if ptr.Implements(binaryUnmarshalerType) { + if ptr.Implements(textUnmarshalerType) { + return addrDecoder(nilAwareDecoder(typ, unmarshalBinaryOrTextValue)) + } return addrDecoder(nilAwareDecoder(typ, unmarshalBinaryValue)) } if ptr.Implements(textUnmarshalerType) { @@ -240,6 +249,21 @@ func unmarshalBinaryValue(d *Decoder, v reflect.Value) error { return unmarshaler.UnmarshalBinary(data) } +// unmarshalBinaryOrTextValue is used for types that implement both +// encoding.BinaryUnmarshaler and encoding.TextUnmarshaler. +// It prefers TextUnmarshaler when the wire format is a MessagePack str, and +// BinaryUnmarshaler for bin and all other formats. +func unmarshalBinaryOrTextValue(d *Decoder, v reflect.Value) error { + code, err := d.PeekCode() + if err != nil { + return err + } + if msgpcode.IsString(code) { + return unmarshalTextValue(d, v) + } + return unmarshalBinaryValue(d, v) +} + func unmarshalTextValue(d *Decoder, v reflect.Value) error { data, err := d.DecodeBytes() if err != nil { diff --git a/types_test.go b/types_test.go index f8bfda1..183c893 100644 --- a/types_test.go +++ b/types_test.go @@ -1144,3 +1144,66 @@ func mustParseTime(format, s string) time.Time { } return tm } + +// dualMarshaler implements both BinaryMarshaler/Unmarshaler and +// TextMarshaler/Unmarshaler (like uuid.UUID). +type dualMarshaler struct { + data string +} + +func (d dualMarshaler) MarshalBinary() ([]byte, error) { + return []byte("bin:" + d.data), nil +} + +func (d *dualMarshaler) UnmarshalBinary(b []byte) error { + d.data = "bin:" + string(b) + return nil +} + +func (d dualMarshaler) MarshalText() ([]byte, error) { + return []byte(d.data), nil +} + +func (d *dualMarshaler) UnmarshalText(b []byte) error { + d.data = string(b) + return nil +} + +// TestDualMarshalerPreference is a regression test for #370. +// When a type implements both BinaryUnmarshaler and TextUnmarshaler, decoding +// a msgpack str should use TextUnmarshaler, and decoding a msgpack bin should +// use BinaryUnmarshaler. +func TestDualMarshalerPreference(t *testing.T) { + // Simulate a str payload (e.g. produced by another language or library) + // by encoding a plain Go string. + strMsg, err := msgpack.Marshal("hello") + if err != nil { + t.Fatal(err) + } + + // Decode str → should use UnmarshalText. + var gotText dualMarshaler + if err := msgpack.Unmarshal(strMsg, &gotText); err != nil { + t.Fatal(err) + } + if gotText.data != "hello" { + t.Errorf("UnmarshalText: got %q, want %q", gotText.data, "hello") + } + + // Simulate a bin payload by encoding raw []byte. + binPayload := []byte("raw-bytes") + binMsg, err := msgpack.Marshal(binPayload) + if err != nil { + t.Fatal(err) + } + + // Decode bin → should use UnmarshalBinary. + var gotBin dualMarshaler + if err := msgpack.Unmarshal(binMsg, &gotBin); err != nil { + t.Fatal(err) + } + // UnmarshalBinary stores "bin:" + if gotBin.data != "bin:raw-bytes" { + t.Errorf("UnmarshalBinary: got %q, want %q", gotBin.data, "bin:raw-bytes") + } +}