From 020ae25e916d6bfeb48ac0b1fc3935086fe8e161 Mon Sep 17 00:00:00 2001 From: Ian Eure Date: Mon, 13 Sep 2021 10:26:39 -0700 Subject: [PATCH 1/2] Work around buggy encloding/xml ns attribute marshalling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go’s `encoding/xml` handling of namespaces is deeply broken. In particular, it can’t marshal XML using namespace prefixes, ala: ```xml … ``` Only by setting the default namespace, ala: ```xml … ``` This is broken. Per [the XML Specification](https://www.w3.org/TR/xml-names/#dt-defaultNS), 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: ```go Type string `xml:"http://namespace.uri/whatever type,attr,omitempty" json:"type,omitempty"` ``` This has the unfortunate side-effect of generating overly verbose output: ```xml … ``` 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. --- gowsdl.go | 14 ++++++++++++++ types_tmpl.go | 7 ++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/gowsdl.go b/gowsdl.go index 8b6f7ba..0c2bbe8 100644 --- a/gowsdl.go +++ b/gowsdl.go @@ -35,6 +35,18 @@ type GoWSDL struct { wsdl *WSDL resolvedXSDExternals map[string]bool currentRecursionLevel uint8 + currentNamespace string +} + +// Method setNS sets (and returns) the currently active XML namespace. +func (g *GoWSDL) setNS(ns string) string { + g.currentNamespace = ns + return ns +} + +// Method setNS returns the currently active XML namespace. +func (g *GoWSDL) getNS() string { + return g.currentNamespace } var cacheDir = filepath.Join(os.TempDir(), "gowsdl-cache") @@ -268,6 +280,8 @@ func (g *GoWSDL) genTypes() ([]byte, error) { "goString": goString, "findNameByType": g.findNameByType, "removePointerFromType": removePointerFromType, + "setNS": g.setNS, + "getNS": g.getNS, } data := new(bytes.Buffer) diff --git a/types_tmpl.go b/types_tmpl.go index 3d0396c..5501ce7 100644 --- a/types_tmpl.go +++ b/types_tmpl.go @@ -43,12 +43,13 @@ var typesTmpl = ` {{end}} {{define "Attributes"}} + {{ $targetNamespace := getNS }} {{range .}} {{if .Doc}} {{.Doc | comment}} {{end}} {{ if ne .Type "" }} - {{ normalize .Name | makeFieldPublic}} {{toGoType .Type false}} ` + "`" + `xml:"{{.Name}},attr,omitempty" json:"{{.Name}},omitempty"` + "`" + ` + {{ normalize .Name | makeFieldPublic}} {{toGoType .Type false}} ` + "`" + `xml:"{{with $targetNamespace}}{{.}} {{end}}{{.Name}},attr,omitempty" json:"{{.Name}},omitempty"` + "`" + ` {{ else }} - {{ normalize .Name | makeFieldPublic}} string ` + "`" + `xml:"{{.Name}},attr,omitempty" json:"{{.Name}},omitempty"` + "`" + ` + {{ normalize .Name | makeFieldPublic}} string ` + "`" + `xml:"{{with $targetNamespace}}{{.}} {{end}}{{.Name}},attr,omitempty" json:"{{.Name}},omitempty"` + "`" + ` {{ end }} {{end}} {{end}} @@ -106,7 +107,7 @@ var typesTmpl = ` {{end}} {{range .Schemas}} - {{ $targetNamespace := .TargetNamespace }} + {{ $targetNamespace := setNS .TargetNamespace }} {{range .SimpleType}} {{template "SimpleType" .}} From 3aaab23e81e6eeb1022bed3d6970ec35bb03c91a Mon Sep 17 00:00:00 2001 From: Ian Eure Date: Mon, 13 Sep 2021 11:31:36 -0700 Subject: [PATCH 2/2] Fix broken tests. --- fixtures/epcis/epcisquery.src | 34 +++++++++++++++++----------------- gowsdl_test.go | 4 ++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/fixtures/epcis/epcisquery.src b/fixtures/epcis/epcisquery.src index 795312b..f01f408 100644 --- a/fixtures/epcis/epcisquery.src +++ b/fixtures/epcis/epcisquery.src @@ -27,13 +27,13 @@ type Document struct { // The version of the schema corresponding to which the instance conforms. // - SchemaVersion float64 `xml:"schemaVersion,attr,omitempty" json:"schemaVersion,omitempty"` + SchemaVersion float64 `xml:"urn:epcglobal:xsd:1 schemaVersion,attr,omitempty" json:"schemaVersion,omitempty"` // // The date the message was created. Used for auditing and logging. // - CreationDate soap.XSDDateTime `xml:"creationDate,attr,omitempty" json:"creationDate,omitempty"` + CreationDate soap.XSDDateTime `xml:"urn:epcglobal:xsd:1 creationDate,attr,omitempty" json:"creationDate,omitempty"` } type EPC string @@ -132,25 +132,25 @@ type BusinessService struct { } type ServiceTransaction struct { - TypeOfServiceTransaction *TypeOfServiceTransaction `xml:"TypeOfServiceTransaction,attr,omitempty" json:"TypeOfServiceTransaction,omitempty"` + TypeOfServiceTransaction *TypeOfServiceTransaction `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader TypeOfServiceTransaction,attr,omitempty" json:"TypeOfServiceTransaction,omitempty"` - IsNonRepudiationRequired string `xml:"IsNonRepudiationRequired,attr,omitempty" json:"IsNonRepudiationRequired,omitempty"` + IsNonRepudiationRequired string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader IsNonRepudiationRequired,attr,omitempty" json:"IsNonRepudiationRequired,omitempty"` - IsAuthenticationRequired string `xml:"IsAuthenticationRequired,attr,omitempty" json:"IsAuthenticationRequired,omitempty"` + IsAuthenticationRequired string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader IsAuthenticationRequired,attr,omitempty" json:"IsAuthenticationRequired,omitempty"` - IsNonRepudiationOfReceiptRequired string `xml:"IsNonRepudiationOfReceiptRequired,attr,omitempty" json:"IsNonRepudiationOfReceiptRequired,omitempty"` + IsNonRepudiationOfReceiptRequired string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader IsNonRepudiationOfReceiptRequired,attr,omitempty" json:"IsNonRepudiationOfReceiptRequired,omitempty"` - IsIntegrityCheckRequired string `xml:"IsIntegrityCheckRequired,attr,omitempty" json:"IsIntegrityCheckRequired,omitempty"` + IsIntegrityCheckRequired string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader IsIntegrityCheckRequired,attr,omitempty" json:"IsIntegrityCheckRequired,omitempty"` - IsApplicationErrorResponseRequested string `xml:"IsApplicationErrorResponseRequested,attr,omitempty" json:"IsApplicationErrorResponseRequested,omitempty"` + IsApplicationErrorResponseRequested string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader IsApplicationErrorResponseRequested,attr,omitempty" json:"IsApplicationErrorResponseRequested,omitempty"` - TimeToAcknowledgeReceipt string `xml:"TimeToAcknowledgeReceipt,attr,omitempty" json:"TimeToAcknowledgeReceipt,omitempty"` + TimeToAcknowledgeReceipt string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader TimeToAcknowledgeReceipt,attr,omitempty" json:"TimeToAcknowledgeReceipt,omitempty"` - TimeToAcknowledgeAcceptance string `xml:"TimeToAcknowledgeAcceptance,attr,omitempty" json:"TimeToAcknowledgeAcceptance,omitempty"` + TimeToAcknowledgeAcceptance string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader TimeToAcknowledgeAcceptance,attr,omitempty" json:"TimeToAcknowledgeAcceptance,omitempty"` - TimeToPerform string `xml:"TimeToPerform,attr,omitempty" json:"TimeToPerform,omitempty"` + TimeToPerform string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader TimeToPerform,attr,omitempty" json:"TimeToPerform,omitempty"` - Recurrence string `xml:"Recurrence,attr,omitempty" json:"Recurrence,omitempty"` + Recurrence string `xml:"http://www.unece.org/cefact/namespaces/StandardBusinessDocumentHeader Recurrence,attr,omitempty" json:"Recurrence,omitempty"` } type StandardBusinessDocumentHeader struct { @@ -268,7 +268,7 @@ type VocabularyType struct { Items []string `xml:",any" json:"items,omitempty"` - Type AnyURI `xml:"type,attr,omitempty" json:"type,omitempty"` + Type AnyURI `xml:"urn:epcglobal:epcis:xsd:1 type,attr,omitempty" json:"type,omitempty"` } type VocabularyElementListType struct { @@ -284,13 +284,13 @@ type VocabularyElementType struct { Items []string `xml:",any" json:"items,omitempty"` - Id AnyURI `xml:"id,attr,omitempty" json:"id,omitempty"` + Id AnyURI `xml:"urn:epcglobal:epcis:xsd:1 id,attr,omitempty" json:"id,omitempty"` } type AttributeType struct { AnyType - Id AnyURI `xml:"id,attr,omitempty" json:"id,omitempty"` + Id AnyURI `xml:"urn:epcglobal:epcis:xsd:1 id,attr,omitempty" json:"id,omitempty"` } type IDListType struct { @@ -378,7 +378,7 @@ type BusinessLocationExtensionType struct { type BusinessTransactionType struct { Value *BusinessTransactionIDType `xml:",chardata" json:"-,"` - Type *BusinessTransactionTypeIDType `xml:"type,attr,omitempty" json:"type,omitempty"` + Type *BusinessTransactionTypeIDType `xml:"urn:epcglobal:epcis:xsd:1 type,attr,omitempty" json:"type,omitempty"` } type BusinessTransactionListType struct { @@ -388,7 +388,7 @@ type BusinessTransactionListType struct { type SourceDestType struct { Value *SourceDestIDType `xml:",chardata" json:"-,"` - Type *SourceDestTypeIDType `xml:"type,attr,omitempty" json:"type,omitempty"` + Type *SourceDestTypeIDType `xml:"urn:epcglobal:epcis:xsd:1 type,attr,omitempty" json:"type,omitempty"` } type SourceListType struct { diff --git a/gowsdl_test.go b/gowsdl_test.go index 5b3d45d..f3b238c 100644 --- a/gowsdl_test.go +++ b/gowsdl_test.go @@ -83,10 +83,10 @@ func TestAttributeRef(t *testing.T) { Status []struct { Value string ` + "`" + `xml:",chardata" json:"-,"` + "`" + ` - Code string ` + "`" + `xml:"code,attr,omitempty" json:"code,omitempty"` + "`" + ` + Code string ` + "`" + `xml:"http://www.mnb.hu/webservices/ code,attr,omitempty" json:"code,omitempty"` + "`" + ` } ` + "`" + `xml:"status,omitempty" json:"status,omitempty"` + "`" + ` - ResponseCode string ` + "`" + `xml:"responseCode,attr,omitempty" json:"responseCode,omitempty"` + "`" + ` + ResponseCode string ` + "`" + `xml:"http://www.mnb.hu/webservices/ responseCode,attr,omitempty" json:"responseCode,omitempty"` + "`" + ` }` actual = string(bytes.ReplaceAll([]byte(actual), []byte("\t"), []byte(" "))) expected = string(bytes.ReplaceAll([]byte(expected), []byte("\t"), []byte(" ")))