From b0d80ae4982534127589e8b37462569012223fa1 Mon Sep 17 00:00:00 2001
From: Amelia Aronsohn <squirrel@wearing.black>
Date: Mon, 11 Jul 2022 11:52:18 -0700
Subject: [PATCH] DNSIMPLE: do not support unpaired double quotes in TXT
 (#1610)

* DNSIMPLE: do not support unpaired double quotes in TXT

DNSimple supports multiple double-quotes strings in a TXT record, but does not correctly support unpaired or escaped double-quotes currently.

IE the following are valid
```
asdf
"asdf"
"asdf" "asdf"
!@#$ %^&*()([][{}{<></'`:;-_=+\
```

however `as\"df` and `as"df` are not

This removes the extra string processing in getTargetRecordPriority as all tests pass without it and currently only double-quotes cause problems in our TXT validations.

I added another test to prove additional quoting is not needed. We can remove it if undesired.

Also applied small lint changes to make my editor happy.

* Use backticks to prevent escaping

* Set TXT target record content to GetTargetRFC1035Quoted()

Co-authored-by: Tom Limoncelli <tlimoncelli@stackoverflow.com>
---
 integrationTest/integration_test.go    |  2 ++
 providers/dnsimple/auditrecords.go     |  4 +++
 providers/dnsimple/dnsimpleProvider.go | 42 ++++++--------------------
 3 files changed, 15 insertions(+), 33 deletions(-)

diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go
index de65599a4..cf5b90247 100644
--- a/integrationTest/integration_test.go
+++ b/integrationTest/integration_test.go
@@ -905,6 +905,8 @@ func makeTests(t *testing.T) []*TestGroup {
 			tc("Create TXT with double-quote", txt("foodq", `quo"te`)),
 			clear(),
 			tc("Create TXT with ws at end", txt("foows1", "with space at end ")),
+			clear(),
+			tc("Create TXT with frequently escaped characters", txt("fooex", `!^.*$@#%^&()([][{}{<></:;-_=+\`)),
 		),
 
 		//
diff --git a/providers/dnsimple/auditrecords.go b/providers/dnsimple/auditrecords.go
index fba9a98b4..411b67ddd 100644
--- a/providers/dnsimple/auditrecords.go
+++ b/providers/dnsimple/auditrecords.go
@@ -2,10 +2,14 @@ package dnsimple
 
 import (
 	"github.com/StackExchange/dnscontrol/v3/models"
+	"github.com/StackExchange/dnscontrol/v3/pkg/recordaudit"
 )
 
 // AuditRecords returns an error if any records are not
 // supportable by this provider.
 func AuditRecords(records []*models.RecordConfig) error {
+	if err := recordaudit.TxtNoDoubleQuotes(records); err != nil {
+		return err
+	}
 	return nil
 }
diff --git a/providers/dnsimple/dnsimpleProvider.go b/providers/dnsimple/dnsimpleProvider.go
index 020da74dd..2e5e25f03 100644
--- a/providers/dnsimple/dnsimpleProvider.go
+++ b/providers/dnsimple/dnsimpleProvider.go
@@ -61,7 +61,7 @@ type dnsimpleProvider struct {
 }
 
 // GetNameservers returns the name servers for a domain.
-func (c *dnsimpleProvider) GetNameservers(domainName string) ([]*models.Nameserver, error) {
+func (c *dnsimpleProvider) GetNameservers(_ string) ([]*models.Nameserver, error) {
 	return models.ToNameservers(defaultNameServerNames)
 }
 
@@ -130,7 +130,7 @@ func (c *dnsimpleProvider) GetZoneRecords(domain string) (models.Records, error)
 
 // GetDomainCorrections returns corrections that update a domain.
 func (c *dnsimpleProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
-	corrections := []*models.Correction{}
+	var corrections []*models.Correction
 	err := dc.Punycode()
 	if err != nil {
 		return nil, err
@@ -201,7 +201,7 @@ func removeApexNS(records models.Records) models.Records {
 
 // GetRegistrarCorrections returns corrections that update a domain's registrar.
 func (c *dnsimpleProvider) GetRegistrarCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
-	corrections := []*models.Correction{}
+	var corrections []*models.Correction
 
 	nameServers, err := c.getNameservers(dc.Name)
 	if err != nil {
@@ -211,7 +211,7 @@ func (c *dnsimpleProvider) GetRegistrarCorrections(dc *models.DomainConfig) ([]*
 
 	actual := strings.Join(nameServers, ",")
 
-	expectedSet := []string{}
+	var expectedSet []string
 	for _, ns := range dc.Nameservers {
 		expectedSet = append(expectedSet, ns.Name)
 	}
@@ -298,7 +298,7 @@ func (c *dnsimpleProvider) getRecords(domainName string) ([]dnsimpleapi.ZoneReco
 	}
 
 	opts := &dnsimpleapi.ZoneRecordListOptions{}
-	recs := []dnsimpleapi.ZoneRecord{}
+	var recs []dnsimpleapi.ZoneRecord
 	page := 1
 	for {
 		opts.Page = &page
@@ -540,7 +540,7 @@ func newDsp(conf map[string]string, metadata json.RawMessage) (providers.DNSServ
 	return newProvider(conf, metadata)
 }
 
-func newProvider(m map[string]string, metadata json.RawMessage) (*dnsimpleProvider, error) {
+func newProvider(m map[string]string, _ json.RawMessage) (*dnsimpleProvider, error) {
 	api := &dnsimpleProvider{}
 	api.AccountToken = m["token"]
 	if api.AccountToken == "" {
@@ -584,21 +584,15 @@ func getTargetRecordContent(rc *models.RecordConfig) string {
 	case "DS":
 		return fmt.Sprintf("%d %d %d %s", rc.DsKeyTag, rc.DsAlgorithm, rc.DsDigestType, rc.DsDigest)
 	case "NAPTR":
-		return fmt.Sprintf("%d %d %s %s %s %s",
-			rc.NaptrOrder, rc.NaptrPreference,
-			quoteDNSString(rc.NaptrFlags), quoteDNSString(rc.NaptrService),
-			quoteDNSString(rc.NaptrRegexp),
+		return fmt.Sprintf(`%d %d "%s" "%s" "%s" %s`,
+			rc.NaptrOrder, rc.NaptrPreference, rc.NaptrFlags, rc.NaptrService, rc.NaptrRegexp,
 			rc.GetTargetField())
 	case "SSHFP":
 		return fmt.Sprintf("%d %d %s", rc.SshfpAlgorithm, rc.SshfpFingerprint, rc.GetTargetField())
 	case "SRV":
 		return fmt.Sprintf("%d %d %s", rc.SrvWeight, rc.SrvPort, rc.GetTargetField())
 	case "TXT":
-		quoted := make([]string, len(rc.TxtStrings))
-		for i := range rc.TxtStrings {
-			quoted[i] = quoteDNSString(rc.TxtStrings[i])
-		}
-		return strings.Join(quoted, " ")
+		return rc.GetTargetRFC1035Quoted()
 	default:
 		return rc.GetTargetField()
 	}
@@ -618,21 +612,3 @@ func getTargetRecordPriority(rc *models.RecordConfig) int {
 		return 0
 	}
 }
-
-// Return a DNS string appropriately escaped for DNSimple.
-// Should include the surrounding quotes.
-//
-// Warning: the DNSimple API is severely underdocumented in this area.
-// I know that it takes multiple quoted strings just fine, and constructs the
-// DNS multiple quoted items.
-// I'm not 100% on the escaping, but since it's a JSON API, JSON escaping seems
-// reasonable.
-// I do know that DNSimple have their own checks, so anything too crazy will
-// get a "400 Validation failed" HTTP response.
-func quoteDNSString(unquoted string) string {
-	b, err := json.Marshal(unquoted)
-	if err != nil {
-		panic(fmt.Errorf("unable to marshal to JSON: %q", unquoted))
-	}
-	return string(b)
-}