Просмотр исходного кода

proto: fix memory aliasing bug in Reset

Fix a bug where using Message.Range alone does not clear
memory references for empty, but allocated lists and maps.
Thus, we iterate over every known field and explicitly call Clear.

Change-Id: I9c1847d4056cf66f3199947150f3140d0783444a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183197
Reviewed-by: Damien Neil <dneil@google.com>
Joe Tsai 6 лет назад
Родитель
Сommit
ef75becaa8
2 измененных файлов с 68 добавлено и 3 удалено
  1. 10 3
      proto/reset.go
  2. 58 0
      proto/reset_test.go

+ 10 - 3
proto/reset.go

@@ -14,11 +14,18 @@ func Reset(m Message) {
 }
 
 func resetMessage(m protoreflect.Message) {
+	// Clear all known fields.
+	fds := m.Descriptor().Fields()
+	for i := 0; i < fds.Len(); i++ {
+		m.Clear(fds.Get(i))
+	}
+
+	// Clear extension fields.
 	m.Range(func(fd protoreflect.FieldDescriptor, _ protoreflect.Value) bool {
 		m.Clear(fd)
 		return true
 	})
-	if m.GetUnknown() != nil {
-		m.SetUnknown(nil)
-	}
+
+	// Clear unknown fields.
+	m.SetUnknown(nil)
 }

+ 58 - 0
proto/reset_test.go

@@ -0,0 +1,58 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style.
+// license that can be found in the LICENSE file.
+
+package proto_test
+
+import (
+	"testing"
+
+	"google.golang.org/protobuf/internal/scalar"
+	"google.golang.org/protobuf/proto"
+
+	testpb "google.golang.org/protobuf/internal/testprotos/test"
+)
+
+func TestReset(t *testing.T) {
+	m := &testpb.TestAllTypes{
+		OptionalSfixed64:       scalar.Int64(5),
+		RepeatedInt32:          []int32{},
+		RepeatedFloat:          []float32{1.234, 5.678},
+		MapFixed64Fixed64:      map[uint64]uint64{5: 7},
+		MapStringString:        map[string]string{},
+		OptionalForeignMessage: &testpb.ForeignMessage{},
+		OneofField:             (*testpb.TestAllTypes_OneofUint32)(nil),
+	}
+	m.ProtoReflect().SetUnknown([]byte{})
+
+	proto.Reset(m)
+
+	if m.OptionalSfixed64 != nil {
+		t.Errorf("m.OptionalSfixed64 = %p, want nil", m.OptionalSfixed64)
+	}
+	if m.RepeatedInt32 != nil {
+		t.Errorf("m.RepeatedInt32 = %p, want nil", m.RepeatedInt32)
+	}
+	if m.RepeatedFloat != nil {
+		t.Errorf("m.RepeatedFloat = %p, want nil", m.RepeatedFloat)
+	}
+	if m.MapFixed64Fixed64 != nil {
+		t.Errorf("m.MapFixed64Fixed64 = %p, want nil", m.MapFixed64Fixed64)
+	}
+	if m.MapStringString != nil {
+		t.Errorf("m.MapStringString = %p, want nil", m.MapStringString)
+	}
+	if m.OptionalForeignMessage != nil {
+		t.Errorf("m.OptionalForeignMessage = %p, want nil", m.OptionalForeignMessage)
+	}
+	if m.OneofField != nil {
+		t.Errorf("m.OneofField = %p, want nil", m.OneofField)
+	}
+
+	if got := m.ProtoReflect().Len(); got != 0 {
+		t.Errorf("m.ProtoReflect().Len() = %d, want 0", got)
+	}
+	if got := m.ProtoReflect().GetUnknown(); got != nil {
+		t.Errorf("m.ProtoReflect().GetUnknown() = %d, want nil", got)
+	}
+}