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.

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: foo>bar,attr - foo ignored #3688

Open
gopherbot opened this issue May 30, 2012 · 35 comments

Comments

@gopherbot
Copy link

commented May 30, 2012

by krolaw:

see - http://play.golang.org/p/KIbSFNO0Sz

In the case of foo>bar,attr 
foo is ignored in both Marshal an Unmarshal.  This behavior contradicts tags.

Thanks.
@robpike

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2012

Comment 1:

Labels changed: added priority-soon, removed priority-triage.

Owner changed to builder@golang.org.

Status changed to Accepted.

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2012

Comment 2:

I think this was fixed in go 1.0.2. If you run the playground example above it now
passes.

Status changed to WaitingForReply.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Jun 29, 2012

Comment 3 by krolaw:

Unfortunately it's not fixed.  The attributes should be read from, and be output to, the
getBookings element, but instead they wind up in the root element.  Hence foo being
ignored.
Thanks.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Jun 29, 2012

Comment 4 by krolaw:

Here's a better example:
http://play.golang.org/p/VrtQ0GSF1v
@gopherbot

This comment has been minimized.

Copy link
Author

commented Aug 9, 2012

Comment 5 by lost.goblin:

Ping? Seems the Status at least needs updating?
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 3, 2012

Comment 6 by allard.guy.m:

This is an interesting "bug" ....
I agree, the status does need to be updated.  WaitingForReply is certainly incorrect.
In the above examples, this can be 'fixed' by slightly changing the data definitions.
See:
http://play.golang.org/p/VE74VQoJ7c
The == comparison still fails.  As it should.  
But actual results on Unmarshal and MarshalIndent seem to be correct by visual
inspection.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 5, 2012

Comment 7 by krolaw:

Your fix was helpful to me, and my example needs more work.  Suggestions welcome.
However, it would be great if the xml package was consistent with elements and
attributes, for example:
root>data>value works for
<root><data><value>tada</value></data></root>
root>data>value,attr should imho work for <root><data
value="tada"/></root>
It would mean you wouldn't need to create struct stepping stones to the attribute.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 5, 2012

Comment 8 by allard.guy.m:

I am glad you found that useful.
Using == to compare results seems .... perhaps not the best approach given output from
my example above.  Note however, that the == comparison should show true if you add
</getBookings> to the raw input properly.
Should the xml package handle your original input?  My current thinking is: yes.  But if
it will not or can not, then that behavior needs to be somehow documented at least.
The source for the xml package is a bit daunting.  To me anyway.  In particular note the
one big BUG comment by rsc in read.go. That makes it very unclear to me what the go team
is thinking in regards to the package as a whole.
Good luck with this.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 5, 2012

Comment 9 by krolaw:

Here's a better test: http://play.golang.org/p/KN6MWrvFJD
It compares the xml output of both our structs, which should be identical.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 5, 2012

Comment 10 by allard.guy.m:

Nice.  However .....
After thinking / reading more I am not convinced that what you are seeing is totally
incorrect.
The godoc for the xml packages says (in several different places):
<godoc>
a non-pointer anonymous struct field is handled as if the
fields of its value were part of the outer struct.
</godoc>
I now think that has something to do with what you are seeing.
I originally thought this was a problem with Unmarshal, and if that could be corrected
then Marshal would behave as might be expected.  That was incorrect.  Consider:
http://play.golang.org/p/4PCdcVY8Az
That begs the question of why the Unmarshalled attribute values in your original example
appear to be incorrect.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 6, 2012

Comment 11 by krolaw:

I'm aware of that behaviour, hence the name of the issue, foo is being ignored.  My
example fails because the attributes are being retrieved from the root tag (where there
are none), instead of the getBookings tag.  Thus when you Marshall the data, foo again
is ignored and the attributes end up in the root tag, instead of where I want.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2012

Comment 12:

Labels changed: added go1.1.

@niemeyer

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2012

Comment 13:

When fixing this, it should be possible to specify an attribute that doesn't match the
field name, along the lines of:
    `bson:"div>img,src,attr"
@niemeyer

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2012

Comment 14:

Hmmm, or perhaps:
    `bson:"div>img>src,attr"`
Given that this works:
     `bson:"src,attr"
as a counterpart of:
     `bson:"src"`
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2012

Comment 15:

Labels changed: added size-m.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2013

