Ver Fonte

proto: unmarshal extensions eagerly

Prior to this CL, Go did not eagerly unmarshal extension fields.
The unmarshaling was done when GetExtension was called.
The prior behavior has always been problematic:
* This is incorrect since we are not validating that extensions
are valid upon the initial unmarshal, which is a property of correctness
that C++ and Java provide.
* Usage is racy. GetExtension is a mutating operation since it can
transform some bytes in the unknown fields to become extension fields.
Thus, concurrent read-only operations with GetExtension is racy.
In v1, we added a lock around the extension map to try and avoid a race,
but this is just a facade. This fixes a race condition in terms of the
Go memory model, but it is still racy since a local operation can still
observe the effects of a remote GetExtension operation.
* The API is clunky since delayed unmarshaling means that GetExtension
needs to deal with possible unmarshal errors.

We make extensions eagerly unmarshaled to avoid all of these problems.

Change-Id: Iafccf2aeaa839c0d3f88ca18bd84af481e9b1662
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/172399
Reviewed-by: Damien Neil <dneil@google.com>
Joe Tsai há 6 anos atrás
pai
commit
292a74a82a
1 ficheiros alterados com 31 adições e 0 exclusões
  1. 31 0
      proto/table_unmarshal.go

+ 31 - 0
proto/table_unmarshal.go

@@ -116,6 +116,7 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
 	}
 	var reqMask uint64 // bitmask of required fields we've seen.
 	var errLater error
+	var hasExtensions bool
 	for len(b) > 0 {
 		// Read tag and wire type.
 		// Special case 1 and 2 byte varints.
@@ -191,6 +192,7 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
 		var e Extension
 		for _, r := range u.extensionRanges {
 			if uint64(r.Start) <= tag && tag <= uint64(r.End) {
+				hasExtensions = true
 				if u.extensions.IsValid() {
 					mp := m.offset(u.extensions).toExtensions()
 					emap = protoimpl.X.ExtensionFieldsOf(mp)
@@ -223,6 +225,35 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
 			emap.Set(protoreflect.FieldNumber(tag), e)
 		}
 	}
+
+	// If there were unknown extensions, eagerly unmarshal them.
+	if hasExtensions {
+		mi := m.asPointerTo(u.typ).Interface().(Message)
+		ep, _ := extendable(mi)
+		if ep != nil {
+			var errFatal error
+			emap := RegisteredExtensions(mi) // map[int32]*ExtensionDesc
+			ep.Range(func(id protoreflect.FieldNumber, ef Extension) bool {
+				ed := emap[int32(id)]
+				if ed != nil {
+					_, err := GetExtension(mi, ed)
+					var nerr nonFatal
+					if !nerr.Merge(err) {
+						errFatal = err
+						return false
+					}
+					if errLater == nil {
+						errLater = nerr.E
+					}
+				}
+				return true
+			})
+			if errFatal != nil {
+				return errFatal
+			}
+		}
+	}
+
 	if reqMask != u.reqMask && errLater == nil {
 		// A required field of this message is missing.
 		for _, n := range u.reqFields {