From fa75a13b00f4f2e53f5466544726699f1678c8d1 Mon Sep 17 00:00:00 2001
From: Lauren Tomasello <laurentomasello@monzo.com>
Date: Sun, 17 Mar 2024 16:05:05 +0000
Subject: [PATCH] Fix issue where adjacent comments were skipped during
 canonicalization

Addresses issue #50

If comments were adjacent in a document with no whitespace between them, then for each pair of comments the second comment would be skipped due to the underlying slice being modified as it was being ranged over.

For example, the following document:

```xml
<a><!-- comment1 --><!-- comment2 --><!-- comment 3 --></a>
```

Would be incorrectly canonicalized to:

```xml
<a><!-- comment2 --></a>
```

This patch fixes that issue and adds a new test case to cover it.
---
 examples/canonicalization_test.go | 4 ++++
 exclusivecanonicalization.go      | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/examples/canonicalization_test.go b/examples/canonicalization_test.go
index 498036d..6baab5e 100644
--- a/examples/canonicalization_test.go
+++ b/examples/canonicalization_test.go
@@ -168,6 +168,9 @@ var example37SubsetExpression = `<!-- Evaluate with declaration xmlns:ietf="http
 
 var example37Output = `<e1 xmlns="http://www.ietf.org" xmlns:w3c="http://www.w3.org"><e3 xmlns="" id="E3" xml:space="preserve"></e3></e1>`
 
+var exampleGHIssue50Input = `<a><!-- comment0 --><!-- comment1 --><!-- comment2 --></a>`
+var exampleGHIssue50Output = `<a></a>`
+
 type exampleXML struct {
 	input        string
 	output       string
@@ -190,6 +193,7 @@ func TestCanonicalizationExamples(t *testing.T) {
 			// http://stackoverflow.com/questions/6002619/unmarshal-an-iso-8859-1-xml-input-in-go
 			// "(Example 3.6)": {input: example36Input, output: example36Output},
 			// "(Example 3.7)": {input: example37Input, output: example37Output, expression: example37SubsetExpression},
+			"(Example from GitHub Issue #50)": {input: exampleGHIssue50Input, output: exampleGHIssue50Output},
 		}
 		for description, test := range cases {
 			Convey(fmt.Sprintf("When transformed %s", description), func() {
diff --git a/exclusivecanonicalization.go b/exclusivecanonicalization.go
index 2df61d6..94e4310 100644
--- a/exclusivecanonicalization.go
+++ b/exclusivecanonicalization.go
@@ -161,7 +161,9 @@ func (e ExclusiveCanonicalization) processRecursive(node *etree.Element,
 
 	newDefaultNS, newPrefixesInScope := e.renderAttributes(node, prefixesInScope, defaultNS)
 
-	for _, child := range node.Child {
+	for i := 0; i < len(node.Child); i++ {
+		child := node.Child[i]
+
 		oldNamespaces := e.namespaces
 		e.namespaces = copyNamespace(oldNamespaces)
 
@@ -169,6 +171,7 @@ func (e ExclusiveCanonicalization) processRecursive(node *etree.Element,
 		case *etree.Comment:
 			if !e.WithComments {
 				removeTokenFromElement(etree.Token(child), node)
+				i--
 			}
 		case *etree.Element:
 			e.processRecursive(child, newPrefixesInScope, newDefaultNS)