Comment 16:

Labels changed: added priority-later, removed priority-soon.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2013

Comment 17:

For Go 1.1, I think all we can do is make foo>bar,attr an error (can't use > and ,attr
together).
The real fix is too invasive this late in the game.
I filed issue #5033 for the error addition. Leaving this bug open for the real fix.

Labels changed: added go1.2, removed go1.1.

Owner changed to ---.

Status changed to Accepted.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 18:

Labels changed: added feature.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2013

Comment 19:

Too late for 1.2.

Labels changed: removed go1.2.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2013

Comment 20:

Labels changed: added go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2013

Comment 21:

Labels changed: removed feature.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

Comment 22:

Labels changed: added release-none, removed go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

Comment 23:

Labels changed: added repo-main.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Mar 1, 2014

Comment 24 by benjamin.n.damm:

This would be a really nice addition to the XML package.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2014

Comment 25:

Labels changed: added suggested, removed priority-later.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@max-pwg

This comment has been minimized.

Copy link

commented May 17, 2015

I also think this suggestion would be a great improvement to the XML parser

@sorenhoyer

This comment has been minimized.

Copy link

commented Jun 1, 2015

I also think this would be an important fix. Currently this is not as straightforward as it should be:

Members []int 'xml:"members>member,id,attr"'

<members>
    <member id="1"/>
    <member id="2"/>
</members>

which is basically going to target the id attribute inside the member elements.

Error: "members>member chain not valid with id attr flag"

I also tried it this way, although I'd think the first was more logically "correct" members>member>id,attr

Error: "id chain not valid with attr flag"

I was hoping in the 2nd attempt, that Go would automagically check for attributes if no child elements matched.

I might look into a fix, if noone else is working on this.

@ivankravchenko

This comment has been minimized.

Copy link

commented Apr 24, 2016

I tried to solve it for two use cases and came up with something: https://gist.github.com/ivankravchenko/036f68e671e33179b636bd58f6ebc9d0
There is a little bit of code duplication though. Slice handling code could be extracted from unmarshal and unmarshalPath into helper method.
I would appreciate any feedback or suggestions.

@FrenchBen

This comment has been minimized.

Copy link

commented Sep 6, 2016

More than 4 years later, Golang still can't Unmarshall XML attr properly?
https://play.golang.org/p/ndPkqPdizp

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 19, 2016

Everybody, see https://golang.org/wiki/NoMeToo.

If you have something unique to add here, please share, but otherwise use the emoji voting reactions at the top of this bug.

Fixes welcome. Most Go hackers find ourselves using XML much less than we needed to years ago.

@iWdGo

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2018

Refering to the last example, any usage of a namespace must be declared before its use (https://www.w3.org/TR/xml-names/#dt-NSName). Unmarshaling fails as no namespace is explicitly bound. A valid version is like
<entry xmlns:yt="yt"> <yt:location>SI</yt:location> <yt:statistics xmlns:subscriber="subscriber" xmlns:totalUpload="totalUpload" lastWebAccess="1970-01-01T00:00:00.000Z" subscriber:Count="739" videoWatchCount="0" viewCount="0" totalUpload:Views="86772" /> <yt:username>foo</yt:username> </entry>

Other issues are duplicates.

@kempjeng

This comment has been minimized.

Copy link

commented Nov 1, 2018

More than 4 years later, Golang still can't Unmarshall XML attr properly?
https://play.golang.org/p/ndPkqPdizp

https://play.golang.org/p/Js4cIw_NVo1

@FrenchBen

This comment has been minimized.

Copy link

commented Nov 16, 2018

@kempjeng

https://play.golang.org/p/Js4cIw_NVo1

what if you have the same attribute under 2 different namespace?
totalUpload:Views='86772' and currenUpload:Views='172' - How would you handle this?

@kempjeng

This comment has been minimized.

Copy link

commented Nov 16, 2018

@FrenchBen

what if you have the same attribute under 2 different namespace?
totalUpload:Views='86772' and currenUpload:Views='172' - How would you handle this?

Then I would make sure the XML is well formed, and namespace the attributes

https://play.golang.org/p/9XiE3nLpH17

@FrenchBen

This comment has been minimized.

Copy link

commented Nov 16, 2018

@kempjeng 🌮 🌮 🌮
I expected the : to carry into the struct definition, but I see that I was incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.