Fix TXT round-trip for long and non-ASCII records

.github/workflows/go.yml

@@ -17,11 +17,13 @@ jobs:
       with:
         go-version: "1.23"
 
-    - name: Test
+    - name: Unit tests (root module)
+      run: go test -v ./...
+
+    - name: Integration tests (libdnstest)
       env:
         CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
         CLOUDFLARE_TEST_ZONE: ${{ secrets.CLOUDFLARE_TEST_ZONE }}
-
       run: |
         cd libdnstest/
         go test -v ./...

client.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"strings"
 
 	"github.com/libdns/libdns"
 )
@@ -69,15 +68,12 @@ func (p *Provider) getDNSRecords(ctx context.Context, zoneInfo cfZone, rec libdn
 	qs.Set("type", rr.Type)
 	qs.Set("name", libdns.AbsoluteName(rr.Name, zoneInfo.Name))
 
-	var unwrappedContent string
 	if matchContent {
-		if rr.Type == "TXT" {
-			unwrappedContent = unwrapContent(rr.Content)
-			// Use the contains (wildcard) search with unquoted content to return both quoted and unquoted content
-			qs.Set("content.contains", unwrappedContent)
-		} else if rr.Type != "SRV" && rr.Type != "HTTPS" && rr.Type != "SVCB" {
-			// SRV, HTTPS, SVCB records don't support content.exact filtering in Cloudflare API
-			// They will be matched by type and name only
+		// TXT is matched client-side by decoded value (below): Cloudflare
+		// re-chunks and re-escapes stored TXT presentation, so neither
+		// content.exact nor content.contains reliably matches our encoding.
+		// SRV, HTTPS, SVCB don't support content.exact filtering at all.
+		if rr.Type != "TXT" && rr.Type != "SRV" && rr.Type != "HTTPS" && rr.Type != "SVCB" {
 			qs.Set("content.exact", rr.Content)
 		}
 	}
@@ -89,28 +85,34 @@ func (p *Provider) getDNSRecords(ctx context.Context, zoneInfo cfZone, rec libdn
 	}
 
 	var results []cfDNSRecord
-	_, err = p.doAPIRequest(req, &results)
+	if _, err = p.doAPIRequest(req, &results); err != nil {
+		return nil, err
+	}
 
-	// Since the TXT search used contains (wildcard), check for exact matches
+	// Match TXT by decoded value, since Cloudflare's stored presentation need
+	// not equal what we sent.
 	if matchContent && rr.Type == "TXT" {
-		for i := 0; i < len(results); i++ {
-			// Prefer exact quoted content
-			if results[i].Content == rr.Content {
-				return []cfDNSRecord{results[i]}, nil
+		want := rec.RR().Data
+		var matched []cfDNSRecord
+		for _, cand := range results {
+			// Per the libdns RecordDeleter contract, an empty value matches any
+			// value for the given name+type.
+			if want == "" {
+				matched = append(matched, cand)
+				continue
 			}
-		}
-
-		for i := 0; i < len(results); i++ {
-			// Using exact unquoted content is acceptable
-			if results[i].Content == unwrappedContent {
-				return []cfDNSRecord{results[i]}, nil
+			got, decErr := decodeTXT(cand.Content)
+			if decErr != nil {
+				return nil, fmt.Errorf("decoding TXT content %q: %v", cand.Content, decErr)
+			}
+			if got == want {
+				matched = append(matched, cand)
 			}
 		}
-
-		return []cfDNSRecord{}, nil
+		return matched, nil
 	}
 
-	return results, err
+	return results, nil
 }
 
 func (p *Provider) getZoneInfo(ctx context.Context, zoneName string) (cfZone, error) {
@@ -201,17 +203,3 @@ func (p *Provider) doAPIRequest(req *http.Request, result any) (cfResponse, erro
 }
 
 const baseURL = "https://api.cloudflare.com/client/v4"
-
-func unwrapContent(content string) string {
-	if strings.HasPrefix(content, `"`) && strings.HasSuffix(content, `"`) {
-		content = strings.TrimPrefix(strings.TrimSuffix(content, `"`), `"`)
-	}
-	return content
-}
-
-func wrapContent(content string) string {
-	if !strings.HasPrefix(content, `"`) && !strings.HasSuffix(content, `"`) {
-		content = fmt.Sprintf("%q", content)
-	}
-	return content
-}

client_test.go

@@ -0,0 +1,99 @@
+package cloudflare
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/libdns/libdns"
+)
+
+// fakeClient returns a fixed response body for every request, so getDNSRecords'
+// TXT matching can be exercised deterministically without network or creds.
+type fakeClient struct{ body string }
+
+func (f fakeClient) Do(req *http.Request) (*http.Response, error) {
+	return &http.Response{
+		StatusCode: 200,
+		Body:       io.NopCloser(bytes.NewBufferString(f.body)),
+		Header:     make(http.Header),
+	}, nil
+}
+
+func TestGetDNSRecordsTXTMatching(t *testing.T) {
+	// Two TXT records at the same name with distinct decoded values.
+	const body = `{"success":true,"result":[` +
+		`{"id":"a","type":"TXT","name":"foo.example.com","content":"\"hello\""},` +
+		`{"id":"b","type":"TXT","name":"foo.example.com","content":"\"world\""}` +
+		`]}`
+	p := &Provider{APIToken: "test", HTTPClient: fakeClient{body: body}}
+	zone := cfZone{ID: "zone123", Name: "example.com"}
+	ctx := context.Background()
+
+	cases := []struct {
+		name    string
+		text    string
+		wantIDs []string
+	}{
+		{"specific-match", "hello", []string{"a"}},
+		{"other-specific-match", "world", []string{"b"}},
+		{"empty-matches-all", "", []string{"a", "b"}}, // libdns empty-value contract
+		{"no-match", "nope", nil},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			got, err := p.getDNSRecords(ctx, zone, libdns.TXT{Name: "foo", Text: c.text}, true)
+			if err != nil {
+				t.Fatalf("getDNSRecords: %v", err)
+			}
+			var ids []string
+			for _, r := range got {
+				ids = append(ids, r.ID)
+			}
+			if !equalStrings(ids, c.wantIDs) {
+				t.Fatalf("text=%q: want IDs %v, got %v", c.text, c.wantIDs, ids)
+			}
+		})
+	}
+}
+
+func TestGetDNSRecordsTXTMatchingMalformed(t *testing.T) {
+	// A candidate whose stored content cannot be decoded (here, an unterminated
+	// quoted string) must surface an error rather than be silently skipped.
+	const body = `{"success":true,"result":[` +
+		`{"id":"x","type":"TXT","name":"foo.example.com","content":"\"abc"}` +
+		`]}`
+	p := &Provider{APIToken: "test", HTTPClient: fakeClient{body: body}}
+	zone := cfZone{ID: "zone123", Name: "example.com"}
+	ctx := context.Background()
+
+	// Non-empty value forces a decode, which must fail loudly.
+	if _, err := p.getDNSRecords(ctx, zone, libdns.TXT{Name: "foo", Text: "abc"}, true); err == nil {
+		t.Fatal("expected error for malformed TXT content, got nil")
+	}
+
+	// Empty value ("match any") never decodes, so it still matches the
+	// undecodable record without error.
+	got, err := p.getDNSRecords(ctx, zone, libdns.TXT{Name: "foo"}, true)
+	if err != nil {
+		t.Fatalf("empty-value match should not decode: %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("empty-value match: want 1, got %d", len(got))
+	}
+}
+
+func equalStrings(a, b []string) bool {
+	// FIXME: replace with slices.Equal in go >= 1.21.
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}