Repository: StackExchange/dnscontrol
Pull request #2015 (2023-01-29 23:38:20)

NS1: Improve NS1_URLFWD handling
This changeset improves handling for NS1_URLFWD inside dnscontrol's code:
* Adds integration tests for the record, confined to NS1.
* Replaces `strings.Split()` with `strings.Fields()` in URLFWD's case. While there, other occurrences of it in MX and SRV code are replaced for the same reasons.
* Adds code under `pkg/normalize/validate.go` and relevant tests to ensure record's format is always validated on parsing time.


Patch:
diff --git a/pkg/normalize/validate.go b/pkg/normalize/validate.go
index 940560ee..66491105 100644
--- a/pkg/normalize/validate.go
+++ b/pkg/normalize/validate.go
@@ -196,6 +196,10 @@ func checkTargets(rec *models.RecordConfig, domain string) (errs []error) {
 		if label == "@" {
 			check(fmt.Errorf("cannot create NS record for bare domain. Use NAMESERVER instead"))
 		}
+	case "URLFWD":
+		if len(strings.Fields(target)) != 5 {
+			check(fmt.Errorf("record should follow format: \"from to redirectType pathForwardingMode queryForwarding\""))
+		}
 	case "PTR":
 		check(checkTarget(target))
 	case "SOA":
@@ -331,6 +335,7 @@ func ValidateAndNormalizeConfig(config *models.DNSConfig) (errs []error) {
 		// Normalize Records.
 		models.PostProcessRecords(domain.Records)
 		for _, rec := range domain.Records {
+
 			if rec.TTL == 0 {
 				rec.TTL = models.DefaultTTL
 			}
@@ -364,6 +369,7 @@ func ValidateAndNormalizeConfig(config *models.DNSConfig) (errs []error) {
 			if err := checkLabel(rec.GetLabel(), rec.Type, rec.GetTargetField(), domain.Name, rec.Metadata); err != nil {
 				errs = append(errs, err)
 			}
+
 			if errs2 := checkTargets(rec, domain.Name); errs2 != nil {
 				errs = append(errs, errs2...)
 			}
diff --git a/providers/ns1/ns1Provider.go b/providers/ns1/ns1Provider.go
index f37f0cd8..85e97d37 100644
--- a/providers/ns1/ns1Provider.go
+++ b/providers/ns1/ns1Provider.go
@@ -280,7 +280,7 @@ func buildRecord(recs models.Records, domain string, id string) *dns.Record {
 	}
 	for _, r := range recs {
 		if r.Type == "MX" {
-			rec.AddAnswer(&dns.Answer{Rdata: strings.Split(fmt.Sprintf("%d %v", r.MxPreference, r.GetTargetField()), " ")})
+			rec.AddAnswer(&dns.Answer{Rdata: strings.Fields(fmt.Sprintf("%d %v", r.MxPreference, r.GetTargetField()))})
 		} else if r.Type == "TXT" {
 			rec.AddAnswer(&dns.Answer{Rdata: r.TxtStrings})
 		} else if r.Type == "CAA" {
@@ -291,7 +291,7 @@ func buildRecord(recs models.Records, domain string, id string) *dns.Record {
 					r.GetTargetField(),
 				}})
 		} else if r.Type == "SRV" {
-			rec.AddAnswer(&dns.Answer{Rdata: strings.Split(fmt.Sprintf("%d %d %d %v", r.SrvPriority, r.SrvWeight, r.SrvPort, r.GetTargetField()), " ")})
+			rec.AddAnswer(&dns.Answer{Rdata: strings.Fields(fmt.Sprintf("%d %d %d %v", r.SrvPriority, r.SrvWeight, r.SrvPort, r.GetTargetField()))})
 		} else if r.Type == "NAPTR" {
 			rec.AddAnswer(&dns.Answer{Rdata: []string{
 				strconv.Itoa(int(r.NaptrOrder)),
@@ -307,7 +307,7 @@ func buildRecord(recs models.Records, domain string, id string) *dns.Record {
 				strconv.Itoa(int(r.DsDigestType)),
 				r.DsDigest}})
 		} else {
-			rec.AddAnswer(&dns.Answer{Rdata: strings.Split(r.GetTargetField(), " ")})
+			rec.AddAnswer(&dns.Answer{Rdata: strings.Fields(r.GetTargetField())})
 		}
 	}
 	return rec


Test patch:
diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go
index 6b6bdd82..cc7523f2 100644
--- a/integrationTest/integration_test.go
+++ b/integrationTest/integration_test.go
@@ -496,6 +496,10 @@ func tlsa(name string, usage, selector, matchingtype uint8, target string) *mode
 	return r
 }
 
+func urlfwd(name, target string) *models.RecordConfig {
+	return makeRec(name, target, "URLFWD")
+}
+
 func ignoreName(name string) *models.RecordConfig {
 	r := &models.RecordConfig{
 		Type: "IGNORE_NAME",
@@ -917,6 +921,12 @@ func makeTests(t *testing.T) []*TestGroup {
 			tc("Update 1200 records", manyA("rec%04d", "1.2.3.5", 1200)...),
 		),
 
+		testgroup("NS1_URLFWD tests",
+			only("NS1"),
+			tc("Add a urlfwd", urlfwd("urlfwd1", "/ http://example.com 302 2 0")),
+			tc("Update a urlfwd", urlfwd("urlfwd1", "/ http://example.org 301 2 0")),
+		),
+
 		//
 		// CanUse* types:
 		//
diff --git a/pkg/normalize/validate_test.go b/pkg/normalize/validate_test.go
index ea1282a4..839b303c 100644
--- a/pkg/normalize/validate_test.go
+++ b/pkg/normalize/validate_test.go
@@ -199,6 +199,28 @@ func TestNSAtRoot(t *testing.T) {
 	}
 }
 
+func TestURLFWDValid(t *testing.T) {
+	rec := &models.RecordConfig{Type: "URLFWD"}
+	rec.SetLabel("test1", "foo.com")
+	rec.SetTarget("/ http://example.com 302 2 0")
+
+	errs := checkTargets(rec, "foo.com")
+	if len(errs) > 0 {
+		t.Error("Expect no error with valid URLFWD target")
+	}
+}
+
+func TestURLFWDInvalid(t *testing.T) {
+	rec := &models.RecordConfig{Type: "URLFWD"}
+	rec.SetLabel("test2", "foo.com")
+	rec.SetTarget("/ http://example.com 302 2")
+
+	errs := checkTargets(rec, "foo.com")
+	if len(errs) == 0 {
+		t.Error("Expect error with invalid URLFWD target")
+	}
+}
+
 func TestTransforms(t *testing.T) {
 	var tests = []struct {
 		givenIP         string