From 2ab177483124b1a3d1dd23cd87dca6e81755827f Mon Sep 17 00:00:00 2001
From: Krzysztof Drys <krzysztof@ingrid.com>
Date: Mon, 22 Mar 2021 10:51:44 +0100
Subject: [PATCH 1/2] jsonpb: bring back old behaviour of handling nulls and
 JSONPBUnmarshaler

Change how jsonpb.Unmarshal handles nested JSONPBMarshaler fields a null json values.

When json null is encountered for a field which implements JSONPBUnmarshaler, jsonpb will now calli unmarshal method from the field, instead of just assigning nil to this field.
---
 jsonpb/decode.go    | 27 +++++++++++++++++++++------
 jsonpb/json_test.go |  2 +-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/jsonpb/decode.go b/jsonpb/decode.go
index 7c6c5a5244..b5d681be93 100644
--- a/jsonpb/decode.go
+++ b/jsonpb/decode.go
@@ -135,14 +135,14 @@ func (u *Unmarshaler) unmarshalMessage(m protoreflect.Message, in []byte) error
 	md := m.Descriptor()
 	fds := md.Fields()
 
-	if string(in) == "null" && md.FullName() != "google.protobuf.Value" {
-		return nil
-	}
-
 	if jsu, ok := proto.MessageV1(m.Interface()).(JSONPBUnmarshaler); ok {
 		return jsu.UnmarshalJSONPB(u, in)
 	}
 
+	if string(in) == "null" && md.FullName() != "google.protobuf.Value" {
+		return nil
+	}
+
 	switch wellKnownType(md.FullName()) {
 	case "Any":
 		var jsonObject map[string]json.RawMessage
@@ -332,11 +332,12 @@ func (u *Unmarshaler) unmarshalMessage(m protoreflect.Message, in []byte) error
 			raw = v
 		}
 
+		field := m.NewField(fd)
 		// Unmarshal the field value.
-		if raw == nil || (string(raw) == "null" && !isSingularWellKnownValue(fd)) {
+		if raw == nil || (string(raw) == "null" && !isSingularWellKnownValue(fd) && !isUnmarshalJSONPB(field, fd)) {
 			continue
 		}
-		v, err := u.unmarshalValue(m.NewField(fd), raw, fd)
+		v, err := u.unmarshalValue(field, raw, fd)
 		if err != nil {
 			return err
 		}
@@ -390,6 +391,20 @@ func isSingularWellKnownValue(fd protoreflect.FieldDescriptor) bool {
 	return false
 }
 
+func isUnmarshalJSONPB(v protoreflect.Value, fd protoreflect.FieldDescriptor) bool {
+	if fd.Kind() != protoreflect.MessageKind {
+		return false
+	}
+	if fd.Cardinality() == protoreflect.Repeated {
+		return false
+	}
+
+	i := v.Interface()
+	m := proto.MessageV1(i)
+	_, ok := m.(JSONPBUnmarshaler)
+	return ok
+}
+
 func (u *Unmarshaler) unmarshalValue(v protoreflect.Value, in []byte, fd protoreflect.FieldDescriptor) (protoreflect.Value, error) {
 	switch {
 	case fd.IsList():
diff --git a/jsonpb/json_test.go b/jsonpb/json_test.go
index 0ef23f2d30..a98ad169f2 100644
--- a/jsonpb/json_test.go
+++ b/jsonpb/json_test.go
@@ -1009,7 +1009,7 @@ func TestUnmarshalNullWithJSONPBUnmarshaler(t *testing.T) {
 		t.Errorf("unmarshal error: %v", err)
 	}
 
-	want := ptrFieldMessage{}
+	want := ptrFieldMessage{StringField: &stringField{IsSet: true, StringValue: "null"}}
 	if !proto.Equal(&ptrFieldMsg, &want) {
 		t.Errorf("unmarshal result StringField: got %v, want %v", ptrFieldMsg, want)
 	}

From 80c245acf0beb22f1fc0dd76d1c4c92dc7efae26 Mon Sep 17 00:00:00 2001
From: Krzysztof Drys <krzysztof@ingrid.com>
Date: Wed, 24 Mar 2021 12:14:56 +0100
Subject: [PATCH 2/2] code review changes

---
 jsonpb/decode.go | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/jsonpb/decode.go b/jsonpb/decode.go
index b5d681be93..60e82caa9a 100644
--- a/jsonpb/decode.go
+++ b/jsonpb/decode.go
@@ -334,7 +334,7 @@ func (u *Unmarshaler) unmarshalMessage(m protoreflect.Message, in []byte) error
 
 		field := m.NewField(fd)
 		// Unmarshal the field value.
-		if raw == nil || (string(raw) == "null" && !isSingularWellKnownValue(fd) && !isUnmarshalJSONPB(field, fd)) {
+		if raw == nil || (string(raw) == "null" && !isSingularWellKnownValue(fd) && !isSingularJSONPBUnmarshaler(field, fd)) {
 			continue
 		}
 		v, err := u.unmarshalValue(field, raw, fd)
@@ -365,11 +365,12 @@ func (u *Unmarshaler) unmarshalMessage(m protoreflect.Message, in []byte) error
 			return fmt.Errorf("extension field %q does not extend message %q", xname, m.Descriptor().FullName())
 		}
 
+		field := m.NewField(fd)
 		// Unmarshal the field value.
-		if raw == nil || (string(raw) == "null" && !isSingularWellKnownValue(fd)) {
+		if raw == nil || (string(raw) == "null" && !isSingularWellKnownValue(fd) && !isSingularJSONPBUnmarshaler(field, fd)) {
 			continue
 		}
-		v, err := u.unmarshalValue(m.NewField(fd), raw, fd)
+		v, err := u.unmarshalValue(field, raw, fd)
 		if err != nil {
 			return err
 		}
@@ -391,18 +392,12 @@ func isSingularWellKnownValue(fd protoreflect.FieldDescriptor) bool {
 	return false
 }
 
-func isUnmarshalJSONPB(v protoreflect.Value, fd protoreflect.FieldDescriptor) bool {
-	if fd.Kind() != protoreflect.MessageKind {
-		return false
-	}
-	if fd.Cardinality() == protoreflect.Repeated {
-		return false
+func isSingularJSONPBUnmarshaler(v protoreflect.Value, fd protoreflect.FieldDescriptor) bool {
+	if fd.Message() != nil && fd.Cardinality() != protoreflect.Repeated {
+		_, ok := proto.MessageV1(v.Interface()).(JSONPBUnmarshaler)
+		return ok
 	}
-
-	i := v.Interface()
-	m := proto.MessageV1(i)
-	_, ok := m.(JSONPBUnmarshaler)
-	return ok
+	return false
 }
 
 func (u *Unmarshaler) unmarshalValue(v protoreflect.Value, in []byte, fd protoreflect.FieldDescriptor) (protoreflect.Value, error) {