From 48d1a5ab36b0013117756b281176e12d9b5ae4fb Mon Sep 17 00:00:00 2001 From: David Arroyo Date: Thu, 3 Jan 2019 18:18:29 -0500 Subject: [PATCH] Fix bug when modifying the last type declaration in a schema. 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 ... to ... in a separate pass, rather than doing it in two places (the `copyEltNamesToAnonymousTypes` and `nameAnonymousTypes` functions). --- xsd/parse.go | 16 +++++++++------- xsd/testdata/CopyElementName.json | 22 ++++++++++++++++++++++ xsd/testdata/CopyElementName.xsd | 17 +++++++++++++++++ 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 xsd/testdata/CopyElementName.json create mode 100644 xsd/testdata/CopyElementName.xsd diff --git a/xsd/parse.go b/xsd/parse.go index 4edde53..c232bd1 100644 --- a/xsd/parse.go +++ b/xsd/parse.go @@ -189,11 +189,10 @@ func anonTypeName(n int, ns string) xml.Name { to - - + + ... - - + */ func copyEltNamesToAnonTypes(root *xmltree.Element) { used := make(map[xml.Name]struct{}) @@ -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) @@ -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...) } /* @@ -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) { @@ -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 } diff --git a/xsd/testdata/CopyElementName.json b/xsd/testdata/CopyElementName.json new file mode 100644 index 0000000..d770de4 --- /dev/null +++ b/xsd/testdata/CopyElementName.json @@ -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}] + } + } + ] + } +} diff --git a/xsd/testdata/CopyElementName.xsd b/xsd/testdata/CopyElementName.xsd new file mode 100644 index 0000000..161c3df --- /dev/null +++ b/xsd/testdata/CopyElementName.xsd @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + +