Forráskód Böngészése

proto: do not allow unknown fields to satisfy required field bit (#565)

A prior changed (see #511) allows Unmarshal to treat fields with
unknown wire types with known tags as unknown fields.
When doing so, we must be careful not to set the required field bit.

For example, suppose we have:
	message Foo { require fixed32 my_field = 1; }

Then parsing a message with a field of type varint and tag 1 should
not satisfy the required that Foo.my_field be set.
Joe Tsai 7 éve
szülő
commit
91cccdb44a
2 módosított fájl, 22 hozzáadás és 2 törlés
  1. 19 0
      proto/all_test.go
  2. 3 2
      proto/table_unmarshal.go

+ 19 - 0
proto/all_test.go

@@ -1860,6 +1860,25 @@ func TestRequiredNotSetError(t *testing.T) {
 	}
 }
 
+func TestRequiredNotSetErrorWithBadWireTypes(t *testing.T) {
+	// Required field expects a varint, and properly found a varint.
+	if err := Unmarshal([]byte{0x08, 0x00}, new(GoEnum)); err != nil {
+		t.Errorf("Unmarshal = %v, want nil", err)
+	}
+	// Required field expects a varint, but found a fixed32 instead.
+	if err := Unmarshal([]byte{0x0d, 0x00, 0x00, 0x00, 0x00}, new(GoEnum)); err == nil {
+		t.Errorf("Unmarshal = nil, want RequiredNotSetError")
+	}
+	// Required field expects a varint, and found both a varint and fixed32 (ignored).
+	m := new(GoEnum)
+	if err := Unmarshal([]byte{0x08, 0x00, 0x0d, 0x00, 0x00, 0x00, 0x00}, m); err != nil {
+		t.Errorf("Unmarshal = %v, want nil", err)
+	}
+	if !bytes.Equal(m.XXX_unrecognized, []byte{0x0d, 0x00, 0x00, 0x00, 0x00}) {
+		t.Errorf("expected fixed32 to appear as unknown bytes: %x", m.XXX_unrecognized)
+	}
+}
+
 func fuzzUnmarshal(t *testing.T, data []byte) {
 	defer func() {
 		if e := recover(); e != nil {

+ 3 - 2
proto/table_unmarshal.go

@@ -166,20 +166,21 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
 		} else {
 			f = u.sparse[tag]
 		}
-		reqMask |= f.reqMask
 		if fn := f.unmarshal; fn != nil {
 			var err error
 			b, err = fn(b, m.offset(f.field), wire)
 			if err == nil {
+				reqMask |= f.reqMask
 				continue
 			}
 			if r, ok := err.(*RequiredNotSetError); ok {
 				// Remember this error, but keep parsing. We need to produce
 				// a full parse even if a required field is missing.
 				rnse = r
+				reqMask |= f.reqMask
 				continue
 			}
-			if err == nil || err != errInternalBadWireType {
+			if err != errInternalBadWireType {
 				return err
 			}
 			// Fragments with bad wire type are treated as unknown fields.