From 976a0f5558e20ed7cb7ba2cd68d7429d1ef01db9 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 5 Feb 2020 15:29:59 +0100
Subject: [PATCH] cmd/devp2p: fix Route53 TXT record splitting (#20626)

For longer records and subtree entries, the deployer created two
separate TXT records. This doesn't work as intended because the client
will receive the two records in arbitrary order. The fix is to encode
longer values as "string1""string2" instead of "string1", "string2".
This encoding creates a single record on AWS Route53.
---
 cmd/devp2p/dns_route53.go      | 28 ++++++++--------------------
 cmd/devp2p/dns_route53_test.go |  6 ++----
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/cmd/devp2p/dns_route53.go b/cmd/devp2p/dns_route53.go
index bdad5c2e9c90..71118be54344 100644
--- a/cmd/devp2p/dns_route53.go
+++ b/cmd/devp2p/dns_route53.go
@@ -171,7 +171,7 @@ func (c *route53Client) computeChanges(name string, records map[string]string, e
 		}
 
 		prevRecords, exists := existing[path]
-		prevValue := combineTXT(prevRecords.values)
+		prevValue := strings.Join(prevRecords.values, "")
 		if !exists {
 			// Entry is unknown, push a new one
 			log.Info(fmt.Sprintf("Creating %s = %q", path, val))
@@ -191,8 +191,8 @@ func (c *route53Client) computeChanges(name string, records map[string]string, e
 			continue
 		}
 		// Stale entry, nuke it.
-		log.Info(fmt.Sprintf("Deleting %s = %q", path, combineTXT(set.values)))
-		changes = append(changes, newTXTChange("DELETE", path, set.ttl, set.values))
+		log.Info(fmt.Sprintf("Deleting %s = %q", path, strings.Join(set.values, "")))
+		changes = append(changes, newTXTChange("DELETE", path, set.ttl, set.values...))
 	}
 
 	sortChanges(changes)
@@ -263,7 +263,7 @@ func (c *route53Client) collectRecords(name string) (map[string]recordSet, error
 }
 
 // newTXTChange creates a change to a TXT record.
-func newTXTChange(action, name string, ttl int64, values []string) *route53.Change {
+func newTXTChange(action, name string, ttl int64, values ...string) *route53.Change {
 	var c route53.Change
 	var r route53.ResourceRecordSet
 	var rrs []*route53.ResourceRecord
@@ -288,28 +288,16 @@ func isSubdomain(name, domain string) bool {
 	return strings.HasSuffix("."+name, "."+domain)
 }
 
-// combineTXT concatenates the given quoted strings into a single unquoted string.
-func combineTXT(values []string) string {
-	result := ""
-	for _, v := range values {
-		if v[0] == '"' {
-			v = v[1 : len(v)-1]
-		}
-		result += v
-	}
-	return result
-}
-
 // splitTXT splits value into a list of quoted 255-character strings.
-func splitTXT(value string) []string {
-	var result []string
+func splitTXT(value string) string {
+	var result strings.Builder
 	for len(value) > 0 {
 		rlen := len(value)
 		if rlen > 253 {
 			rlen = 253
 		}
-		result = append(result, strconv.Quote(value[:rlen]))
+		result.WriteString(strconv.Quote(value[:rlen]))
 		value = value[rlen:]
 	}
-	return result
+	return result.String()
 }
diff --git a/cmd/devp2p/dns_route53_test.go b/cmd/devp2p/dns_route53_test.go
index c64f1d169814..f6ab6c1762db 100644
--- a/cmd/devp2p/dns_route53_test.go
+++ b/cmd/devp2p/dns_route53_test.go
@@ -28,8 +28,7 @@ import (
 func TestRoute53ChangeSort(t *testing.T) {
 	testTree0 := map[string]recordSet{
 		"2kfjogvxdqtxxugbh7gs7naaai.n": {ttl: 3333, values: []string{
-			`"enr:-HW4QO1ml1DdXLeZLsUxewnthhUy8eROqkDyoMTyavfks9JlYQIlMFEUoM78PovJDPQrAkrb3LRJ-"`,
-			`"vtrymDguKCOIAWAgmlkgnY0iXNlY3AyNTZrMaEDffaGfJzgGhUif1JqFruZlYmA31HzathLSWxfbq_QoQ4"`,
+			`"enr:-HW4QO1ml1DdXLeZLsUxewnthhUy8eROqkDyoMTyavfks9JlYQIlMFEUoM78PovJDPQrAkrb3LRJ-""vtrymDguKCOIAWAgmlkgnY0iXNlY3AyNTZrMaEDffaGfJzgGhUif1JqFruZlYmA31HzathLSWxfbq_QoQ4"`,
 		}},
 		"fdxn3sn67na5dka4j2gok7bvqi.n": {ttl: treeNodeTTL, values: []string{`"enrtree-branch:"`}},
 		"n":                            {ttl: rootTTL, values: []string{`"enrtree-root:v1 e=2KFJOGVXDQTXXUGBH7GS7NAAAI l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=0 sig=v_-J_q_9ICQg5ztExFvLQhDBGMb0lZPJLhe3ts9LAcgqhOhtT3YFJsl8BWNDSwGtamUdR-9xl88_w-X42SVpjwE"`}},
@@ -116,8 +115,7 @@ func TestRoute53ChangeSort(t *testing.T) {
 			ResourceRecordSet: &route53.ResourceRecordSet{
 				Name: sp("2kfjogvxdqtxxugbh7gs7naaai.n"),
 				ResourceRecords: []*route53.ResourceRecord{
-					{Value: sp(`"enr:-HW4QO1ml1DdXLeZLsUxewnthhUy8eROqkDyoMTyavfks9JlYQIlMFEUoM78PovJDPQrAkrb3LRJ-"`)},
-					{Value: sp(`"vtrymDguKCOIAWAgmlkgnY0iXNlY3AyNTZrMaEDffaGfJzgGhUif1JqFruZlYmA31HzathLSWxfbq_QoQ4"`)},
+					{Value: sp(`"enr:-HW4QO1ml1DdXLeZLsUxewnthhUy8eROqkDyoMTyavfks9JlYQIlMFEUoM78PovJDPQrAkrb3LRJ-""vtrymDguKCOIAWAgmlkgnY0iXNlY3AyNTZrMaEDffaGfJzgGhUif1JqFruZlYmA31HzathLSWxfbq_QoQ4"`)},
 				},
 				TTL:  ip(3333),
 				Type: sp("TXT"),