encoding/xml: bad type for comment field of "pointer to string" #19063
Labels
Milestone
Comments
josharian
changed the title
|
From empirical examination this seems to be the offending commit daa1211 and CL https://go-review.googlesource.com/c/15684. In particular the code patch from that CL is the one that causes it, so undoing it the bug goes away diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go
index abb078c..4fa1de0 100644
--- a/src/encoding/xml/marshal.go
+++ b/src/encoding/xml/marshal.go
@@ -760,14 +760,6 @@
}
vf := finfo.value(val)
- // Dereference or skip nil pointer, interface values.
- switch vf.Kind() {
- case reflect.Ptr, reflect.Interface:
- if !vf.IsNil() {
- vf = vf.Elem()
- }
- }
-
switch finfo.flags & fMode {
case fCDATA, fCharData:
emit := EscapeText
@@ -800,6 +792,16 @@
continue
}
}
+ // Drill into interfaces and pointers.
+ // This can turn into an infinite loop given a cyclic chain,
+ // but it matches the Go 1 behavior.
+ for vf.Kind() == reflect.Interface || vf.Kind() == reflect.Ptr {
+ if vf.IsNil() {
+ return nil
+ }
+ vf = vf.Elem()
+ }
+
var scratch [64]byte
switch vf.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: |
|
Just an FYI: that CL was written in October 2015, but submitted a year later in October 2016. On first glance the dates might be confusing. |
|
Will look later tonight. |
|
CL https://golang.org/cl/36954 mentions this issue. |
|
credit to @Civil for reporting. |
|
CL https://golang.org/cl/37016 mentions this issue. |
|
Merged. |
gopherbot
pushed a commit
that referenced
this issue
Feb 15, 2017
…hardata, comment, innerxml fields The new tests in this CL have been checked against Go 1.7 as well and all pass in Go 1.7, with the one exception noted in a comment (an intentional change to omitempty already present before this CL). CL 15684 made the intentional change to omitempty. This CL fixes bugs introduced along the way. Most of these are corner cases that are arguably not that important, but they've always worked all the way back to Go 1, and someone cared enough to file #19063. The most significant problem found while adding tests is that in the case of a nil *string field with `xml:",chardata"`, the existing code silently stops processing not just that field but the entire remainder of the struct. Even if #19063 were not worth fixing, this chardata bug would be. Fixes #19063. Change-Id: I318cf8f9945e1a4615982d9904e109fde577ebf9 Reviewed-on: https://go-review.googlesource.com/36954 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> (cherry picked from commit 72aa757) Reviewed-on: https://go-review.googlesource.com/37016 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
|
Thank you |
albertjin
added a commit
to albertjin/golang-go
that referenced
this issue
Feb 16, 2017
…hardata, comment, innerxml fields The new tests in this CL have been checked against Go 1.7 as well and all pass in Go 1.7, with the one exception noted in a comment (an intentional change to omitempty already present before this CL). CL 15684 made the intentional change to omitempty. This CL fixes bugs introduced along the way. Most of these are corner cases that are arguably not that important, but they've always worked all the way back to Go 1, and someone cared enough to file golang#19063. The most significant problem found while adding tests is that in the case of a nil *string field with `xml:",chardata"`, the existing code silently stops processing not just that field but the entire remainder of the struct. Even if golang#19063 were not worth fixing, this chardata bug would be. Fixes golang#19063. Change-Id: I318cf8f9945e1a4615982d9904e109fde577ebf9 Reviewed-on: https://go-review.googlesource.com/36954 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> (cherry picked from commit 72aa757) Reviewed-on: https://go-review.googlesource.com/37016 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version)?go1.8rc3
What operating system and processor architecture are you using (
go env)?GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/waschulze/code"
GORACE=""
GOROOT="/Users/waschulze/go1.8rc3"
GOTOOLDIR="/Users/waschulze/go1.8rc3/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vg/phjwfkn14tjdh3l838gn4q_c395_bf/T/go-build149844404=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
What did you do?
https://play.golang.org/p/AgMlZ8P4eN
What did you expect to see?
Program exited
What did you see instead?
panic: xml: bad type for comment field of main.A
The text was updated successfully, but these errors were encountered: