types: prefer TextUnmarshaler over BinaryUnmarshaler for str format

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
 }

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 {

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

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:

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:<input>"
+	if gotBin.data != "bin:raw-bytes" {
+		t.Errorf("UnmarshalBinary: got %q, want %q", gotBin.data, "bin:raw-bytes")
+	}
+}