From 851ecea4cc99ab276109493477b2c7e30c253ea8 Mon Sep 17 00:00:00 2001
From: hopehook <hopehook.com@gmail.com>
Date: Sun, 13 Feb 2022 22:03:56 +0800
Subject: [PATCH] encoding/xml: embedded reference to substruct causes XML
 marshaller to panic on encoding
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

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>
---
 src/encoding/xml/marshal.go      |  2 +-
 src/encoding/xml/marshal_test.go | 36 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

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)
+	}
+}
-- 
2.48.1