From f7c145a54c3e32bbe9f560e7b56b646f81e5c902 Mon Sep 17 00:00:00 2001 From: Tom Limoncelli <6293917+tlimoncelli@users.noreply.github.com> Date: Thu, 18 Dec 2025 07:00:38 -0500 Subject: [PATCH] BUGFIX: Multiple bugs in RP (RecordConfig v2) (#3931) # Issue Fixes https://github.com/StackExchange/dnscontrol/issues/3918 New "modern" types like RP had multiple bugs: * When RP() has an error (for example, wrong # of arguments) no error was printed. * DefaultTTL() was ignored. * FQDNs listed in RP() not properly checked to verify the are part of the D()/D_EXTEND() domain. * REFACTOR: Implement "double domain" checking and the skip_fqdn_check override (instead of in validate.go). * REFACTOR: Always list "names" as Raw, then ASCII, then Unicode. * REFACTOR: Generate domain DisplayName once, use many places # Resolution Fixed and test-cases added to prevent regressions. --- integrationTest/helpers_integration_test.go | 8 +- models/domain.go | 9 +- models/record.go | 11 +- models/record_test.go | 2 +- pkg/domaintags/domaintags.go | 22 +- pkg/domaintags/idn.go | 10 +- pkg/js/helpers.js | 1 + pkg/js/js.go | 4 +- pkg/js/js_test.go | 3 +- pkg/js/parse_tests/059-rawttls.js | 25 +++ pkg/js/parse_tests/059-rawttls.json | 180 ++++++++++++++++ .../parse_tests/059-rawttls/example.com.zone | 9 + pkg/printer/printer.go | 6 +- pkg/rtype/rp.go | 7 +- pkg/rtypecontrol/import.go | 98 ++++----- pkg/rtypecontrol/import_test.go | 59 ++++++ pkg/rtypecontrol/setrecordnames.go | 193 +++++++++++++++++ pkg/rtypecontrol/setrecordnames_test.go | 200 ++++++++++++++++-- pkg/rtypecontrol/stringify.go | 19 ++ providers/cloudflare/rest.go | 3 +- 20 files changed, 761 insertions(+), 108 deletions(-) create mode 100644 pkg/js/parse_tests/059-rawttls.js create mode 100644 pkg/js/parse_tests/059-rawttls.json create mode 100644 pkg/js/parse_tests/059-rawttls/example.com.zone create mode 100644 pkg/rtypecontrol/import_test.go create mode 100644 pkg/rtypecontrol/setrecordnames.go create mode 100644 pkg/rtypecontrol/stringify.go diff --git a/integrationTest/helpers_integration_test.go b/integrationTest/helpers_integration_test.go index 39b455abf..dc958d014 100644 --- a/integrationTest/helpers_integration_test.go +++ b/integrationTest/helpers_integration_test.go @@ -343,7 +343,7 @@ func cfSingleRedirectEnabled() bool { } func cfSingleRedirect(name string, code any, when, then string) *models.RecordConfig { - rec, err := rtypecontrol.NewRecordConfigFromRaw("CLOUDFLAREAPI_SINGLE_REDIRECT", 1, []any{name, code, when, then}, globalDCN) + rec, err := rtypecontrol.NewRecordConfigFromRaw("CLOUDFLAREAPI_SINGLE_REDIRECT", 1, []any{name, code, when, then}, globalDCN, "") panicOnErr(err) return rec } @@ -355,13 +355,13 @@ func cfWorkerRoute(pattern, target string) *models.RecordConfig { } func cfRedir(pattern, target string) *models.RecordConfig { - rec, err := rtypecontrol.NewRecordConfigFromRaw("CF_REDIRECT", 1, []any{pattern, target}, globalDCN) + rec, err := rtypecontrol.NewRecordConfigFromRaw("CF_REDIRECT", 1, []any{pattern, target}, globalDCN, "") panicOnErr(err) return rec } func cfRedirTemp(pattern, target string) *models.RecordConfig { - rec, err := rtypecontrol.NewRecordConfigFromRaw("CF_TEMP_REDIRECT", 1, []any{pattern, target}, globalDCN) + rec, err := rtypecontrol.NewRecordConfigFromRaw("CF_TEMP_REDIRECT", 1, []any{pattern, target}, globalDCN, "") panicOnErr(err) return rec } @@ -487,7 +487,7 @@ func r53alias(name, aliasType, target, evalTargetHealth string) *models.RecordCo } func rp(name string, m, t string) *models.RecordConfig { - rec, err := rtypecontrol.NewRecordConfigFromRaw("RP", 300, []any{name, m, t}, globalDCN) + rec, err := rtypecontrol.NewRecordConfigFromRaw("RP", 300, []any{name, m, t}, globalDCN, "") panicOnErr(err) return rec } diff --git a/models/domain.go b/models/domain.go index b79c88d37..a7b0d8538 100644 --- a/models/domain.go +++ b/models/domain.go @@ -25,12 +25,13 @@ const ( // DomainConfig describes a DNS domain (technically a DNS zone). type DomainConfig struct { - Name string `json:"name"` // NO trailing "." Converted to IDN (punycode) early in the pipeline. NameRaw string `json:"-"` // name as entered by user in dnsconfig.js + Name string `json:"name"` // NO trailing "." Converted to IDN (punycode) early in the pipeline. NameUnicode string `json:"-"` // name in Unicode format - Tag string `json:"tag,omitempty"` // Split horizon tag. - UniqueName string `json:"uniquename"` // .Name + "!" + .Tag (no !tag added if tag is "") + Tag string `json:"tag,omitempty"` // Split horizon tag. + UniqueName string `json:"uniquename"` // .Name + "!" + .Tag (no !tag added if tag is "") + DisplayName string `json:"-"` // For TUI display: "canonical!tag" or "canonical!tag (unicode)" RegistrarName string `json:"registrar"` DNSProviderNames map[string]int `json:"dnsProviders"` @@ -80,7 +81,7 @@ func (dc *DomainConfig) PostProcess() { // Turn the user-supplied name into the fixed forms. ff := domaintags.MakeDomainNameVarieties(dc.Name) - dc.Tag, dc.NameRaw, dc.Name, dc.NameUnicode, dc.UniqueName = ff.Tag, ff.NameRaw, ff.NameASCII, ff.NameUnicode, ff.UniqueName + dc.Tag, dc.NameRaw, dc.Name, dc.NameUnicode, dc.UniqueName, dc.DisplayName = ff.Tag, ff.NameRaw, ff.NameASCII, ff.NameUnicode, ff.UniqueName, ff.DisplayName // Store the FixForms is Metadata so we don't have to change the signature of every function that might need them. // This is a bit ugly but avoids a huge refactor. Please avoid using these to make the future refactor easier. diff --git a/models/record.go b/models/record.go index a907e971b..cc518c833 100644 --- a/models/record.go +++ b/models/record.go @@ -22,15 +22,15 @@ type RecordConfig struct { // TTL is the DNS record's TTL in seconds. 0 means provider default. TTL uint32 `json:"ttl,omitempty"` - // Name is the shortname i.e. the FQDN without the parent directory's suffix. + // Name is the shortname i.e. the FQDN without the parent domains's suffix. // It should never be "". Record at the apex (naked domain) are represented by "@". + NameRaw string `json:"name_raw,omitempty"` // .Name as the user entered it in dnsconfig.js Name string `json:"name"` // The short name, PunyCode. See above. - NameRaw string `json:"name_raw,omitempty"` // .Name as the user entered it in dnsconfig.js (downcased). NameUnicode string `json:"name_unicode,omitempty"` // .Name as Unicode (downcased, then convertedot Unicode). // This is the FQDN version of .Name. It should never have a trailing ".". - NameFQDN string `json:"-"` // Must end with ".$origin". NameFQDNRaw string `json:"-"` // .NameFQDN as the user entered it in dnsconfig.js (downcased). + NameFQDN string `json:"-"` // Must end with ".$origin". NameFQDNUnicode string `json:"-"` // .NameFQDN as Unicode (downcased, then convertedot Unicode). // F is the binary representation of the record's data usually a dns.XYZ struct. @@ -247,8 +247,9 @@ func FixPosition(str string) string { } str = strings.TrimSpace(str) str = strings.ReplaceAll(str, "\n", " ") - str = strings.TrimPrefix(str, "at :") - return fmt.Sprintf("[line:%s]", str) + str = strings.ReplaceAll(str, "", "line") + str = strings.TrimPrefix(str, "at ") + return fmt.Sprintf("[%s]", str) } // Copy returns a deep copy of a RecordConfig. diff --git a/models/record_test.go b/models/record_test.go index 841f18fa3..1c27f07ee 100644 --- a/models/record_test.go +++ b/models/record_test.go @@ -248,7 +248,7 @@ func TestFixPosition(t *testing.T) { { name: "random string", pos: "alsdjfsljd", - want: "[line:alsdjfsljd]", + want: "[alsdjfsljd]", }, } for _, tt := range tests { diff --git a/pkg/domaintags/domaintags.go b/pkg/domaintags/domaintags.go index 2b22b534e..7c5365c40 100644 --- a/pkg/domaintags/domaintags.go +++ b/pkg/domaintags/domaintags.go @@ -12,9 +12,11 @@ type DomainNameVarieties struct { NameASCII string // "punycode.com" (converted to punycode and downcase) NameUnicode string // "unicode.com" (converted to unicode, ASCII portions downcased) UniqueName string // "punycode.com!tag" (canonical unique name with tag) + DisplayName string // "canonical" or "canonical (unicode.com)" if unicode Tag string // The tag portion of `example.com!tag` HasBang bool // Was there a "!" in the input when creating this struct? + } // MakeDomainNameVarieties turns the user-supplied name into the varioius forms. @@ -23,6 +25,7 @@ type DomainNameVarieties struct { // * .NameASCII: punycode version, downcased // * .NameUnicode: unicode version of the name, downcased. // * .UniqueName: "example.com!tag" unique across the entire config. +// * .NameDisplay: "punycode.com!tag" or "punycode.com!tag (unicode.com)" if unicode. func MakeDomainNameVarieties(n string) *DomainNameVarieties { var err error var tag, nameRaw, nameASCII, nameUnicode, uniqueName string @@ -73,12 +76,27 @@ func MakeDomainNameVarieties(n string) *DomainNameVarieties { uniqueName = nameASCII } + // Display this as "example.com" or "punycode.com (unicode.com)" + display := Display(uniqueName, nameASCII, nameUnicode) + return &DomainNameVarieties{ - Tag: tag, NameRaw: nameRaw, NameASCII: nameASCII, NameUnicode: nameUnicode, UniqueName: uniqueName, - HasBang: hasBang, + DisplayName: display, + + Tag: tag, + HasBang: hasBang, } } + +// Display constructs the string suitable for displaying to the user +// "example.com" or "punycode.com (unicode.com)" +// If we add a user-configurable display format, it will be implemented here. +func Display(canonical, nameASCII, nameUnicode string) string { + if nameUnicode != nameASCII { + return canonical + " (" + nameUnicode + ")" + } + return canonical +} diff --git a/pkg/domaintags/idn.go b/pkg/domaintags/idn.go index e6a599e88..da79003d4 100644 --- a/pkg/domaintags/idn.go +++ b/pkg/domaintags/idn.go @@ -1,15 +1,21 @@ package domaintags -import "golang.org/x/net/idna" +import ( + "strings" + + "golang.org/x/net/idna" +) // EfficientToASCII converts a domain name to its ASCII representation using // IDNA, on error returns the original name, and avoids allocating new memory -// when possible. +// when possible. The final string is lowercased. func EfficientToASCII(name string) string { nameIDN, err := idna.ToASCII(name) if err != nil { return name // Fallback to raw name on error. } + nameIDN = strings.ToLower(nameIDN) + // Avoid pointless duplication. if nameIDN == name { return name diff --git a/pkg/js/helpers.js b/pkg/js/helpers.js index a18e01714..aa20d8285 100644 --- a/pkg/js/helpers.js +++ b/pkg/js/helpers.js @@ -2456,6 +2456,7 @@ function rawrecordBuilder(type) { var record = { type: type, filepos: position, + ttl: d.defaultTTL, }; // Process the args: Functions are executed, objects are assumed to diff --git a/pkg/js/js.go b/pkg/js/js.go index 948416968..8a00db140 100644 --- a/pkg/js/js.go +++ b/pkg/js/js.go @@ -126,7 +126,9 @@ func ExecuteJavascriptString(script []byte, devMode bool, variables map[string]s return nil, err } - rtypecontrol.ImportRawRecords(conf.Domains) + if err := rtypecontrol.ImportRawRecords(conf.Domains); err != nil { + return nil, err + } return conf, nil } diff --git a/pkg/js/js_test.go b/pkg/js/js_test.go index 870269bd9..b3ab7ded6 100644 --- a/pkg/js/js_test.go +++ b/pkg/js/js_test.go @@ -15,6 +15,7 @@ import ( "github.com/StackExchange/dnscontrol/v4/pkg/prettyzone" "github.com/StackExchange/dnscontrol/v4/pkg/providers" _ "github.com/StackExchange/dnscontrol/v4/pkg/providers/_all" + _ "github.com/StackExchange/dnscontrol/v4/pkg/rtype" testifyrequire "github.com/stretchr/testify/require" ) @@ -117,7 +118,7 @@ func TestParsedFiles(t *testing.T) { } else { zoneFile = filepath.Join(testDir, testName, dc.Name+".zone") } - // fmt.Printf("DEBUG: zonefile = %q\n", zoneFile) + //fmt.Printf("DEBUG: zonefile = %q\n", zoneFile) expectedZone, err := os.ReadFile(zoneFile) if err != nil { continue diff --git a/pkg/js/parse_tests/059-rawttls.js b/pkg/js/parse_tests/059-rawttls.js new file mode 100644 index 000000000..11fa33b53 --- /dev/null +++ b/pkg/js/parse_tests/059-rawttls.js @@ -0,0 +1,25 @@ +D("example.com", "none", + + TXT("mytxt", "Do not call me on my phone"), + + // Test at the apex two ways: + RP("@", "user.example.com.", "mytxt.example.com."), + RP("example.com.", "user2.example.com.", "mytxt.example.com."), + + // Test the default TTL + RP("aaa300", "user.example.com.", "mytxt.example.com."), + + // Test DefaultTTL() + DefaultTTL(1111), + RP("bbb1", "user.example.com.", "mytxt.example.com."), + + // Test TTL() + RP("ccc2", "user.example.com.", "mytxt.example.com.", TTL(2222)), + + // Test the default TTL + RP("ddd1", "user.example.com.", "mytxt.example.com."), + + // Test a second DefaultTTL() + DefaultTTL(3333), + RP("eee3", "user.example.com.", "mytxt.example.com."), +); diff --git a/pkg/js/parse_tests/059-rawttls.json b/pkg/js/parse_tests/059-rawttls.json new file mode 100644 index 000000000..1b645a553 --- /dev/null +++ b/pkg/js/parse_tests/059-rawttls.json @@ -0,0 +1,180 @@ +{ + "dns_providers": [], + "domains": [ + { + "dnsProviders": {}, + "meta": { + "dnscontrol_nameraw": "example.com", + "dnscontrol_nameunicode": "example.com", + "dnscontrol_uniquename": "example.com" + }, + "name": "example.com", + "records": [ + { + "comparable": "user.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:6:5]", + "name": "@", + "name_raw": "@", + "name_unicode": "@", + "target": "", + "ttl": 300, + "type": "RP", + "zonfefilepartial": "user.example.com. mytxt.example.com." + }, + { + "comparable": "user2.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user2.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:7:5]", + "name": "@", + "name_raw": "@", + "name_unicode": "@", + "target": "", + "ttl": 300, + "type": "RP", + "zonfefilepartial": "user2.example.com. mytxt.example.com." + }, + { + "comparable": "user.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:10:5]", + "name": "aaa300", + "name_raw": "aaa300", + "name_unicode": "aaa300", + "target": "", + "ttl": 300, + "type": "RP", + "zonfefilepartial": "user.example.com. mytxt.example.com." + }, + { + "comparable": "user.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:14:5]", + "name": "bbb1", + "name_raw": "bbb1", + "name_unicode": "bbb1", + "target": "", + "ttl": 1111, + "type": "RP", + "zonfefilepartial": "user.example.com. mytxt.example.com." + }, + { + "comparable": "user.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:17:5]", + "name": "ccc2", + "name_raw": "ccc2", + "name_unicode": "ccc2", + "target": "", + "ttl": 2222, + "type": "RP", + "zonfefilepartial": "user.example.com. mytxt.example.com." + }, + { + "comparable": "user.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:20:5]", + "name": "ddd1", + "name_raw": "ddd1", + "name_unicode": "ddd1", + "target": "", + "ttl": 1111, + "type": "RP", + "zonfefilepartial": "user.example.com. mytxt.example.com." + }, + { + "comparable": "user.example.com. mytxt.example.com.", + "fields": { + "Hdr": { + "Class": 0, + "Name": "", + "Rdlength": 0, + "Rrtype": 0, + "Ttl": 0 + }, + "Mbox": "user.example.com.", + "Txt": "mytxt.example.com." + }, + "filepos": "[line:24:5]", + "name": "eee3", + "name_raw": "eee3", + "name_unicode": "eee3", + "target": "", + "ttl": 3333, + "type": "RP", + "zonfefilepartial": "user.example.com. mytxt.example.com." + }, + { + "filepos": "[line:3:5]", + "name": "mytxt", + "target": "Do not call me on my phone", + "ttl": 300, + "type": "TXT" + } + ], + "registrar": "none", + "uniquename": "example.com" + } + ], + "registrars": [] +} diff --git a/pkg/js/parse_tests/059-rawttls/example.com.zone b/pkg/js/parse_tests/059-rawttls/example.com.zone new file mode 100644 index 000000000..f2f5bbff5 --- /dev/null +++ b/pkg/js/parse_tests/059-rawttls/example.com.zone @@ -0,0 +1,9 @@ +$TTL 300 +@ IN RP user.example.com. mytxt.example.com. + IN RP user2.example.com. mytxt.example.com. +aaa300 IN RP user.example.com. mytxt.example.com. +bbb1 1111 IN RP user.example.com. mytxt.example.com. +ccc2 2222 IN RP user.example.com. mytxt.example.com. +ddd1 1111 IN RP user.example.com. mytxt.example.com. +eee3 3333 IN RP user.example.com. mytxt.example.com. +mytxt IN TXT "Do not call me on my phone" diff --git a/pkg/printer/printer.go b/pkg/printer/printer.go index b5ba9d338..617ac6db2 100644 --- a/pkg/printer/printer.go +++ b/pkg/printer/printer.go @@ -90,11 +90,7 @@ type ConsolePrinter struct { // StartDomain is called at the start of each domain. func (c ConsolePrinter) StartDomain(dc *models.DomainConfig) { - if dc.Name == dc.NameUnicode { - fmt.Fprintf(c.Writer, "******************** Domain: %s\n", dc.UniqueName) - } else { - fmt.Fprintf(c.Writer, "******************** Domain: %s (%s)\n", dc.UniqueName, dc.NameUnicode) - } + fmt.Fprintf(c.Writer, "******************** Domain: %s\n", dc.DisplayName) } // PrintCorrection is called to print/format each correction. diff --git a/pkg/rtype/rp.go b/pkg/rtype/rp.go index c4b9b15f2..f6b7cdc3d 100644 --- a/pkg/rtype/rp.go +++ b/pkg/rtype/rp.go @@ -1,6 +1,8 @@ package rtype import ( + "fmt" + "github.com/StackExchange/dnscontrol/v4/models" "github.com/StackExchange/dnscontrol/v4/pkg/domaintags" "github.com/StackExchange/dnscontrol/v4/pkg/rtypecontrol" @@ -25,7 +27,10 @@ func (handle *RP) Name() string { // FromArgs fills in the RecordConfig from []any, which is typically from a parsed config file. func (handle *RP) FromArgs(dcn *domaintags.DomainNameVarieties, rec *models.RecordConfig, args []any) error { if err := rtypecontrol.PaveArgs(args[1:], "ss"); err != nil { - return err + return fmt.Errorf("ERROR: (%s) [RP(%q, %v)]: %w", + rec.FilePos, + rec.Name, rtypecontrol.StringifyQuoted(args[1:]), + err) } fields := &RP{ dns.RP{ diff --git a/pkg/rtypecontrol/import.go b/pkg/rtypecontrol/import.go index 7d3d011f5..218a04a2e 100644 --- a/pkg/rtypecontrol/import.go +++ b/pkg/rtypecontrol/import.go @@ -14,11 +14,24 @@ func ImportRawRecords(domains []*models.DomainConfig) error { for _, dc := range domains { for _, rawRec := range dc.RawRecords { - rec, err := NewRecordConfigFromRaw(rawRec.Type, rawRec.TTL, rawRec.Args, dc.DomainNameVarieties()) + rec, err := NewRecordConfigFromRaw(rawRec.Type, rawRec.TTL, rawRec.Args, dc.DomainNameVarieties(), models.FixPosition(rawRec.FilePos)) if err != nil { return err } - rec.FilePos = models.FixPosition(rawRec.FilePos) + if rec.Metadata["skip_fqdn_check"] != "true" && stutters(rec.Name, dc.Name) { + var shortname string + if rec.Name == dc.Name { + shortname = "@" + } else { + shortname = strings.TrimSuffix(rec.Name, "."+dc.Name) + } + return fmt.Errorf( + "The name %q is an error (repeats the domain). Maybe instead of %q you intended %q? If not add DISABLE_REPEATED_DOMAIN_CHECK to this record to disable this check", + rec.NameFQDNRaw, + rec.NameRaw, + shortname, + ) + } // Free memeory: clear(rawRec.Args) @@ -32,10 +45,20 @@ func ImportRawRecords(domains []*models.DomainConfig) error { return nil } +func stutters(name, domain string) bool { + if name == "@" { + return false + } + if name == domain || strings.HasSuffix(name, "."+domain) { + return true + } + return false +} + // NewRecordConfigFromRaw creates a new RecordConfig from the raw ([]any) args, // usually from the parsed dnsconfig.js file, but also useful when a provider // returns the fields of a record as individual values. -func NewRecordConfigFromRaw(t string, ttl uint32, args []any, dcn *domaintags.DomainNameVarieties) (*models.RecordConfig, error) { +func NewRecordConfigFromRaw(t string, ttl uint32, args []any, dcn *domaintags.DomainNameVarieties, FilePos string) (*models.RecordConfig, error) { if _, ok := Func[t]; !ok { return nil, fmt.Errorf("record type %q is not supported", t) } @@ -48,8 +71,14 @@ func NewRecordConfigFromRaw(t string, ttl uint32, args []any, dcn *domaintags.Do Type: t, TTL: ttl, Metadata: map[string]string{}, + FilePos: FilePos, + } + if err := setRecordNames(rec, dcn, args[0].(string)); err != nil { + return rec, err + } + if strings.HasSuffix(rec.Name, ".") { + return nil, fmt.Errorf("label %q is not in zone %s", args[0].(string), dcn.DisplayName) } - setRecordNames(rec, dcn, args[0].(string)) // Fill in the .F/.Fields* fields. err := Func[t].FromArgs(dcn, rec, args) @@ -96,7 +125,12 @@ func NewRecordConfigFromStruct(name string, ttl uint32, t string, fields any, dc TTL: ttl, Metadata: map[string]string{}, } - setRecordNames(rec, dcn, name) + if err := setRecordNames(rec, dcn, name); err != nil { + return rec, err + } + if strings.HasSuffix(rec.Name, ".") { + return nil, fmt.Errorf("label %q is not in zone %q", name, dcn.NameASCII+".") + } err := Func[t].FromStruct(dcn, rec, name, fields) if err != nil { @@ -105,57 +139,3 @@ func NewRecordConfigFromStruct(name string, ttl uint32, t string, fields any, dc return rec, nil } - -// setRecordNames updates the .Name* fields. -func setRecordNames(rec *models.RecordConfig, dcn *domaintags.DomainNameVarieties, n string) { - - if strings.HasSuffix(n, ".") { - nr := n[:len(n)-1] - rec.Name = strings.ToLower(domaintags.EfficientToASCII(nr)) - rec.NameRaw = nr - rec.NameUnicode = domaintags.EfficientToUnicode(rec.Name) - rec.NameFQDN = rec.Name - rec.NameFQDNRaw = rec.NameRaw - rec.NameFQDNUnicode = rec.NameUnicode - return - } - - if rec.SubDomain == "" { - // Not _EXTEND() mode: - if n == "@" { - rec.Name = "@" - rec.NameRaw = "@" - rec.NameUnicode = "@" - rec.NameFQDN = dcn.NameASCII - rec.NameFQDNRaw = dcn.NameRaw - rec.NameFQDNUnicode = dcn.NameUnicode - } else { - rec.Name = strings.ToLower(domaintags.EfficientToASCII(n)) - rec.NameRaw = n - rec.NameUnicode = domaintags.EfficientToUnicode(n) - rec.NameFQDN = rec.Name + "." + dcn.NameASCII - rec.NameFQDNRaw = rec.NameRaw + "." + dcn.NameRaw - rec.NameFQDNUnicode = rec.NameUnicode + "." + dcn.NameUnicode - } - } else { - // D_EXTEND() mode: - sdRaw := rec.SubDomain - sdASCII := strings.ToLower(domaintags.EfficientToASCII(rec.SubDomain)) - sdUnicode := domaintags.EfficientToUnicode(sdASCII) - if n == "@" { - rec.Name = sdASCII - rec.NameRaw = sdRaw - rec.NameUnicode = sdUnicode - rec.NameFQDN = rec.Name + "." + dcn.NameASCII - rec.NameFQDNRaw = rec.NameRaw + "." + dcn.NameRaw - rec.NameFQDNUnicode = rec.NameUnicode + "." + dcn.NameUnicode - } else { - rec.Name = domaintags.EfficientToASCII(n) + "." + sdASCII - rec.NameRaw = n + "." + sdRaw - rec.NameUnicode = domaintags.EfficientToUnicode(rec.Name) - rec.NameFQDN = rec.Name + "." + dcn.NameASCII - rec.NameFQDNRaw = rec.NameRaw + "." + dcn.NameRaw - rec.NameFQDNUnicode = rec.NameUnicode + "." + dcn.NameUnicode - } - } -} diff --git a/pkg/rtypecontrol/import_test.go b/pkg/rtypecontrol/import_test.go new file mode 100644 index 000000000..50256ce1a --- /dev/null +++ b/pkg/rtypecontrol/import_test.go @@ -0,0 +1,59 @@ +package rtypecontrol + +import "testing" + +func Test_stutters(t *testing.T) { + tests := []struct { + name string + rName string + want bool + }{ + { + name: "@ symbol should not stutter", + rName: "@", + want: false, + }, + { + name: "exact domain match should stutter", + rName: "example.com", + want: true, + }, + { + name: "subdomain with dot prefix should stutter", + rName: "www.example.com", + want: true, + }, + { + name: "simple subdomain should not stutter", + rName: "www", + want: false, + }, + { + name: "partial match without dot should not stutter", + rName: "testexample.com", + want: false, + }, + { + name: "empty name should not stutter", + rName: "", + want: false, + }, + { + name: "nested subdomain should stutter", + rName: "api.staging.example.com", + want: true, + }, + { + name: "different domain should not stutter", + rName: "example.org", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := stutters(tt.rName, "example.com"); got != tt.want { + t.Errorf("stutters(%q, %q) = %v, want %v", tt.rName, "example.com", got, tt.want) + } + }) + } +} diff --git a/pkg/rtypecontrol/setrecordnames.go b/pkg/rtypecontrol/setrecordnames.go new file mode 100644 index 000000000..3593984b5 --- /dev/null +++ b/pkg/rtypecontrol/setrecordnames.go @@ -0,0 +1,193 @@ +package rtypecontrol + +import ( + "fmt" + "strings" + + "github.com/StackExchange/dnscontrol/v4/models" + "github.com/StackExchange/dnscontrol/v4/pkg/domaintags" +) + +// This code defines many variables to make the logic easier to read. The Go optimizer +// should eliminate any performance impact. +// We could probably fold some of the logic together, but it would be harder to read. +// It's difficult enough to understand it as-is, so clarity is preferred. + +// setRecordNames uses n to update the .Name* fields. If the name is a FQDN +// (ends with a "."), it will be handled accordingly. However if it does not +// match the domain name, no error is returned but rec.Name* fields will end +// with a ".". +func setRecordNames(rec *models.RecordConfig, dcn *domaintags.DomainNameVarieties, n string) error { + if rec.SubDomain == "" { + return setRecordNamesNonExtend(rec, dcn, n) + } + return setRecordNamesExtend(rec, dcn, n) +} + +func setRecordNamesNonExtend(rec *models.RecordConfig, dcn *domaintags.DomainNameVarieties, n string) error { + + nRaw := n + nASCII := domaintags.EfficientToASCII(n) + nUnicode := domaintags.EfficientToUnicode(nASCII) + + if strings.HasSuffix(nRaw, ".") { + // The user specified a FQDN, we give it special handling. + + // with trailing dot: + nameRawdot := nRaw + nameASCIIdot := nASCII + nameUnicodedot := nUnicode + // without trailing dot: + nameRaw := nameRawdot[:len(nameRawdot)-1] + nameASCII := nameASCIIdot[:len(nameASCIIdot)-1] + nameUnicode := nameUnicodedot[:len(nameUnicodedot)-1] + // suffix: + suffixRaw := dcn.NameRaw + suffixASCII := dcn.NameASCII + suffixUnicode := dcn.NameUnicode + dotsuffixASCII := "." + suffixASCII + + if nameASCII == suffixASCII { + // Name is the apex: + rec.NameRaw = "@" + rec.Name = "@" + rec.NameUnicode = "@" + rec.NameFQDNRaw = dcn.NameRaw + rec.NameFQDN = dcn.NameASCII + rec.NameFQDNUnicode = dcn.NameUnicode + return nil + } else if strings.HasSuffix(nameASCII, dotsuffixASCII) { + // Name is a subdomain of the domain: + rec.NameRaw = nameRaw[:len(nameRaw)-len(suffixRaw)-1] + rec.Name = nameASCII[:len(nameASCII)-len(suffixASCII)-1] + rec.NameUnicode = nameUnicode[:len(nameUnicode)-len(suffixUnicode)-1] + rec.NameFQDNRaw = nameRaw + rec.NameFQDN = nameASCII + rec.NameFQDNUnicode = nameUnicode + return nil + } + // Name is not in this domain. Don't error, just return names with trailing dots. + rec.NameRaw = nameRawdot + rec.Name = nameASCIIdot + rec.NameUnicode = nameUnicodedot + rec.NameFQDNRaw = nameRawdot + dcn.NameRaw + "." + rec.NameFQDN = nameASCIIdot + dcn.NameASCII + "." + rec.NameFQDNUnicode = nameUnicodedot + dcn.NameUnicode + "." + return nil + } + + // Non-FQDN case: + + if nRaw == "@" { + // Name is the apex. We never store "", we store "@". + rec.NameRaw = "@" + rec.Name = "@" + rec.NameUnicode = "@" + rec.NameFQDNRaw = dcn.NameRaw + rec.NameFQDN = dcn.NameASCII + rec.NameFQDNUnicode = dcn.NameUnicode + return nil + } + + // Everything else: + + nameRaw := nRaw + nameASCII := nASCII + nameUnicode := nUnicode + + rec.NameRaw = nameRaw + rec.Name = nameASCII + rec.NameUnicode = nameUnicode + rec.NameFQDNRaw = nameRaw + "." + dcn.NameRaw + rec.NameFQDN = nameASCII + "." + dcn.NameASCII + rec.NameFQDNUnicode = nameUnicode + "." + dcn.NameUnicode + return nil +} + +func setRecordNamesExtend(rec *models.RecordConfig, dcn *domaintags.DomainNameVarieties, n string) error { + // NB(tlim): When a record has a subdomain "foo" and domain "example.com", a + // record such as "www" is added as "www.foo" (short name) or + // "www.foo.example.com" (FQDN name). + // When generating the shortname, we are truncating the "D()" name, not the + // D_EXTEND() name. That is... dcn.NameASCII, not + // rec.SubDomain+dcn.NameASCII. + + nRaw := n + nASCII := domaintags.EfficientToASCII(n) + nUnicode := domaintags.EfficientToUnicode(nASCII) + + sdRaw := rec.SubDomain + sdASCII := domaintags.EfficientToASCII(sdRaw) + sdUnicode := domaintags.EfficientToUnicode(sdASCII) + + if strings.HasSuffix(nRaw, ".") { + // The user specified a FQDN, we give it special handling. + + // with trailing dot: + nameRawdot := nRaw + nameASCIIdot := nASCII + nameUnicodedot := nUnicode + // without trailing dot: + nameRaw := nameRawdot[:len(nameRawdot)-1] + nameASCII := nameASCIIdot[:len(nameASCIIdot)-1] + nameUnicode := nameUnicodedot[:len(nameUnicodedot)-1] + // suffixes: + dotsdsuffixASCII := "." + sdASCII + "." + dcn.NameASCII + dotsuffixASCII := "." + dcn.NameASCII + suffixASCII := dcn.NameASCII + + if strings.HasSuffix(nameASCII, dotsdsuffixASCII) { + // The name is in the D_EXTEND() domain: + rec.NameRaw = nameRaw[:len(nameRaw)-len(dcn.NameRaw)-1] + rec.Name = nameASCII[:len(nameASCII)-len(dcn.NameASCII)-1] + rec.NameUnicode = nameUnicode[:len(nameUnicode)-len(dcn.NameUnicode)-1] + rec.NameFQDNRaw = nameRaw + rec.NameFQDN = nameASCII + rec.NameFQDNUnicode = nameUnicode + return nil + } + if strings.HasSuffix(nameASCII, suffixASCII) || strings.HasSuffix(nameASCII, dotsuffixASCII) { + // The name is NOT in the D_EXTEND() domain; but it is in the D() domain. + return fmt.Errorf("label %q should be in the apex domain, D(%q), not D_EXTEND(%q)", + nameRaw, + dcn.NameRaw, + rec.SubDomain+"."+dcn.NameRaw, + ) + } + // The name is not i the D_EXTEND() nor D() domain. Don't error, just return names with trailing dots. + rec.NameRaw = nameRawdot + rec.Name = nameASCIIdot + rec.NameUnicode = nameUnicodedot + rec.NameFQDNRaw = nameRawdot + rec.NameFQDN = nameASCIIdot + rec.NameFQDNUnicode = nameUnicodedot + return nil + } + + // Non-FQDN case: + + if nRaw == "@" { + // Name is the apex. Therefore, the subdomain is the name. + rec.NameRaw = sdRaw + rec.Name = sdASCII + rec.NameUnicode = sdUnicode + rec.NameFQDNRaw = rec.NameRaw + "." + dcn.NameRaw + rec.NameFQDN = rec.Name + "." + dcn.NameASCII + rec.NameFQDNUnicode = rec.NameUnicode + "." + dcn.NameUnicode + return nil + } + + // Everything else: + + nameRaw := nRaw + nameASCII := nASCII + nameUnicode := nUnicode + + rec.NameRaw = nameRaw + "." + sdRaw + rec.Name = nameASCII + "." + sdASCII + rec.NameUnicode = nameUnicode + "." + sdUnicode + rec.NameFQDNRaw = rec.NameRaw + "." + dcn.NameRaw + rec.NameFQDN = rec.Name + "." + dcn.NameASCII + rec.NameFQDNUnicode = rec.NameUnicode + "." + dcn.NameUnicode + return nil +} diff --git a/pkg/rtypecontrol/setrecordnames_test.go b/pkg/rtypecontrol/setrecordnames_test.go index 8be330bed..d5bb80865 100644 --- a/pkg/rtypecontrol/setrecordnames_test.go +++ b/pkg/rtypecontrol/setrecordnames_test.go @@ -24,8 +24,10 @@ func TestSetRecordNames(t *testing.T) { rec *models.RecordConfig dc *domaintags.DomainNameVarieties n string + expectedErr bool expectedRec *models.RecordConfig }{ + { name: "normal_at", rec: &models.RecordConfig{}, @@ -172,42 +174,196 @@ func TestSetRecordNames(t *testing.T) { NameFQDNUnicode: "www.bücher.bücher.com", }, }, + { - name: "dotted_label", + name: "dotted_normal_at", rec: &models.RecordConfig{}, dc: dc, n: "example.com.", expectedRec: &models.RecordConfig{ - Name: "example.com", - NameRaw: "example.com", - NameUnicode: "example.com", + Name: "@", + NameRaw: "@", + NameUnicode: "@", NameFQDN: "example.com", NameFQDNRaw: "example.com", NameFQDNUnicode: "example.com", }, }, + { + name: "dotted_normal_label_outside", + rec: &models.RecordConfig{}, + dc: dc, + n: "www.example.com.", + expectedRec: &models.RecordConfig{ + Name: "www", + NameRaw: "www", + NameUnicode: "www", + NameFQDN: "www.example.com", + NameFQDNRaw: "www.example.com", + NameFQDNUnicode: "www.example.com", + }, + }, + { + name: "dotted_normal_idn_label", + rec: &models.RecordConfig{}, + dc: dc, + n: "bücher.example.com.", + expectedRec: &models.RecordConfig{ + Name: "xn--bcher-kva", + NameRaw: "bücher", + NameUnicode: "bücher", + NameFQDN: "xn--bcher-kva.example.com", + NameFQDNRaw: "bücher.example.com", + NameFQDNUnicode: "bücher.example.com", + }, + }, + { + name: "dotted_normal_idn_domain", + rec: &models.RecordConfig{}, + dc: dcIDN, + n: "www.bücher.com.", + expectedRec: &models.RecordConfig{ + Name: "www", + NameRaw: "www", + NameUnicode: "www", + NameFQDN: "www.xn--bcher-kva.com", + NameFQDNRaw: "www.bücher.com", + NameFQDNUnicode: "www.bücher.com", + }, + }, + { + name: "dotted_extend_at", + rec: &models.RecordConfig{SubDomain: "sub"}, + dc: dc, + n: "example.com.", + expectedErr: true, + }, + { + name: "dotted_extend_label", + rec: &models.RecordConfig{SubDomain: "sub"}, + dc: dc, + n: "www.example.com.", + expectedErr: true, + }, + { + name: "dotted_extend_idn_subdomain", + rec: &models.RecordConfig{SubDomain: "bücher"}, + dc: dc, + n: "www.bücher.example.com.", + expectedRec: &models.RecordConfig{ + SubDomain: "bücher", + + NameRaw: "www.bücher", + Name: "www.xn--bcher-kva", + NameUnicode: "www.bücher", + NameFQDNRaw: "www.bücher.example.com", + NameFQDN: "www.xn--bcher-kva.example.com", + NameFQDNUnicode: "www.bücher.example.com", + }, + }, + { + name: "dotted_extend_idn_label", + rec: &models.RecordConfig{SubDomain: "sub"}, + dc: dc, + n: "bücher.sub.example.com.", + expectedRec: &models.RecordConfig{ + SubDomain: "sub", + Name: "xn--bcher-kva.sub", + NameRaw: "bücher.sub", + NameUnicode: "bücher.sub", + NameFQDN: "xn--bcher-kva.sub.example.com", + NameFQDNRaw: "bücher.sub.example.com", + NameFQDNUnicode: "bücher.sub.example.com", + }, + }, + { + name: "dotted_extend_idn_subdomain_and_label", + rec: &models.RecordConfig{SubDomain: "bücher"}, + dc: dc, + n: "könig.bücher.example.com.", + expectedRec: &models.RecordConfig{ + SubDomain: "bücher", + Name: "xn--knig-5qa.xn--bcher-kva", + NameRaw: "könig.bücher", + NameUnicode: "könig.bücher", + NameFQDN: "xn--knig-5qa.xn--bcher-kva.example.com", + NameFQDNRaw: "könig.bücher.example.com", + NameFQDNUnicode: "könig.bücher.example.com", + }, + }, + { + name: "dotted_extend_idn_domain_and_subdomain", + rec: &models.RecordConfig{SubDomain: "bücher"}, + dc: dcIDN, + n: "www.bücher.bücher.com.", + expectedRec: &models.RecordConfig{ + SubDomain: "bücher", + Name: "www.xn--bcher-kva", + NameRaw: "www.bücher", + NameUnicode: "www.bücher", + NameFQDN: "www.xn--bcher-kva.xn--bcher-kva.com", + NameFQDNRaw: "www.bücher.bücher.com", + NameFQDNUnicode: "www.bücher.bücher.com", + }, + }, + + { + name: "dotted_apex", + rec: &models.RecordConfig{}, + dc: dc, + n: "example.com.", + expectedRec: &models.RecordConfig{ + Name: "@", + NameRaw: "@", + NameUnicode: "@", + NameFQDN: "example.com", + NameFQDNRaw: "example.com", + NameFQDNUnicode: "example.com", + }, + }, + + { + name: "dotted_label", + rec: &models.RecordConfig{}, + dc: dcIDN, + n: "www.bücher.com.", + expectedRec: &models.RecordConfig{ + Name: "www", + NameRaw: "www", + NameUnicode: "www", + NameFQDN: "www.xn--bcher-kva.com", + NameFQDNRaw: "www.bücher.com", + NameFQDNUnicode: "www.bücher.com", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - setRecordNames(tt.rec, tt.dc, tt.n) - if tt.rec.Name != tt.expectedRec.Name { - t.Errorf("Name: got %q, want %q", tt.rec.Name, tt.expectedRec.Name) - } - if tt.rec.NameRaw != tt.expectedRec.NameRaw { - t.Errorf("NameRaw: got %q, want %q", tt.rec.NameRaw, tt.expectedRec.NameRaw) - } - if tt.rec.NameUnicode != tt.expectedRec.NameUnicode { - t.Errorf("NameUnicode: got %q, want %q", tt.rec.NameUnicode, tt.expectedRec.NameUnicode) - } - if tt.rec.NameFQDN != tt.expectedRec.NameFQDN { - t.Errorf("NameFQDN: got %q, want %q", tt.rec.NameFQDN, tt.expectedRec.NameFQDN) - } - if tt.rec.NameFQDNRaw != tt.expectedRec.NameFQDNRaw { - t.Errorf("NameFQDNRaw: got %q, want %q", tt.rec.NameFQDNRaw, tt.expectedRec.NameFQDNRaw) - } - if tt.rec.NameFQDNUnicode != tt.expectedRec.NameFQDNUnicode { - t.Errorf("NameFQDNUnicode: got %q, want %q", tt.rec.NameFQDNUnicode, tt.expectedRec.NameFQDNUnicode) + gotErr := setRecordNames(tt.rec, tt.dc, tt.n) + if (gotErr != nil && (!tt.expectedErr)) || (gotErr == nil && tt.expectedErr) { + t.Errorf("Error: got \"%v\", want %v", gotErr, tt.expectedErr) + } else if gotErr != nil && tt.expectedErr { + // Expected error, test passed. + } else { + if tt.rec.NameRaw != tt.expectedRec.NameRaw { + t.Errorf("NameRaw: got %q, want %q", tt.rec.NameRaw, tt.expectedRec.NameRaw) + } + if tt.rec.Name != tt.expectedRec.Name { + t.Errorf("Name: got %q, want %q", tt.rec.Name, tt.expectedRec.Name) + } + if tt.rec.NameUnicode != tt.expectedRec.NameUnicode { + t.Errorf("NameUnicode: got %q, want %q", tt.rec.NameUnicode, tt.expectedRec.NameUnicode) + } + if tt.rec.NameFQDN != tt.expectedRec.NameFQDN { + t.Errorf("NameFQDN: got %q, want %q", tt.rec.NameFQDN, tt.expectedRec.NameFQDN) + } + if tt.rec.NameFQDNRaw != tt.expectedRec.NameFQDNRaw { + t.Errorf("NameFQDNRaw: got %q, want %q", tt.rec.NameFQDNRaw, tt.expectedRec.NameFQDNRaw) + } + if tt.rec.NameFQDNUnicode != tt.expectedRec.NameFQDNUnicode { + t.Errorf("NameFQDNUnicode: got %q, want %q", tt.rec.NameFQDNUnicode, tt.expectedRec.NameFQDNUnicode) + } } }) } diff --git a/pkg/rtypecontrol/stringify.go b/pkg/rtypecontrol/stringify.go new file mode 100644 index 000000000..c2357c870 --- /dev/null +++ b/pkg/rtypecontrol/stringify.go @@ -0,0 +1,19 @@ +package rtypecontrol + +import ( + "fmt" + "strings" +) + +func StringifyQuoted(args []any) string { + if len(args) == 0 { + return "" + } + + var sb strings.Builder + sb.WriteString(fmt.Sprintf("%q", args[0])) + for _, arg := range args[1:] { + sb.WriteString(fmt.Sprintf(" %q", arg)) + } + return sb.String() +} diff --git a/providers/cloudflare/rest.go b/providers/cloudflare/rest.go index 389ae3038..62da6480b 100644 --- a/providers/cloudflare/rest.go +++ b/providers/cloudflare/rest.go @@ -326,7 +326,8 @@ func (c *cloudflareProvider) getSingleRedirects(id string, domain string) ([]*mo "CLOUDFLAREAPI_SINGLE_REDIRECT", 1, []any{srName, code, srWhen, srThen}, - domaintags.MakeDomainNameVarieties(domain)) + domaintags.MakeDomainNameVarieties(domain), + "") if err != nil { return nil, err }