proto: allow text proto map keys to be omitted or order swapped

proto/equal.go

@@ -191,6 +191,13 @@ func equalAny(v1, v2 reflect.Value, prop *Properties) bool {
 		}
 		return true
 	case reflect.Ptr:
+		// Maps may have nil values in them, so check for nil.
+		if v1.IsNil() && v2.IsNil() {
+			return true
+		}
+		if v1.IsNil() != v2.IsNil() {
+			return false
+		}
 		return equalAny(v1.Elem(), v2.Elem(), prop)
 	case reflect.Slice:
 		if v1.Type().Elem().Kind() == reflect.Uint8 {

proto/text_parser.go

@@ -602,8 +602,9 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) error {
 
 			// The map entry should be this sequence of tokens:
 			//	< key : KEY value : VALUE >
-			// Technically the "key" and "value" could come in any order,
-			// but in practice they won't.
+			// However, implementations may omit key or value, and technically
+			// we should support them in any order.  See b/28924776 for a time
+			// this went wrong.
 
 			tok := p.next()
 			var terminator string
@@ -615,32 +616,39 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) error {
 			default:
 				return p.errorf("expected '{' or '<', found %q", tok.value)
 			}
-			if err := p.consumeToken("key"); err != nil {
-				return err
-			}
-			if err := p.consumeToken(":"); err != nil {
-				return err
-			}
-			if err := p.readAny(key, props.mkeyprop); err != nil {
-				return err
-			}
-			if err := p.consumeOptionalSeparator(); err != nil {
-				return err
-			}
-			if err := p.consumeToken("value"); err != nil {
-				return err
-			}
-			if err := p.checkForColon(props.mvalprop, dst.Type().Elem()); err != nil {
-				return err
-			}
-			if err := p.readAny(val, props.mvalprop); err != nil {
-				return err
-			}
-			if err := p.consumeOptionalSeparator(); err != nil {
-				return err
-			}
-			if err := p.consumeToken(terminator); err != nil {
-				return err
+			for {
+				tok := p.next()
+				if tok.err != nil {
+					return tok.err
+				}
+				if tok.value == terminator {
+					break
+				}
+				switch tok.value {
+				case "key":
+					if err := p.consumeToken(":"); err != nil {
+						return err
+					}
+					if err := p.readAny(key, props.mkeyprop); err != nil {
+						return err
+					}
+					if err := p.consumeOptionalSeparator(); err != nil {
+						return err
+					}
+				case "value":
+					if err := p.checkForColon(props.mvalprop, dst.Type().Elem()); err != nil {
+						return err
+					}
+					if err := p.readAny(val, props.mvalprop); err != nil {
+						return err
+					}
+					if err := p.consumeOptionalSeparator(); err != nil {
+						return err
+					}
+				default:
+					p.back()
+					return p.errorf(`expected "key", "value", or %q, found %q`, terminator, tok.value)
+				}
 			}
 
 			dst.SetMapIndex(key, val)

proto/text_parser_test.go

@@ -508,7 +508,10 @@ func TestMapParsing(t *testing.T) {
 	const in = `name_mapping:<key:1234 value:"Feist"> name_mapping:<key:1 value:"Beatles">` +
 		`msg_mapping:<key:-4, value:<f: 2.0>,>` + // separating commas are okay
 		`msg_mapping<key:-2 value<f: 4.0>>` + // no colon after "value"
-		`byte_mapping:<key:true value:"so be it">`
+		`msg_mapping:<value:<f: 5.0>>` + // omitted key
+		`msg_mapping:<key:1>` + // omitted value
+		`byte_mapping:<key:true value:"so be it">` +
+		`byte_mapping:<>` // omitted key and value
 	want := &MessageWithMap{
 		NameMapping: map[int32]string{
 			1:    "Beatles",
@@ -517,9 +520,12 @@ func TestMapParsing(t *testing.T) {
 		MsgMapping: map[int64]*FloatingPoint{
 			-4: {F: Float64(2.0)},
 			-2: {F: Float64(4.0)},
+			0:  {F: Float64(5.0)},
+			1:  nil,
 		},
 		ByteMapping: map[bool][]byte{
-			true: []byte("so be it"),
+			false: nil,
+			true:  []byte("so be it"),
 		},
 	}
 	if err := UnmarshalText(in, m); err != nil {