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.

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

encoding/xml: lacks support for decoding arrays #20709

Open
ITR13 opened this Issue Jun 16, 2017 · 6 comments

Comments

Projects
None yet
5 participants
@ITR13

ITR13 commented Jun 16, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8.1 windows/amd64

What did you do?

Tried to store a struct containing [2]int as xml, then load it:
https://play.golang.org/p/TDMcHlf3fn

What did you expect to see?

{[0 255]} {[255 255]}

What did you see instead?

panic: unknown type [2]uint8

goroutine 1 [running]:
main.main()
	/tmp/sandbox393989733/main.go:22 +0x260

Additional Notes

There is no case reflect.Array: in (p *Decoder) unmarshal in "encoding/xml/read.go"
Encoding arrays works
Decoding arrays as slices works

@bradfitz bradfitz changed the title from encoding/xml lacks support for decoding arrays to encoding/xml: lacks support for decoding arrays Jun 16, 2017

@bradfitz bradfitz added this to the Go1.10 milestone Jun 16, 2017

@leighmcculloch

This comment has been minimized.

Contributor

leighmcculloch commented Oct 27, 2017

As a comparison to slices, which work: https://play.golang.org/p/4Jo05Hdq-m

@leighmcculloch

This comment has been minimized.

Contributor

leighmcculloch commented Oct 28, 2017

Encoding arrays works
Decoding arrays as slices works

Given it is possible to encode arrays, I would expect it should be possible to decode arrays.

However, if the package supported decoding arrays these scenarios aren't completely obvious:

More XML elements than the array's length

Should it error, or ignore the remaining?

I would probably expect it to error since the data being decoded would not be of the same type, where the type is an array of specific length and the data won't fit into an array of that length.

Counter to that though, what if an application didn't care about additional elements?

Less XML elements than the array's length

Should it error, or leave the extra elements in the array as zero values?

I would probably expect it to leave the extra elements in the array as zero values, given that when fields in a struct are missing in data being decoded they remain as their zero value.

Counter to that though, what if an application needed to distinguish between zero values and an incomplete list?

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Oct 28, 2017

I think that if we add support for decoding arrays, it should return an error if the number of elements is not exactly the length of the array. If you aren't sure of how exactly many elements you are getting, you should be using a slice, not an array.

@leighmcculloch

This comment has been minimized.

Contributor

leighmcculloch commented Oct 28, 2017

👍 That makes sense to me. It simplifies the story that has to be told about its behavior since all scenarios have the same requirement - all or nothing.

If this gets approved I'm happy to put together a CL for it if @ITR13 want planning to.

@ITR13

This comment has been minimized.

ITR13 commented Oct 29, 2017

Sounds good to me, what's a CL?

@leighmcculloch

This comment has been minimized.

Contributor

leighmcculloch commented Oct 29, 2017

A CL (change list) is a set of changes pushed to Gerrit for review, like a PR (pull request) on GitHub.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Jun 13, 2018

@rsc rsc modified the milestones: Go1.12, Go1.13 Nov 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment