proto: Equal prints to stderr and fails on what's handled by Marshal/Unmarshal

[dvyukov]

Two problems with the following program:

package main

import (
    "fmt"

    "github.com/golang/protobuf/proto"
)

func main() {
    data := []byte("\xf5\a\xa0\xe000")
    v := new(M18)
    var err error
    err = proto.Unmarshal(data, v)
    if err != nil {
        return
    }
    sz := proto.Size(v)
    var data1 []byte
    data1, err = proto.Marshal(v)
    if err != nil {
        panic(err)
    }
    v1 := new(M18)
    if sz != len(data1) {
        panic(fmt.Sprintf("Size returned %v, while Marshal returned %v", sz, len(data1)))
    }
    err = proto.Unmarshal(data1, v1)
    if err != nil {
        panic(err)
    }
    if !proto.Equal(v, v1) {
        fmt.Printf("v0: %#v\n", v)
        fmt.Printf("v1: %#v\n", v1)
        panic(fmt.Sprintf("non idempotent marshal of %T", v))
    }
}

/*
message M18 {
  optional string f0 = 1;
  extensions 100 to 199;
}

extend M18 {
  optional int32 f1 = 126;
}
*/
type M18 struct {
    F0               *string                   `protobuf:"bytes,1,opt,name=f0" json:"f0,omitempty"`
    XXX_extensions   map[int32]proto.Extension `json:"-"`
    XXX_unrecognized []byte                    `json:"-"`
}

func (m *M18) Reset()         { *m = M18{} }
func (m *M18) String() string { return proto.CompactTextString(m) }
func (*M18) ProtoMessage()    {}

var extRange_M18 = []proto.ExtensionRange{
    {100, 199},
}

func (*M18) ExtensionRangeArray() []proto.ExtensionRange {
    return extRange_M18
}
func (m *M18) ExtensionMap() map[int32]proto.Extension {
    if m.XXX_extensions == nil {
        m.XXX_extensions = make(map[int32]proto.Extension)
    }
    return m.XXX_extensions
}

func (m *M18) GetF0() string {
    if m != nil && m.F0 != nil {
        return *m.F0
    }
    return ""
}

var E_F1 = &proto.ExtensionDesc{
    ExtendedType:  (*M18)(nil),
    ExtensionType: (*int32)(nil),
    Field:         126,
    Name:          "example.f1",
    Tag:           "varint,126,opt,name=f1",
}

func init() {
    proto.RegisterExtension(E_F1)
}

1. It prints to stderr:

2016/01/17 13:15:30 proto: badly encoded extension 126 of main.M18: unexpected EOF

1. Equal fails on round-trip data:

v0: &main.M18{F0:(*string)(nil), XXX_extensions:map[int32]proto.Extension{126:proto.Extension{desc:(*proto.ExtensionDesc)(nil), value:interface {}(nil), enc:[]uint8{0xf5, 0x7, 0xa0, 0xe0, 0x30, 0x30}}}, XXX_unrecognized:[]uint8(nil)}
v1: &main.M18{F0:(*string)(nil), XXX_extensions:map[int32]proto.Extension{126:proto.Extension{desc:(*proto.ExtensionDesc)(nil), value:interface {}(nil), enc:[]uint8{0xf5, 0x7, 0xa0, 0xe0, 0x30, 0x30}}}, XXX_unrecognized:[]uint8(nil)}
panic: non idempotent marshal of *main.M18

Data must be Equal after Marshal/Unmarshal.

On commit 2402d76.

[dsnet]

Closing as duplicate of #667, which is to investigate all usages of log.Print

[dvyukov]

Again, #667 does not cover the data corruption. Somebody will fix logs, but the data corruption is still there.

[dsnet]

In the presence of unknown fields (for which unregistered extensions are special-case of), Equal always returns false. It could be argued that it should fall back on direct string comparison on the raw text. However, comparing raw bytes has very few guarantees:

• Identical raw bytes does not imply equality due to the possible presence of NaNs.
• Non-identical raw bytes does not imply inequality as fields may be re-ordered in the bytes wire type, but the lack of proto type information makes it impossible to know.

The current position isn't great (only false negatives), but I'm not sure it's worse than the alternative where the equality can have false-negatives and false-positives.