private/protocol: Fix protocol unit test for Go 1.16. (#3790)

Fixes the SDK's handling of XML protocol tests to correctly compare two
XML document strings without mangling the XML namespace.

.travis.yml

@@ -17,6 +17,7 @@ go:
     - 1.13.x
     - 1.14.x
     - 1.15.x
+    - 1.16.x
     - tip
 
 matrix:
@@ -38,6 +39,8 @@ matrix:
           go: 1.14.x
         - os: windows
           go: 1.15.x
+        - os: windows
+          go: 1.16.x
         - os: windows
           go: tip
 
@@ -46,7 +49,8 @@ before_install:
 
 script:
   - if [ -z $(go env GOMOD) ]; then
-      if [ "$TRAVIS_GO_VERSION" == "1.13.x" ] ||
+      if [ "$TRAVIS_GO_VERSION" == "1.14.x" ] ||
+          [ "$TRAVIS_GO_VERSION" == "1.13.x" ] ||
           [ "$TRAVIS_GO_VERSION" == "1.12.x" ] ||
           [ "$TRAVIS_GO_VERSION" == "1.11.x" ] ||
           [ "$TRAVIS_GO_VERSION" == "1.10.x" ]; then
@@ -55,8 +59,8 @@ script:
       make get-deps;
     fi;
     if [ "$TRAVIS_GO_VERSION" == "tip" ] ||
-        [ "$TRAVIS_GO_VERSION" == "1.15.x" ] ||
-        [ "$TRAVIS_GO_VERSION" == "1.14.x" ]; then
+        [ "$TRAVIS_GO_VERSION" == "1.16.x" ] ||
+        [ "$TRAVIS_GO_VERSION" == "1.15.x" ]; then
 
       if [ "$TRAVIS_OS_NAME" = "windows" ]; then
         make unit-no-verify;

Makefile

@@ -187,6 +187,13 @@ sandbox-go1.15: sandbox-build-go1.15
 sandbox-test-go1.15: sandbox-build-go1.15
 	docker run -t aws-sdk-go-1.15
 
+sandbox-build-go1.16:
+	docker build -f ./awstesting/sandbox/Dockerfile.test.go1.16 -t "aws-sdk-go-1.16" .
+sandbox-go1.16: sandbox-build-go1.16
+	docker run -i -t aws-sdk-go-1.16 bash
+sandbox-test-go1.16: sandbox-build-go1.16
+	docker run -t aws-sdk-go-1.16
+
 sandbox-build-gotip:
 	@echo "Run make update-aws-golang-tip, if this test fails because missing aws-golang:tip container"
 	docker build -f ./awstesting/sandbox/Dockerfile.test.gotip -t "aws-sdk-go-tip" .

aws/credentials/credentials_bench_test.go

@@ -25,7 +25,7 @@ func BenchmarkCredentials_Get(b *testing.B) {
 					for j := 0; j < b.N; j++ {
 						v, err := creds.Get()
 						if err != nil {
-							b.Fatalf("expect no error %v, %v", v, err)
+							b.Errorf("expect no error %v, %v", v, err)
 						}
 					}
 					wg.Done()
@@ -55,7 +55,7 @@ func BenchmarkCredentials_Get_Expire(b *testing.B) {
 						for j := 0; j < b.N; j++ {
 							v, err := creds.Get()
 							if err != nil {
-								b.Fatalf("expect no error %v, %v", v, err)
+								b.Errorf("expect no error %v, %v", v, err)
 							}
 							// periodically expire creds to cause rwlock
 							if id == 0 && j%expRate == 0 {

awstesting/assert.go

@@ -1,9 +1,7 @@
 package awstesting
 
 import (
-	"bytes"
 	"encoding/json"
-	"encoding/xml"
 	"fmt"
 	"net/url"
 	"reflect"
@@ -12,7 +10,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil"
+	"github.com/aws/aws-sdk-go/internal/smithytesting"
 )
 
 // Match is a testing helper to test for testing error by comparing expected
@@ -114,26 +112,14 @@ func AssertJSON(t *testing.T, expect, actual string, msgAndArgs ...interface{})
 	return equal(t, expectVal, actualVal, msgAndArgs...)
 }
 
-// AssertXML verifies that the expect xml string matches the actual.
-// Elements in actual must match the order they appear in expect to match equally
+// AssertXML verifies that the expect XML string matches the actual.
 func AssertXML(t *testing.T, expect, actual string, msgAndArgs ...interface{}) bool {
-	buffer := bytes.NewReader([]byte(expect))
-	decoder := xml.NewDecoder(buffer)
-
-	expectVal, err := xmlutil.XMLToStruct(decoder, nil)
-	if err != nil {
-		t.Fatalf("failed to umarshal xml to struct: %v", err)
-	}
-
-	buffer = bytes.NewReader([]byte(actual))
-	decoder = xml.NewDecoder(buffer)
-
-	actualVal, err := xmlutil.XMLToStruct(decoder, nil)
-	if err != nil {
-		t.Fatalf("failed to umarshal xml to struct: %v", err)
+	if err := smithytesting.XMLEqual([]byte(expect), []byte(actual)); err != nil {
+		t.Error("expect XML match", err, messageFromMsgAndArgs(msgAndArgs))
+		return false
 	}
 
-	return equal(t, expectVal, actualVal, msgAndArgs...)
+	return true
 }
 
 // DidPanic returns if the function paniced and returns true if the function paniced.

awstesting/sandbox/Dockerfile.test.go1.16

@@ -0,0 +1,12 @@
+FROM golang:1.16
+
+ENV GOPROXY=direct
+
+ADD . /go/src/github.com/aws/aws-sdk-go
+
+RUN apt-get update && apt-get install -y --no-install-recommends \
+		vim \
+	&& rm -rf /var/list/apt/lists/*
+
+WORKDIR /go/src/github.com/aws/aws-sdk-go
+CMD ["make", "get-deps-verify", "unit"]

internal/smithytesting/document.go

@@ -0,0 +1,32 @@
+package smithytesting
+
+import (
+	"bytes"
+	"fmt"
+	"reflect"
+
+	"github.com/aws/aws-sdk-go/internal/smithytesting/xml"
+)
+
+// XMLEqual asserts two XML documents by sorting the XML and comparing the
+// strings It returns an error in case of mismatch or in case of malformed XML
+// found while sorting.  In case of mismatched XML, the error string will
+// contain the diff between the two XML documents.
+func XMLEqual(expectBytes, actualBytes []byte) error {
+	actualString, err := xml.SortXML(bytes.NewBuffer(actualBytes), true)
+	if err != nil {
+		return err
+	}
+
+	expectString, err := xml.SortXML(bytes.NewBuffer(expectBytes), true)
+	if err != nil {
+		return err
+	}
+
+	if !reflect.DeepEqual(expectString, actualString) {
+		return fmt.Errorf("unexpected XML mismatch\nexpect: %+v\nactual: %+v",
+			expectString, actualString)
+	}
+
+	return nil
+}

internal/smithytesting/document_test.go

@@ -0,0 +1,157 @@
+// +build go1.9
+
+package smithytesting
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestEqualXMLUtil(t *testing.T) {
+	cases := map[string]struct {
+		expectedXML string
+		actualXML   string
+		expectErr   string
+	}{
+		"empty": {
+			expectedXML: ``,
+			actualXML:   ``,
+		},
+		"emptyWithDiff": {
+			expectedXML: ``,
+			actualXML:   `<Root></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"simpleXML": {
+			expectedXML: `<Root></Root>`,
+			actualXML:   `<Root></Root>`,
+		},
+		"simpleXMLWithDiff": {
+			expectedXML: `<Root></Root>`,
+			actualXML:   `<Root>abc</Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"nestedXML": {
+			expectedXML: `<Root><abc>123</abc><cde>xyz</cde></Root>`,
+			actualXML:   `<Root><abc>123</abc><cde>xyz</cde></Root>`,
+		},
+		"nestedXMLWithExpectedDiff": {
+			expectedXML: `<Root><abc>123</abc><cde>xyz</cde><pqr>234</pqr></Root>`,
+			actualXML:   `<Root><abc>123</abc><cde>xyz</cde></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"nestedXMLWithActualDiff": {
+			expectedXML: `<Root><abc>123</abc><cde>xyz</cde></Root>`,
+			actualXML:   `<Root><abc>123</abc><cde>xyz</cde><pqr>234</pqr></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"Array": {
+			expectedXML: `<Root><list><member><nested>xyz</nested></member><member><nested>abc</nested></member></list></Root>`,
+			actualXML:   `<Root><list><member><nested>xyz</nested></member><member><nested>abc</nested></member></list></Root>`,
+		},
+		"ArrayWithSecondDiff": {
+			expectedXML: `<Root><list><member><nested>xyz</nested></member><member><nested>123</nested></member></list></Root>`,
+			actualXML:   `<Root><list><member><nested>xyz</nested></member><member><nested>345</nested></member></list></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"ArrayWithFirstDiff": {
+			expectedXML: `<Root><list><member><nested>abc</nested></member><member><nested>345</nested></member></list></Root>`,
+			actualXML:   `<Root><list><member><nested>xyz</nested></member><member><nested>345</nested></member></list></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"ArrayWithMixedDiff": {
+			expectedXML: `<Root><list><member><nested>345</nested></member><member><nested>xyz</nested></member></list></Root>`,
+			actualXML:   `<Root><list><member><nested>xyz</nested></member><member><nested>345</nested></member></list></Root>`,
+		},
+		"ArrayWithRepetitiveMembers": {
+			expectedXML: `<Root><list><member><nested>xyz</nested></member><member><nested>xyz</nested></member></list></Root>`,
+			actualXML:   `<Root><list><member><nested>xyz</nested></member><member><nested>xyz</nested></member></list></Root>`,
+		},
+		"Map": {
+			expectedXML: `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+			actualXML:   `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+		},
+		"MapWithFirstDiff": {
+			expectedXML: `<Root><map><entry><key>bcf</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+			actualXML:   `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"MapWithSecondDiff": {
+			expectedXML: `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>abc</value></entry></map></Root>`,
+			actualXML:   `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"MapWithMixedDiff": {
+			expectedXML: `<Root><map><entry><key>cde</key><value>356</value></entry><entry><key>abc</key><value>123</value></entry></map></Root>`,
+			actualXML:   `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+		},
+		"MismatchCheckforKeyValue": {
+			expectedXML: `<Root><map><entry><key>cde</key><value>abc</value></entry><entry><key>abc</key><value>356</value></entry></map></Root>`,
+			actualXML:   `<Root><map><entry><key>abc</key><value>123</value></entry><entry><key>cde</key><value>356</value></entry></map></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"MixMapAndListNestedXML": {
+			expectedXML: `<Root><list>mem1</list><list>mem2</list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+			actualXML:   `<Root><list>mem1</list><list>mem2</list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+		},
+		"MixMapAndListNestedXMLWithDiff": {
+			expectedXML: `<Root><list>mem1</list><list>mem2</list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+			actualXML:   `<Root><list>mem1</list><list>mem2</list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><beta><x>gamma</x></beta></nested></v></map></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"xmlWithNamespaceAndAttr": {
+			expectedXML: `<Root xmlns:ab="https://example.com" attr="apple">value</Root>`,
+			actualXML:   `<Root xmlns:ab="https://example.com" attr="apple">value</Root>`,
+		},
+		"xmlUnorderedAttributes": {
+			expectedXML: `<Root atr="banana" attrNew="apple">v</Root>`,
+			actualXML:   `<Root attrNew="apple" atr="banana">v</Root>`,
+		},
+		"xmlAttributesWithDiff": {
+			expectedXML: `<Root atr="someAtr" attrNew="apple">v</Root>`,
+			actualXML:   `<Root attrNew="apple" atr="banana">v</Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"xmlUnorderedNamespaces": {
+			expectedXML: `<Root xmlns:ab="https://example.com" xmlns:baz="https://example2.com">v</Root>`,
+			actualXML:   `<Root xmlns:baz="https://example2.com" xmlns:ab="https://example.com">v</Root>`,
+		},
+		"xmlNamespaceWithDiff": {
+			expectedXML: `<Root xmlns:ab="https://example-diff.com" xmlns:baz="https://example2.com">v</Root>`,
+			actualXML:   `<Root xmlns:baz="https://example2.com" xmlns:ab="https://example.com">v</Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"NestedWithNamespaceAndAttributes": {
+			expectedXML: `<Root xmlns:ab="https://example.com" xmlns:un="https://example2.com" attr="test" attr2="test2"><ab:list>mem1</ab:list><ab:list>mem2</ab:list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+			actualXML:   `<Root xmlns:ab="https://example.com" xmlns:un="https://example2.com" attr="test" attr2="test2"><ab:list>mem1</ab:list><ab:list>mem2</ab:list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+		},
+		"NestedWithNamespaceAndAttributesWithDiff": {
+			expectedXML: `<Root xmlns:ab="https://example.com" xmlns:un="https://example2.com" attr="test" attr2="test2"><list>mem2</list><ab:list>mem2</ab:list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+			actualXML:   `<Root xmlns:ab="https://example.com" xmlns:un="https://example2.com" attr="test" attr2="test2"><list>mem1</list><ab:list>mem2</ab:list><map><k>abc</k><v><nested><enorm>abc</enorm></nested></v><k>xyz</k><v><nested><alpha><x>gamma</x></alpha></nested></v></map></Root>`,
+			expectErr:   "XML mismatch",
+		},
+		"MalformedXML": {
+			expectedXML: `<Root><fmap><key>a</key><key2>a2</key2><value>v</value></fmap><fmap><key>b</key><key2>b2</key2><value>w</value></fmap></Root>`,
+			actualXML:   `<Root><fmap><key>a</key><key2>a2</key2><value>v</value></fmap><fmap><key>b</key><key2>b2</key2><value>w</value></fmap></Root>`,
+			expectErr:   "malformed xml",
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			actual := []byte(c.actualXML)
+			expected := []byte(c.expectedXML)
+
+			err := XMLEqual(actual, expected)
+			if err != nil {
+				if len(c.expectErr) == 0 {
+					t.Fatalf("expected no error while parsing xml, got %v", err)
+				} else if !strings.Contains(err.Error(), c.expectErr) {
+					t.Fatalf("expected expected XML err to contain %s, got %s", c.expectErr, err.Error())
+				}
+			} else if len(c.expectErr) != 0 {
+				t.Fatalf("expected error %s, got none", c.expectErr)
+			}
+		})
+	}
+}

internal/smithytesting/xml/doc.go

@@ -0,0 +1,7 @@
+// Package xml is XML testing package that supports XML comparison utility.
+// The package consists of ToStruct and StructToXML utils that help sort XML
+// elements as per their nesting level. ToStruct function converts an XML
+// document into a sorted tree node structure, while StructToXML converts the
+// sorted XML nodes into a sorted XML document.  SortXML function should be
+// used to sort an XML document. It can be configured to ignore indentation
+package xml

internal/smithytesting/xml/sort.go

@@ -0,0 +1,48 @@
+package xml
+
+import (
+	"bytes"
+	"encoding/xml"
+	"io"
+	"strings"
+)
+
+type xmlAttrSlice []xml.Attr
+
+func (x xmlAttrSlice) Len() int {
+	return len(x)
+}
+
+func (x xmlAttrSlice) Less(i, j int) bool {
+	spaceI, spaceJ := x[i].Name.Space, x[j].Name.Space
+	localI, localJ := x[i].Name.Local, x[j].Name.Local
+	valueI, valueJ := x[i].Value, x[j].Value
+
+	spaceCmp := strings.Compare(spaceI, spaceJ)
+	localCmp := strings.Compare(localI, localJ)
+	valueCmp := strings.Compare(valueI, valueJ)
+
+	if spaceCmp == -1 || (spaceCmp == 0 && (localCmp == -1 || (localCmp == 0 && valueCmp == -1))) {
+		return true
+	}
+
+	return false
+}
+
+func (x xmlAttrSlice) Swap(i, j int) {
+	x[i], x[j] = x[j], x[i]
+}
+
+// SortXML sorts the reader's XML elements
+func SortXML(r io.Reader, ignoreIndentation bool) (string, error) {
+	var buf bytes.Buffer
+	d := xml.NewDecoder(r)
+	root, err := ToStruct(d, nil, ignoreIndentation)
+	if err != nil {
+		return buf.String(), err
+	}
+
+	e := xml.NewEncoder(&buf)
+	err = StructToXML(e, root, true)
+	return buf.String(), err
+}