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 } 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/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: 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") + } +}