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: allow avoiding infinite recursion in EncodeElement #6474

Closed
lukescott opened this Issue Sep 24, 2013 · 5 comments

Comments

Projects
None yet
3 participants
@lukescott
Copy link

lukescott commented Sep 24, 2013

When using MarshalXML if you want to modify just the surrounding tag it's difficult to
do so. Calling EncodeElement results in a recursive error.

The only work around, other than manually marshaling each field, is to typedef the
struct to an alternate type:


type _TransactionRequest TransactionRequest;

func (tr TransactionRequest) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
    return e.EncodeElement(_TransactionRequest(tr), xml.StartElement{
            Name: xml.Name{"", "createTransactionRequest"},
            Attr: []xml.Attr{
                    xml.Attr{xml.Name{"", "xmlns:xsi"}, "http://www.w3.org/2001/XMLSchema-instance";},
                    xml.Attr{xml.Name{"", "xmlns:xsd"}, "http://www.w3.org/2001/XMLSchema";},
                    xml.Attr{xml.Name{"", "xmlns"}, "AnetApi/xml/v1/schema/AnetApiSchema.xsd"},
                },
        })
}


There needs to be a way to Marshal the contents of the struct without the surrounding
tag, perhaps "EncodeBody":


func (tr TransactionRequest) MarshalXML(e *xml.Encoder, start xml.StartElement) (err
error) {
    start = xml.StartElement{
            Name: xml.Name{"", "createTransactionRequest"},
            Attr: []xml.Attr{
                xml.Attr{xml.Name{"", "xmlns:xsi"}, "http://www.w3.org/2001/XMLSchema-instance";},
                xml.Attr{xml.Name{"", "xmlns:xsd"}, "http://www.w3.org/2001/XMLSchema";},
                xml.Attr{xml.Name{"", "xmlns"}, "AnetApi/xml/v1/schema/AnetApiSchema.xsd"},
            },
        }
    if err = e.EncodeToken(start); err != nil {
        return
    }
    if err = e.EncodeBody(tr); err != nil {
        return
    }
    if err = e.EncodeToken(EndElement{start.Name}); err != nil {
        return
    }
}


Which compiler are you using (5g, 6g, 8g, gccgo)?

gc default

Which operating system are you using?

OS X 10.8.5

Which version are you using?  (run 'go version')

go version devel +f4d1cb8d9a91 Thu Sep 19 22:34:33 2013 +1000 darwin/amd64

Please provide any additional information below.
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Sep 27, 2013

Comment 1:

Can you please post a full program demonstrating what you are trying to do (using your
workaround)? I am a bit confused.
Thanks.

Status changed to WaitingForReply.

@lukescott

This comment has been minimized.

Copy link
Author

lukescott commented Oct 1, 2013

Comment 2:

I have a struct with many fields in it:
type TransactionRequest struct {
....
}
Some are strings, some are other structs, etc...
Inside of MarshalXML if I do this (where "tr" is an instance of "TransactionRequest"):
return e.EncodeElement(tr, start)
"EncodeElement" causes the same MarshalXML function to be called recursively. What I
want to do is change "start" (add attributes, change the name, etc...) but marshal the
fields normally. In order to do this, without statically/manually marshaling each field,
I have to change the type, where "_TransactionRequest" is just a type from
"TransactionRequest":
return e.EncodeElement(_TransactionRequest(tr), start)
This prevents the recursion, but requires a new type to reflect.
Ideally what I would like to do is simply call EncodeElement like this and not have it
call MarshalXML again:
return e.EncodeElement(tr, start)
Or have some way of telling it to marshal just the fields inside:
e.EncodeToken(start) // <TransactionResponse ...>
e.EncodeBody(tr) // ... <field>...</field>...
e.EncodeToken(EndElement{start.Name}); // </TransactionResponse>
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Oct 1, 2013

Comment 3:

Thanks for elaborating. I agree: this is bad. We should fix this for Go 1.2.

Labels changed: added priority-later, go1.2, removed priority-triage.

Status changed to Accepted.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Oct 2, 2013

Comment 4:

On second thought I think the second type definition is actually a pretty good way to
handle this. It is the canonical way to change the method set in Go, and changing the
method set is exactly what you are trying to do. We could add new API but using the
types is more general and avoids needing new API in every use case.
Note that if you are using pointer methods then you can still do the conversion from
(*TransactionRequest) to (*rawTransactionRequest). Go allows that specifically because
it enables changing the methods associated with a particular piece of data.

Status changed to WorkingAsIntended.

@lukescott

This comment has been minimized.

Copy link
Author

lukescott commented Oct 2, 2013

Comment 5:

Having to redeclare types is very messy, especially when I have many structs that need
to be defined this way. I also have to incur the cost of having twice the reflection.
Would something like this work?
type printer struct {
    *bufio.Writer
    encoder    *Encoder
    seq        int
    indent     string
    prefix     string
    depth      int
    indentedIn bool
    putNewline bool
    attrNS     map[string]string // map prefix -> name space
    attrPrefix map[string]string // map name space -> prefix
    prefixes   []string
    tags       []Name
    lastValue  reflect.Value
}
if p.lastValue != val {
    // Ensure MarshalXML isn't called a second time
    p.lastValue = val
    // Check for marshaler.
    if val.CanInterface() && typ.Implements(marshalerType) {
        return p.marshalInterface(val.Interface().(Marshaler), defaultStart(typ, finfo, startTemplate))
    }
    if val.CanAddr() {
        pv := val.Addr()
        if pv.CanInterface() && pv.Type().Implements(marshalerType) {
            return p.marshalInterface(pv.Interface().(Marshaler), defaultStart(pv.Type(), finfo, startTemplate))
        }
    }   
}
If if it did it would allow me to call EncodeElement inside MarhsalXML and prevent the
recursion.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015

@rsc rsc removed the go1.2 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 25, 2016

This issue was closed.

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