CLOUDFLAREAPI: Fix TXT quoting bug (#1543)

* Cleanup

* wip!

* Fix cloudflare quote bug

* wip!

* cleanup

* go generate

docs/_includes/matrix.html

@@ -74,8 +74,8 @@
 		<td class="danger">
 			<i class="fa fa-times text-danger" aria-hidden="true"></i>
 		</td>
-		<td class="danger">
-			<i class="fa fa-times text-danger" aria-hidden="true"></i>
+		<td class="success">
+			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
 		<td class="danger">
 			<i class="fa fa-times text-danger" aria-hidden="true"></i>
@@ -197,8 +197,8 @@
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
-		<td class="danger">
-			<i class="fa fa-times text-danger" aria-hidden="true"></i>
+		<td class="success">
+			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
@@ -603,7 +603,9 @@
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
-		<td><i class="fa fa-minus dim"></i></td>
+		<td class="success">
+			<i class="fa fa-check text-success" aria-hidden="true"></i>
+		</td>
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
@@ -931,7 +933,9 @@
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
-		<td><i class="fa fa-minus dim"></i></td>
+		<td class="success">
+			<i class="fa fa-check text-success" aria-hidden="true"></i>
+		</td>
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
@@ -1830,7 +1834,9 @@
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>
-		<td><i class="fa fa-minus dim"></i></td>
+		<td class="success">
+			<i class="fa fa-check text-success" aria-hidden="true"></i>
+		</td>
 		<td class="success">
 			<i class="fa fa-check text-success" aria-hidden="true"></i>
 		</td>

models/quotes.go

@@ -20,6 +20,7 @@ func IsQuoted(s string) bool {
 }
 
 // StripQuotes returns the string with the starting and ending quotes removed.
+// If it is not quoted, the original string is returned.
 func StripQuotes(s string) string {
 	if IsQuoted(s) {
 		return s[1 : len(s)-1]
@@ -36,12 +37,20 @@ func ParseQuotedTxt(s string) []string {
 	if !IsQuoted(s) {
 		return []string{s}
 	}
+
+	// TODO(tlim): Consider using r, err := ParseQuotedFields(s)
 	return strings.Split(StripQuotes(s), `" "`)
 }
 
 // ParseQuotedFields is like strings.Fields except individual fields
 // might be quoted using `"`.
 func ParseQuotedFields(s string) ([]string, error) {
+	// Parse according to RFC1035 zonefile specifications.
+	// "foo"  -> one string: `foo``
+	// "foo" "bar"  -> two strings: `foo` and `bar`
+	// Quotes are escaped with \"
+
+	// Implementation note:
 	// Fields are space-separated but a field might be quoted.  This is,
 	// essentially, a CSV where spaces are the field separator (not
 	// commas). Therefore, we use the CSV parser. See https://stackoverflow.com/a/47489846/71978

models/t_caa.go

@@ -43,5 +43,5 @@ func (rc *RecordConfig) SetTargetCAAString(s string) error {
 	if len(part) != 3 {
 		return fmt.Errorf("CAA value does not contain 3 fields: (%#v)", s)
 	}
-	return rc.SetTargetCAAStrings(part[0], part[1], StripQuotes(part[2]))
+	return rc.SetTargetCAAStrings(part[0], part[1], part[2])
 }

models/t_txt.go

@@ -1,5 +1,7 @@
 package models
 
+import "strings"
+
 /*
 Sadly many providers handle TXT records in strange and non-compliant ways.
 
@@ -71,11 +73,30 @@ func (rc *RecordConfig) SetTargetTXTs(s []string) error {
 	return nil
 }
 
+// GetTargetTXTJoined returns the TXT target as one string. If it was stored as multiple strings, concatenate them.
+func (rc *RecordConfig) GetTargetTXTJoined() string {
+	return strings.Join(rc.TxtStrings, "")
+}
+
 // SetTargetTXTString is like SetTargetTXT but accepts one big string,
 // which must be parsed into one or more strings based on how it is quoted.
 // Ex: foo             << 1 string
 //     foo bar         << 1 string
+//     "foo bar"       << 1 string
 //     "foo" "bar"     << 2 strings
+// FIXME(tlim): This function is badly named. It obscures the fact
+// that the string is parsed for quotes and should only be used for returns TXTMulti.
+// Deprecated: Use SetTargetTXTfromRFC1035Quoted instead.
 func (rc *RecordConfig) SetTargetTXTString(s string) error {
 	return rc.SetTargetTXTs(ParseQuotedTxt(s))
 }
+
+func (rc *RecordConfig) SetTargetTXTfromRFC1035Quoted(s string) error {
+	many, err := ParseQuotedFields(s)
+	if err != nil {
+		return err
+	}
+	return rc.SetTargetTXTs(many)
+}
+
+// There is no GetTargetTXTfromRFC1025Quoted(). Use GetTargetRFC1035Quoted()

providers/cloudflare/auditrecords.go

@@ -2,10 +2,24 @@ package cloudflare
 
 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.TxtNoMultipleStrings(records); err != nil {
+		return err
+	} // Still needed as of 2022-06-18
+
+	if err := recordaudit.TxtNoTrailingSpace(records); err != nil {
+		return err
+	} // Still needed as of 2022-06-18
+
+	if err := recordaudit.TxtNotEmpty(records); err != nil {
+		return err
+	} // Still needed as of 2022-06-18
+
 	return nil
 }

providers/cloudflare/cloudflareProvider.go

@@ -214,6 +214,14 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m
 
 	// Normalize
 	models.PostProcessRecords(records)
+	//txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records
+	// Don't split.
+	// Cloudflare's API only supports one TXT string of any non-zero length. No
+	// multiple strings (TXTMulti).
+	// When serving the DNS record, it splits strings >255 octets into
+	// individual segments of 255 each. However that is hidden from the API.
+	// Therefore, whether the string is 1 octet or thousands, just store it as
+	// one string in the first element of .TxtStrings.
 
 	differ := diff.New(dc, getProxyMetadata)
 	_, create, del, mod, err := differ.IncrementalDiff(records)
@@ -636,6 +644,7 @@ func stringDefault(value interface{}, def string) string {
 }
 
 func (c *cloudflareProvider) nativeToRecord(domain string, cr cloudflare.DNSRecord) (*models.RecordConfig, error) {
+
 	// normalize cname,mx,ns records with dots to be consistent with our config format.
 	if cr.Type == "CNAME" || cr.Type == "MX" || cr.Type == "NS" || cr.Type == "PTR" {
 		if cr.Content != "." {
@@ -670,7 +679,10 @@ func (c *cloudflareProvider) nativeToRecord(domain string, cr cloudflare.DNSReco
 			target); err != nil {
 			return nil, fmt.Errorf("unparsable SRV record received from cloudflare: %w", err)
 		}
-	default: // "A", "AAAA", "ANAME", "CAA", "CNAME", "NS", "PTR", "TXT"
+	case "TXT":
+		err := rc.SetTargetTXT(cr.Content)
+		return rc, err
+	default:
 		if err := rc.PopulateFromString(rType, cr.Content, domain); err != nil {
 			return nil, fmt.Errorf("unparsable record received from cloudflare: %w", err)
 		}

providers/cloudflare/rest.go

@@ -47,7 +47,7 @@ func (c *cloudflareProvider) getRecordsForDomain(id string, domain string) ([]*m
 // create a correction to delete a record
 func (c *cloudflareProvider) deleteRec(rec cloudflare.DNSRecord, domainID string) *models.Correction {
 	return &models.Correction{
-		Msg: fmt.Sprintf("DELETE record: %s %s %d %s (id=%s)", rec.Name, rec.Type, rec.TTL, rec.Content, rec.ID),
+		Msg: fmt.Sprintf("DELETE record: %s %s %d %q (id=%s)", rec.Name, rec.Type, rec.TTL, rec.Content, rec.ID),
 		F: func() error {
 			err := c.cfClient.DeleteDNSRecord(context.Background(), domainID, rec.ID)
 			return err
@@ -119,7 +119,7 @@ func (c *cloudflareProvider) createRec(rec *models.RecordConfig, domainID string
 		prio = fmt.Sprintf(" %d ", rec.MxPreference)
 	}
 	if rec.Type == "TXT" {
-		content = rec.GetTargetRFC1035Quoted()
+		content = rec.GetTargetTXTJoined()
 	}
 	if rec.Type == "DS" {
 		content = fmt.Sprintf("%d %d %d %s", rec.DsKeyTag, rec.DsAlgorithm, rec.DsDigestType, rec.DsDigest)
@@ -183,7 +183,7 @@ func (c *cloudflareProvider) modifyRecord(domainID, recID string, proxied bool,
 		TTL:      int(rec.TTL),
 	}
 	if rec.Type == "TXT" {
-		r.Content = rec.GetTargetRFC1035Quoted()
+		r.Content = rec.GetTargetTXTJoined()
 	}
 	if rec.Type == "SRV" {
 		r.Data = cfSrvData(rec)