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: implement dual of 'omitempty' for unmarshalling «optional» fields #13417

Closed
flyingmutant opened this issue Nov 27, 2015 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@flyingmutant
Copy link
Contributor

XML is often used in legacy APIs, which are often not very consistent in the data format they output. One of such inconsistencies is emitting of nodes with empty data in case it is missing, so <tag>123</tag> becomes <tag></tag>, instead of being omitted entirely.

If node data type is non-string, the only way to handle this in Go currently is to mark node data type as string, and handle the parsing by hand. This is verbose, error-prone and in contrast with the rest of parsing, which is quite declarative.

Go 1.5.1 // #8333, #8334

@flyingmutant
Copy link
Contributor Author

To be precise, what I'd like to have is

type Foo struct {
        Bar *int `xml:"BAR,allowempty"`
}

which will parse <BAR></BAR> into Foo{nil}.

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Nov 29, 2015
@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 7, 2016
@rsc rsc modified the milestones: Go1.9Early, Go1.8 Oct 26, 2016
@s-mang
Copy link
Contributor

s-mang commented Mar 17, 2017

I am happy to take this if nobody is working on it already.

I would think that the unmarshaling behavior for <Foo><BAR></BAR></Foo> should be similar to that of encoding/json’s for {"BAR": null}. Both should be unmarshaled to Foo{Bar: (*int)(nil)}. No need to add a new struct field tag option.

Is there a use case that I’m missing for the error that unmarshaling <BAR></BAR> causes when dst is non-string?

Please correct me if I misunderstand the issue.

@s-mang
Copy link
Contributor

s-mang commented Mar 20, 2017

Nvm. ^ goes against xml spec.

@gopherbot
Copy link

CL https://golang.org/cl/38386 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/39607 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 5, 2017
…ntax, like Go 1.7

Consider this struct, which expects an attribute A and a child C both ints:

    type X struct {
        XMLName xml.Name `xml:"X"`
        A       int      `xml:",attr"`
        C       int
    }

Go 1.2 through Go 1.7 were consistent: attributes unchecked,
children strictly checked:

    $ go1.7 run /tmp/x.go
    <X></X>              ok
    <X A=""></X>         ok
    <X A="bad"></X>      ok
    <X></X>              ok
    <X><C></C></X>       ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C/></X>          ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

Go 1.8 made attributes strictly checked, matching children:

    $ go1.8 run /tmp/x.go
    <X></X>              ok
    <X A=""></X>         ERROR strconv.ParseInt: parsing "": invalid syntax
    <X A="bad"></X>      ERROR strconv.ParseInt: parsing "bad": invalid syntax
    <X></X>              ok
    <X><C></C></X>       ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C/></X>          ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

but this broke XML code that had empty attributes (#19333).

In Go 1.9 we plan to start allowing empty children (#13417).
The fix for that will also make empty attributes work again:

    $ go run /tmp/x.go  # Go 1.9 development
    <X></X>              ok
    <X A=""></X>         ok
    <X A="bad"></X>      ERROR strconv.ParseInt: parsing "bad": invalid syntax
    <X></X>              ok
    <X><C></C></X>       ok
    <X><C/></X>          ok
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

For Go 1.8.1, we want to restore the empty attribute behavior
to match Go 1.7 but not yet change the child behavior as planned for Go 1.9,
since that change hasn't been through release testing.

Instead, restore the more lax Go 1.7 behavior, so that XML files
with empty attributes will not be broken until Go 1.9:

    $ go run /tmp/x.go  # after this CL
    <X></X>              ok
    <X A=""></X>         ok
    <X A="bad"></X>      ok
    <X></X>              ok
    <X><C></C></X>       ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C/></X>          ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

Fixes #19333.

Change-Id: I3d38ebd2509f5b6ea3fd4856327f887f9a1a8085
Reviewed-on: https://go-review.googlesource.com/39607
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Sarah Adams <shadams@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Apr 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants