encoding/xml: embedded reference to substruct causes XML marshaller to panic on encoding

When encoding a xml attribute is zero value (IsValid == false), we need
a `continue` to jump over the attribute. If not, followed marshalAttr
function will panic.

Fixes: #50164
Change-Id: I42e064558e7becfbf47728b14cbf5c7afa1e8798
Reviewed-on: https://go-review.googlesource.com/c/go/+/385514
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

2 files changed, 37 insertions, 1 deletions

diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go
index 6859be04a2..7792ac77f8 100644
--- a/src/encoding/xml/marshal.go
+++ b/src/encoding/xml/marshal.go
@@ -512,7 +512,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
 		}
 		fv := finfo.value(val, dontInitNilPointers)
 
-		if finfo.flags&fOmitEmpty != 0 && isEmptyValue(fv) {
+		if finfo.flags&fOmitEmpty != 0 && (!fv.IsValid() || isEmptyValue(fv)) {
 			continue
 		}
 
diff --git a/src/encoding/xml/marshal_test.go b/src/encoding/xml/marshal_test.go
index 5fdbae7ef0..3fe7e2dc00 100644
--- a/src/encoding/xml/marshal_test.go
+++ b/src/encoding/xml/marshal_test.go
@@ -2495,3 +2495,39 @@ func TestInvalidXMLName(t *testing.T) {
 		t.Errorf("error %q does not contain %q", err, want)
 	}
 }
+
+// Issue 50164. Crash on zero value XML attribute.
+type LayerOne struct {
+	XMLName Name `xml:"l1"`
+
+	Value     *float64 `xml:"value,omitempty"`
+	*LayerTwo `xml:",omitempty"`
+}
+
+type LayerTwo struct {
+	ValueTwo *int `xml:"value_two,attr,omitempty"`
+}
+
+func TestMarshalZeroValue(t *testing.T) {
+	proofXml := `<l1><value>1.2345</value></l1>`
+	var l1 LayerOne
+	err := Unmarshal([]byte(proofXml), &l1)
+	if err != nil {
+		t.Fatalf("unmarshal XML error: %v", err)
+	}
+	want := float64(1.2345)
+	got := *l1.Value
+	if got != want {
+		t.Fatalf("unexpected unmarshal result, want %f but got %f", want, got)
+	}
+
+	// Marshal again (or Encode again)
+	// In issue 50164, here `Marshal(l1)` will panic because of the zero value of xml attribute ValueTwo `value_two`.
+	anotherXML, err := Marshal(l1)
+	if err != nil {
+		t.Fatalf("marshal XML error: %v", err)
+	}
+	if string(anotherXML) != proofXml {
+		t.Fatalf("unexpected unmarshal result, want %q but got %q", proofXml, anotherXML)
+	}
+}