Skip to content

Commit

Permalink
Fix bug when modifying the last type declaration in a schema.
Browse files Browse the repository at this point in the history
In the original code, there were two calls to

	root.Children = append(root.Children, t)

Because the `Children` field is a `[]xmltree.Element`, rather than an `[]*xmltree.Element`,
if the call to `append()` causes a reallocation, of the underlying array, then the `el` pointer
used within the same loop will no longer point to the current value, and updates to it will
be lost, as the added test demonstrated.

A better change would be to factor out the rearrangement of XMl such as

	<element name="foo">
	  <complexType name="foo">
	    ...
	  </complexType>
	</element>

to

	<element name="foo" type="foo"/>
	<complexType name="foo">
	  ...
	</complexType>

in a separate pass, rather than doing it in two places (the
`copyEltNamesToAnonymousTypes` and `nameAnonymousTypes` functions).
  • Loading branch information
droyo committed Jan 3, 2019
1 parent 0880b4c commit 48d1a5a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 7 deletions.
16 changes: 9 additions & 7 deletions xsd/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,10 @@ func anonTypeName(n int, ns string) xml.Name {
to
<element name="foo" type="foo">
<complexType name="foo">
<element name="foo" type="foo"/>
<complexType name="foo">
...
</complexType>
</element>
</complexType>
*/
func copyEltNamesToAnonTypes(root *xmltree.Element) {
used := make(map[xml.Name]struct{})
Expand All @@ -209,6 +208,7 @@ func copyEltNamesToAnonTypes(root *xmltree.Element) {
hasAttr("", "name"),
hasAnonymousType)

var newChildren []xmltree.Element
for _, el := range root.SearchFunc(eltWithAnonType) {
// Make sure we can use this element's name
xmlname := el.ResolveDefault(el.Attr("", "name"), tns)
Expand All @@ -221,14 +221,14 @@ func copyEltNamesToAnonTypes(root *xmltree.Element) {
continue
}
t.SetAttr("", "name", el.Attr("", "name"))
newChildren = append(newChildren, t)
el.SetAttr("", "type", el.Prefix(xmlname))

el.Children = append(el.Children[:i], el.Children[i+1:]...)
el.Content = nil
root.Children = append(root.Children, t)
break
}
}
root.Children = append(root.Children, newChildren...)
}

/*
Expand Down Expand Up @@ -261,6 +261,7 @@ func nameAnonymousTypes(root *xmltree.Element) error {
typeCounter int
updateAttr string
accum bool
newChildren []xmltree.Element
)
targetNS := root.Attr("", "targetNamespace")
for _, el := range root.SearchFunc(hasAnonymousType) {
Expand Down Expand Up @@ -303,12 +304,13 @@ func nameAnonymousTypes(root *xmltree.Element) error {
el.SetAttr("", updateAttr, qname)
el.Children = append(el.Children[:i], el.Children[i+1:]...)
el.Content = nil
root.Children = append(root.Children, t)
newChildren = append(newChildren, t)
if !accum {
break
}
}
}
root.Children = append(root.Children, newChildren...)
return nil
}

Expand Down
22 changes: 22 additions & 0 deletions xsd/testdata/CopyElementName.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"_self": {
"Elements": [
{
"Name": {"Space": "tns", "Local": "TradePrice"},
"Type": {
"Base": 0,
"Name": {"Local": "TradePrice"},
"Elements": [{"Name": {"Local": "price"}, "Type": 21}]
}
},
{
"Name": {"Space": "tns", "Local": "TradePriceRequest"},
"Type": {
"Base": 0,
"Name": {"Local": "TradePriceRequest"},
"Elements": [{"Name": {"Local": "tickerSymbol"}, "Type": 38}]
}
}
]
}
}
17 changes: 17 additions & 0 deletions xsd/testdata/CopyElementName.xsd
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!-- Where possible (if the type name has not been used already), anonymous types
declared inside an element or attribute should have the same name as their container. -->

<element name="TradePrice">
<complexType>
<all>
<element name="price" type="float"/>
</all>
</complexType>
</element>
<element name="TradePriceRequest">
<complexType>
<all>
<element name="tickerSymbol" type="string"/>
</all>
</complexType>
</element>

0 comments on commit 48d1a5a

Please sign in to comment.