From 159fdf07adc0335fa4aea2c3ea5d4c05664de17b Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Sat, 18 Nov 2023 15:06:20 -0500 Subject: [PATCH] REFACTOR: providers should not directly access .TxtStrings (#2629) --- models/t_txt.go | 13 +++++++++++++ providers/axfrddns/axfrddnsProvider.go | 4 ++-- providers/azuredns/azureDnsProvider.go | 4 ++-- providers/cscglobal/dns.go | 10 +++++----- providers/digitalocean/auditrecords.go | 3 +-- providers/domainnameshop/dns.go | 2 +- providers/gcore/convert.go | 2 +- providers/hetzner/types.go | 4 ++-- providers/hexonet/records.go | 4 ++-- providers/hostingde/types.go | 5 +++-- providers/inwx/inwxProvider.go | 3 ++- providers/loopia/auditrecords.go | 3 ++- providers/msdns/powershell.go | 2 +- providers/namedotcom/auditrecords.go | 7 ++++--- providers/namedotcom/records.go | 2 +- providers/ns1/ns1Provider.go | 2 +- providers/softlayer/softlayerProvider.go | 5 ++++- providers/vultr/vultrProvider.go | 2 +- 18 files changed, 48 insertions(+), 29 deletions(-) diff --git a/models/t_txt.go b/models/t_txt.go index 87b7470de..7b7b78a4d 100644 --- a/models/t_txt.go +++ b/models/t_txt.go @@ -157,6 +157,19 @@ func (rc *RecordConfig) GetTargetTXTSegmented() []string { return splitChunks(strings.Join(rc.TxtStrings, ""), 255) } +// GetTargetTXTSegmentCount returns the number of 255-octet segments required to store TXT target. +func (rc *RecordConfig) GetTargetTXTSegmentCount() int { + var total int + for i := range rc.TxtStrings { + total = len(rc.TxtStrings[i]) + } + segs := total / 255 // integer division, decimals are truncated + if (total % 255) > 0 { + return segs + 1 + } + return segs +} + func splitChunks(buf string, lim int) []string { var chunk string chunks := make([]string, 0, len(buf)/lim+1) diff --git a/providers/axfrddns/axfrddnsProvider.go b/providers/axfrddns/axfrddnsProvider.go index b7a1f17ce..f283ee941 100644 --- a/providers/axfrddns/axfrddnsProvider.go +++ b/providers/axfrddns/axfrddnsProvider.go @@ -327,8 +327,8 @@ func (c *axfrddnsProvider) GetZoneRecords(domain string, meta map[string]string) last := foundRecords[len(foundRecords)-1] if last.Type == "TXT" && last.Name == dnssecDummyLabel && - len(last.TxtStrings) == 1 && - last.TxtStrings[0] == dnssecDummyTxt { + last.GetTargetTXTSegmentCount() == 1 && + last.GetTargetTXTSegmented()[0] == dnssecDummyTxt { c.hasDnssecRecords = true foundRecords = foundRecords[0:(len(foundRecords) - 1)] } diff --git a/providers/azuredns/azureDnsProvider.go b/providers/azuredns/azureDnsProvider.go index cb2230132..105e46e6e 100644 --- a/providers/azuredns/azureDnsProvider.go +++ b/providers/azuredns/azureDnsProvider.go @@ -525,9 +525,9 @@ func (a *azurednsProvider) recordToNativeDiff2(recordKey models.RecordKey, recor recordSet.Properties.TxtRecords = []*adns.TxtRecord{} } // Empty TXT record needs to have no value set in it's properties - if !(len(rec.TxtStrings) == 1 && rec.TxtStrings[0] == "") { + if !(rec.GetTargetTXTSegmentCount() == 1 && rec.GetTargetTXTSegmented()[0] == "") { var txts []*string - for _, txt := range rec.TxtStrings { + for _, txt := range rec.GetTargetTXTSegmented() { txts = append(txts, to.StringPtr(txt)) } recordSet.Properties.TxtRecords = append(recordSet.Properties.TxtRecords, &adns.TxtRecord{Value: txts}) diff --git a/providers/cscglobal/dns.go b/providers/cscglobal/dns.go index 4cf3cb517..e78432a3b 100644 --- a/providers/cscglobal/dns.go +++ b/providers/cscglobal/dns.go @@ -132,7 +132,7 @@ func makePurge(domainname string, cor diff.Correlation) zoneResourceRecordEdit { switch cor.Existing.Type { case "TXT": - existingTarget = strings.Join(cor.Existing.TxtStrings, "") + existingTarget = cor.Existing.GetTargetTXTJoined() default: existingTarget = cor.Existing.GetTargetField() } @@ -159,7 +159,7 @@ func makeAdd(domainname string, cre diff.Correlation) zoneResourceRecordEdit { var recTarget string switch rec.Type { case "TXT": - recTarget = strings.Join(rec.TxtStrings, "") + recTarget = rec.GetTargetTXTJoined() default: recTarget = rec.GetTargetField() } @@ -185,7 +185,7 @@ func makeAdd(domainname string, cre diff.Correlation) zoneResourceRecordEdit { zer.NewWeight = rec.SrvWeight zer.NewPort = rec.SrvPort case "TXT": - zer.NewValue = strings.Join(rec.TxtStrings, "") + zer.NewValue = rec.GetTargetTXTJoined() default: // "A", "CNAME", "NS" // Nothing to do. } @@ -201,8 +201,8 @@ func makeEdit(domainname string, m diff.Correlation) zoneResourceRecordEdit { var oldTarget, recTarget string switch old.Type { case "TXT": - oldTarget = strings.Join(old.TxtStrings, "") - recTarget = strings.Join(rec.TxtStrings, "") + oldTarget = old.GetTargetTXTJoined() + recTarget = rec.GetTargetTXTJoined() default: oldTarget = old.GetTargetField() recTarget = rec.GetTargetField() diff --git a/providers/digitalocean/auditrecords.go b/providers/digitalocean/auditrecords.go index 49263f45c..95382cc92 100644 --- a/providers/digitalocean/auditrecords.go +++ b/providers/digitalocean/auditrecords.go @@ -44,10 +44,9 @@ func MaxLengthDO(rc *models.RecordConfig) error { // In other words, they're doing the checking on the API protocol // encoded data instead of on on the resulting TXT record. Sigh. - if len(rc.GetTargetField()) > 509 { + if len(rc.GetTargetRFC1035Quoted()) > 509 { return fmt.Errorf("encoded txt too long") } - // FIXME(tlim): Try replacing GetTargetField() with (2 + (3*len(rc.TxtStrings) - 1)) return nil } diff --git a/providers/domainnameshop/dns.go b/providers/domainnameshop/dns.go index e7619c11d..320506b6f 100644 --- a/providers/domainnameshop/dns.go +++ b/providers/domainnameshop/dns.go @@ -30,7 +30,7 @@ func (api *domainNameShopProvider) GetZoneRecordsCorrections(dc *models.DomainCo // Merge TXT strings to one string for _, rc := range dc.Records { if rc.HasFormatIdenticalToTXT() { - rc.SetTargetTXT(strings.Join(rc.TxtStrings, "")) + rc.SetTargetTXT(rc.GetTargetTXTJoined()) } } diff --git a/providers/gcore/convert.go b/providers/gcore/convert.go index f3e4ebb5a..b5c1cc6bf 100644 --- a/providers/gcore/convert.go +++ b/providers/gcore/convert.go @@ -86,7 +86,7 @@ func recordsToNative(rcs []*models.RecordConfig, expectedKey models.RecordKey) * } case "TXT": // Avoid double quoting for TXT records rr = dnssdk.ResourceRecord{ - Content: convertTxtSliceToSdkAnySlice(r.TxtStrings), + Content: convertTxtSliceToSdkAnySlice(r.GetTargetTXTSegmented()), Meta: nil, Enabled: true, } diff --git a/providers/hetzner/types.go b/providers/hetzner/types.go index 80ad3c8e7..20dacdfb1 100644 --- a/providers/hetzner/types.go +++ b/providers/hetzner/types.go @@ -63,7 +63,7 @@ func fromRecordConfig(in *models.RecordConfig, zone *zone) record { ZoneID: zone.ID, } - if r.Type == "TXT" && len(in.TxtStrings) == 1 { + if r.Type == "TXT" && (in.GetTargetTXTSegmentCount() == 1) { // HACK: HETZNER rejects values that fit into 255 bytes w/o quotes, // but do not fit w/ added quotes (via GetTargetCombined()). // Sending the raw, non-quoted value works for the comprehensive @@ -71,7 +71,7 @@ func fromRecordConfig(in *models.RecordConfig, zone *zone) record { // The HETZNER validation does not provide helpful error messages. // {"error":{"message":"422 Unprocessable Entity: missing: ; ","code":422}} // Last checked: 2023-04-01 - valueNotQuoted := in.TxtStrings[0] + valueNotQuoted := in.GetTargetTXTSegmented()[0] if len(valueNotQuoted) == 254 || len(valueNotQuoted) == 255 { r.Value = valueNotQuoted } diff --git a/providers/hexonet/records.go b/providers/hexonet/records.go index 3cb1846db..ac313f714 100644 --- a/providers/hexonet/records.go +++ b/providers/hexonet/records.go @@ -242,7 +242,7 @@ func (n *HXClient) createRecordString(rc *models.RecordConfig, domain string) (s case "CAA": record.Answer = fmt.Sprintf(`%v %s "%s"`, rc.CaaFlag, rc.CaaTag, record.Answer) case "TXT": - record.Answer = encodeTxt(rc.TxtStrings) + record.Answer = encodeTxt(rc.GetTargetTXTSegmented()) case "SRV": if rc.GetTargetField() == "." { return "", fmt.Errorf("SRV records with empty targets are not supported (as of 2020-02-27, the API returns 'Invalid attribute value syntax')") @@ -267,7 +267,7 @@ func (n *HXClient) deleteRecordString(record *HXRecord, domain string) string { return record.Raw } -// encodeTxt encodes TxtStrings for sending in the CREATE/MODIFY API: +// encodeTxt encodes []string for sending in the CREATE/MODIFY API: func encodeTxt(txts []string) string { var r []string for _, txt := range txts { diff --git a/providers/hostingde/types.go b/providers/hostingde/types.go index 0a1f924a5..4f789ba2a 100644 --- a/providers/hostingde/types.go +++ b/providers/hostingde/types.go @@ -193,8 +193,9 @@ func recordToNative(rc *models.RecordConfig) *record { case "A", "AAAA", "ALIAS", "CAA", "CNAME", "DNSKEY", "DS", "NS", "NSEC", "NSEC3", "NSEC3PARAM", "PTR", "RRSIG", "SSHFP", "TSLA": // Nothing special. case "TXT": - txtStrings := make([]string, len(rc.TxtStrings)) - copy(txtStrings, rc.TxtStrings) + // TODO(tlim): Move this to a function with unit tests. + txtStrings := make([]string, rc.GetTargetTXTSegmentCount()) + copy(txtStrings, rc.GetTargetTXTSegmented()) // Escape quotes for i := range txtStrings { diff --git a/providers/inwx/inwxProvider.go b/providers/inwx/inwxProvider.go index 0c4cfdaf8..a2de4cb03 100644 --- a/providers/inwx/inwxProvider.go +++ b/providers/inwx/inwxProvider.go @@ -213,9 +213,10 @@ func (api *inwxAPI) deleteRecord(RecordID int) error { // checkRecords ensures that there is no single-quote inside TXT records which would be ignored by INWX. func checkRecords(records models.Records) error { + // TODO(tlim) Remove this function. auditrecords.go takes care of this now. for _, r := range records { if r.Type == "TXT" { - for _, target := range r.TxtStrings { + for _, target := range r.GetTargetTXTSegmented() { if strings.ContainsAny(target, "`") { return fmt.Errorf("INWX TXT records do not support single-quotes in their target") } diff --git a/providers/loopia/auditrecords.go b/providers/loopia/auditrecords.go index 0accc622c..d9d8cb2d5 100644 --- a/providers/loopia/auditrecords.go +++ b/providers/loopia/auditrecords.go @@ -2,6 +2,7 @@ package loopia import ( "fmt" + "github.com/StackExchange/dnscontrol/v4/models" "github.com/StackExchange/dnscontrol/v4/pkg/rejectif" ) @@ -24,7 +25,7 @@ func AuditRecords(records []*models.RecordConfig) []error { // TxtHasSegmentLen450orLonger audits TXT records for strings that are >450 octets. func TxtHasSegmentLen450orLonger(rc *models.RecordConfig) error { - for _, txt := range rc.TxtStrings { + for _, txt := range rc.GetTargetTXTSegmented() { if len(txt) > 450 { return fmt.Errorf("%q txtstring length > 450", rc.GetLabel()) } diff --git a/providers/msdns/powershell.go b/providers/msdns/powershell.go index 367deb5a5..75fef9d45 100644 --- a/providers/msdns/powershell.go +++ b/providers/msdns/powershell.go @@ -316,7 +316,7 @@ func generatePSCreate(dnsserver, domain string, rec *models.RecordConfig) string //case "WKS": // fmt.Fprintf(&b, ` -Wks -InternetAddress -InternetProtocol {UDP | TCP} -Service `, rec.GetTargetField()) case "TXT": - //printer.Printf("DEBUG TXT len = %v\n", rec.TxtStrings) + //printer.Printf("DEBUG TXT len = %v\n", rec.GetTargetTXTSegmentCount()) //printer.Printf("DEBUG TXT target = %q\n", rec.GetTargetField()) fmt.Fprintf(&b, ` -Txt -DescriptiveText %q`, rec.GetTargetTXTJoined()) //case "RT": diff --git a/providers/namedotcom/auditrecords.go b/providers/namedotcom/auditrecords.go index 7db65ad50..2d9a64c84 100644 --- a/providers/namedotcom/auditrecords.go +++ b/providers/namedotcom/auditrecords.go @@ -29,18 +29,19 @@ func AuditRecords(records []*models.RecordConfig) []error { // are longer than permitted by NDC. Sadly their // length limit is undocumented. This seems to work. func MaxLengthNDC(rc *models.RecordConfig) error { - if len(rc.TxtStrings) == 0 { + txtStrings := rc.GetTargetTXTSegmented() + if len(txtStrings) == 0 { return nil } sum := 2 // Count the start and end quote. // Add the length of each segment. - for _, segment := range rc.TxtStrings { + for _, segment := range txtStrings { sum += len(segment) // The length of each segment sum += strings.Count(segment, `"`) // Add 1 for any char to be escaped } // Add 3 (quote space quote) for each interior join. - sum += 3 * (len(rc.TxtStrings) - 1) + sum += 3 * (len(txtStrings) - 1) if sum > 512 { return fmt.Errorf("encoded txt too long") diff --git a/providers/namedotcom/records.go b/providers/namedotcom/records.go index b9edb4ef8..af58fde7b 100644 --- a/providers/namedotcom/records.go +++ b/providers/namedotcom/records.go @@ -154,7 +154,7 @@ func (n *namedotcomProvider) createRecord(rc *models.RecordConfig, domain string case "A", "AAAA", "ANAME", "CNAME", "MX", "NS": // nothing case "TXT": - // record.Answer = encodeTxt(rc.TxtStrings) + // nothing case "SRV": if rc.GetTargetField() == "." { return errors.New("SRV records with empty targets are not supported (as of 2019-11-05, the API returns 'Parameter Value Error - Invalid Srv Format')") diff --git a/providers/ns1/ns1Provider.go b/providers/ns1/ns1Provider.go index 59a048cc3..a92db2860 100644 --- a/providers/ns1/ns1Provider.go +++ b/providers/ns1/ns1Provider.go @@ -302,7 +302,7 @@ func buildRecord(recs models.Records, domain string, id string) *dns.Record { if r.Type == "MX" { 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}) + rec.AddAnswer(&dns.Answer{Rdata: r.GetTargetTXTSegmented()}) } else if r.Type == "CAA" { rec.AddAnswer(&dns.Answer{ Rdata: []string{ diff --git a/providers/softlayer/softlayerProvider.go b/providers/softlayer/softlayerProvider.go index 4238302b8..a47d432a6 100644 --- a/providers/softlayer/softlayerProvider.go +++ b/providers/softlayer/softlayerProvider.go @@ -173,7 +173,10 @@ func (s *softlayerProvider) getExistingRecords(domain *datatypes.Dns_Domain) (mo } recConfig.SetLabel(fmt.Sprintf("%s.%s", service, strings.ToLower(protocol)), *domain.Name) case "TXT": - recConfig.TxtStrings = append(recConfig.TxtStrings, *record.Data) + // OLD: recConfig.TxtStrings = append(recConfig.TxtStrings, *record.Data) + recConfig.SetTargetTXTs(append(recConfig.GetTargetTXTSegmented(), *record.Data)) + // NB(tlim) The above code seems too complex. Can it be simplied to this? + // recConfig.SetTargetTXT(*record.Data) fallthrough case "MX": if record.MxPriority != nil { diff --git a/providers/vultr/vultrProvider.go b/providers/vultr/vultrProvider.go index 1a5b5b577..c09ec349d 100644 --- a/providers/vultr/vultrProvider.go +++ b/providers/vultr/vultrProvider.go @@ -314,7 +314,7 @@ func toVultrRecord(dc *models.DomainConfig, rc *models.RecordConfig, vultrID str // Vultr doesn't permit TXT strings to include double-quotes // therefore, we don't have to escape interior double-quotes. // Vultr's API requires the string to begin and end with double-quotes. - r.Data = `"` + strings.Join(rc.TxtStrings, "") + `"` + r.Data = `"` + strings.Join(rc.GetTargetTXTSegmented(), "") + `"` default: }