Work around buggy encloding/xml ns attribute marshalling

[ieure]

Go’s encoding/xml handling of namespaces is deeply broken. In particular, it can’t marshal XML using namespace prefixes, ala:

<document xmlns:ns="whatever">
    <ns:node ns:type="foo">…</ns:node>
</document>

Only by setting the default namespace, ala:

<document xmlns="whatever">
    <node type="foo">…</node>
</document>

This is broken. Per the XML Specification, default namespaces don’t affect attributes; they MUST have a prefix:

The namespace name for an unprefixed attribute name always has no value.

This causes any attributes in WSDL requests to end up in the wrong namespace, which makes them fail validation & return a SOAP fault.

The only way to get attributes into the correct namespace is to include the namespace in the xml:"…" struct field, ala:

Type string `xml:"http://namespace.uri/whatever type,attr,omitempty" json:"type,omitempty"`

This has the unfortunate side-effect of generating overly verbose output:

<document xmlns="whatever">
    <node xmlns:ns="whatever" ns:type="foo">…</node>
</document>

However, it’s actually semantically correct, therefore an improvement over the current situation. A proper fix depends on encoding/xml being less hilariously wrong, which is out of scope here.

The implementation approach also leaves much to be desired. The crux of the issue is that attributes are generated in a sub-template which is called from multiple places (the schema, complex types, etc). The XSDAttribute containing the attr information used by that template doesn’t — and shouldn’t — include the namespace it’s scoped under. Ideally, I’d be able to thread the schema’s namespace down through the templates, but of course text/template doens’t support passing more than one argument to any template. Rather than create a bunch of types containing the namespace and whatever data that template needs (or one using namespace and interface{}), and functions to allocate those to pass to the sub-templates, I opted to use mutable state. GoWSDL now has currentNamespace and get/set methods, which allow accessing the ns from the attribute template. This is not a very sound approach to the problem, in my opinion, but it seems like a smart trade to opt for concrete implementation concerns over abstract ideological ones.

[c4milo]

Thanks for the well researched context @ieure and for the patch! I will try to get to it this week. 🙏

[ieure]

@c4milo Thanks for the reply. If there's any chance of getting this and #214 merged and a new release cut, that'd be terrific. Those two, and the fix from #208, are blockers for my usecase. For now, I've vendored 9e1cc9a7689230f4da66504844af631a0c4767cc with patches for the two PRs I opened, but I'd like very much to use a clean tagged release.

Having this stuff is a priority for my employer, so if any changes on either PR are needed, I can turn those around quickly.

[c4milo]

OK, that’s good to know, I will review them tomorrow then.

On Mon, Sep 13, 2021 at 10:15 PM Ian Eure @.***> wrote:

@c4milo https://github.com/c4milo Thanks for the reply. If there's any chance of getting this and #214 https://github.com/hooklift/gowsdl/pull/214 merged and a new release cut, that'd be terrific. Those two, and the fix from #208 https://github.com/hooklift/gowsdl/pull/208, are blockers for my usecase. For now, I've vendored 9e1cc9a https://github.com/hooklift/gowsdl/commit/9e1cc9a7689230f4da66504844af631a0c4767cc with patches for the two PRs I opened, but I'd like very much to use a clean tagged release.

Having this stuff is a priority for my employer, so if any changes on either PR are needed, I can turn those around quickly.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/hooklift/gowsdl/pull/218#issuecomment-918737265, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKFUNK4VTQCPUZC3X6IBTUB2V43ANCNFSM5D6N4G3Q .

-- Camilo Aguilar Software Engineer http://github.com/c4milo

[ieure]

Have you had a chance to take a look at this?

[c4milo]

@ieure, I got caught up by end of quarter deadlines but once I'm out of the bushes I will look into this. Apologies!

[w65536]

Not sure if it would really help. But this describes an approach to actually pass multiple arguments to a Go template using a dictionary helper: https://stackoverflow.com/questions/18276173/calling-a-template-with-several-pipeline-parameters. It is clearly a workaround to Go's template implementation. But given the circumstances, I think it is a pretty nice and good "solution".

[w65536]

FYI, I have described another namespace related problem with naming collisions in issue https://github.com/hooklift/gowsdl/issues/223

The problem had already been identified before (https://github.com/hooklift/gowsdl/pull/184). I have attempted to give a more detailed and broader perspective on the problem. In particular I am hoping that by defining the "correct" approach for handling namespaces, we should be able to address as many problems and as much functionality as possible in one go.

[ieure]

FYI, I have described another namespace related problem with naming collisions in issue #223

The problem had already been identified before (#184). I have attempted to give a more detailed and broader perspective on the problem. In particular I am hoping that by defining the "correct" approach for handling namespaces, we should be able to address as many problems and as much functionality as possible in one go.

Thanks for the context. That looks like a different issue than what I'm discussing here. The root cause of the issue I laid out in this PR is that Go's encoding/xml is broken, and its brokenness seeps into gowsdl. This is just working around the problem, because nobody seems to know what to do about encoding/xml and there seem to be doubts that it can be fixed at all without a full rewrite.

For #223, the issue is 100% gowsdl. It should probably put each ns' schema into its own subpackage based on its ns name, so you have svcX.DuplicateComplexType and svc1.DuplicateComplexType and the correct imports get added as needed.

[w65536]

@ieure Thanks for your clear statement. Makes sense. Yes, yours is definitely a different problem. The reason I mentioned the naming collision here, is that independently of your solution, I had attempted to take a similar approach to solving my problem (although without immediate success). But it might very well be the wrong approach for that matter.