Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

encoding/xml: Unmarshal panic when use generic #48318

Closed
nzlov opened this issue Sep 10, 2021 · 5 comments
Closed

encoding/xml: Unmarshal panic when use generic #48318

nzlov opened this issue Sep 10, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@nzlov
Copy link

nzlov commented Sep 10, 2021

What version of Go are you using (go version)?

$ go version
go version devel go1.18-c69f5c0d76 Fri Sep 10 10:45:59 2021 +0000 darwin/amd64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/nzlov/Library/Caches/go-build"
GOENV="/Users/nzlov/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/nzlov/workspace/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/nzlov/workspace/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/Users/nzlov/program/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/nzlov/program/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="devel go1.18-c69f5c0d76 Fri Sep 10 10:45:59 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/nzlov/workspace/vgo/generices/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/71/4t6lk06x5y77f2wyln9vqnlw0000gn/T/go-build3474694710=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import (
	"encoding/json"
	"encoding/xml"
	"fmt"
)

type A[T any] struct {
	Name          string  `json:"name" xml:"name"`
	Data          T       `json:"data" xml:"data"`
	ErrorResponse ErrResp `json:"errorResponse" xml:"errorResponse"`
}

type ErrResp struct {
	Age int `xml:"age"`
}

func main() {
	fmt.Printf("%+v\n", *b[int]())
}

func b[T any]() *T {
	// data :=`<A[int]><name>name</name><data>1</data><errorResponse><age>10</age></errorResponse></A[int]>`
	data, err := xml.Marshal(&A[int]{Name: "name", Data: 1, ErrorResponse: ErrResp{Age: 10}})
	if err != nil {
		panic(err)
	}
	fmt.Printf("%+v\n", string(data))
	a1 := A[T]{}
	err = xml.Unmarshal([]byte(data), &a1)
	if err != nil {
		panic(err)
	}
	fmt.Printf("%+v\n", a1)
	return &a1.Data
}

What did you expect to see?

1

What did you see instead?

<A[int]><name>name</name><data>1</data><errorResponse><age>10</age></errorResponse></A[int]>
panic: XML syntax error on line 1: expected attribute name in element

goroutine 1 [running]:
main.b[.shape.int](0x10f3f80)
	/Users/nzlov/workspace/vgo/generices/main.go:45 +0x1dd
main.main()
	/Users/nzlov/workspace/vgo/generices/main.go:21 +0x25
exit status 2
@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 10, 2021
@thanm thanm added this to the Backlog milestone Sep 10, 2021
@thanm
Copy link
Contributor

thanm commented Sep 10, 2021

@randall77 @danscales

@randall77
Copy link
Contributor

This patch "fixes" the problem, but makes the parser out-of-spec, so I'm not sure we can do it.

diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go
index c14954df15..9a9acbab9b 100644
--- a/src/encoding/xml/xml.go
+++ b/src/encoding/xml/xml.go
@@ -1222,7 +1222,7 @@ func isNameByte(c byte) bool {
        return 'A' <= c && c <= 'Z' ||
                'a' <= c && c <= 'z' ||
                '0' <= c && c <= '9' ||
-               c == '_' || c == ':' || c == '.' || c == '-'
+               c == '_' || c == ':' || c == '.' || c == '-' || c == '[' || c == ']'
 }
 
 func isName(s []byte) bool {
@@ -1477,6 +1477,8 @@ var second = &unicode.RangeTable{
        R16: []unicode.Range16{
                {0x002D, 0x002E, 1},
                {0x0030, 0x0039, 1},
+               {0x005b, 0x005c, 1}, // [
+               {0x005d, 0x005e, 1}, // ]
                {0x00B7, 0x00B7, 1},
                {0x02D0, 0x02D1, 1},
                {0x0300, 0x0345, 1},

We may just need to accept that generic types cannot be encoded into XML. Or we need a different naming scheme for generics in XML that uses approved punctuation.

In any case, there's something to do here. The marshaler should return an error if the unmarshaler will return an error.

@rsc from owners

@rsc
Copy link
Contributor

rsc commented Sep 11, 2021

https://play.golang.org/p/OoUqANfbaSl shows what happens for an unnamed struct type: we get an unsupported type error.
This should only affect the top-level element: the inner elements are named for the fields they come from.
We could say that named generic structs are disallowed, but I think it is also fine and probably better to chop the name at the first [.

@rsc
Copy link
Contributor

rsc commented Sep 11, 2021

The docs say:

    The name for the XML elements is taken from, in order of preference:

        - the tag on the XMLName field, if the data is a struct
        - the value of the XMLName field of type Name
        - the tag of the struct field used to obtain the data
        - the name of the struct field used to obtain the data
        - the name of the marshaled type

So there are four different preferred choices before we get to this one - plenty of ways to override the decision to chop at the [.

@gopherbot
Copy link

Change https://golang.org/cl/349349 mentions this issue: encoding/xml: truncate generic type names

@randall77 randall77 mentioned this issue Sep 21, 2021
Closed
@golang golang locked and limited conversation to collaborators Sep 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@rsc @nzlov @randall77 @gopherbot @thanm