diff --git a/commands/getZones.go b/commands/getZones.go index 6a09468ee..50fbbe3e6 100644 --- a/commands/getZones.go +++ b/commands/getZones.go @@ -351,12 +351,7 @@ func formatDsl(zonename string, rec *models.RecordConfig, defaultTTL uint32) str case "TLSA": target = fmt.Sprintf("%d, %d, %d, '%s'", rec.TlsaUsage, rec.TlsaSelector, rec.TlsaMatchingType, rec.GetTargetField()) case "TXT": - if len(rec.TxtStrings) == 1 { - target = `'` + rec.TxtStrings[0] + `'` - } else { - target = `['` + strings.Join(rec.TxtStrings, `', '`) + `']` - } - // TODO(tlim): If this is an SPF record, generate a SPF_BUILDER(). + target = jsonQuoted(rec.GetTargetField()) case "NS": // NS records at the apex should be NAMESERVER() records. // DnsControl uses the API to get this info. NAMESERVER() is just diff --git a/models/record.go b/models/record.go index 80b206223..dbd06e781 100644 --- a/models/record.go +++ b/models/record.go @@ -127,7 +127,6 @@ type RecordConfig struct { TlsaUsage uint8 `json:"tlsausage,omitempty"` TlsaSelector uint8 `json:"tlsaselector,omitempty"` TlsaMatchingType uint8 `json:"tlsamatchingtype,omitempty"` - TxtStrings []string `json:"txtstrings,omitempty"` // TxtStrings stores all strings (including the first). Target stores all the strings joined. R53Alias map[string]string `json:"r53_alias,omitempty"` AzureAlias map[string]string `json:"azure_alias,omitempty"` } @@ -195,7 +194,6 @@ func (rc *RecordConfig) UnmarshalJSON(b []byte) error { TlsaUsage uint8 `json:"tlsausage,omitempty"` TlsaSelector uint8 `json:"tlsaselector,omitempty"` TlsaMatchingType uint8 `json:"tlsamatchingtype,omitempty"` - TxtStrings []string `json:"txtstrings,omitempty"` // TxtStrings stores all strings (including the first). Target stores only the first one. R53Alias map[string]string `json:"r53_alias,omitempty"` AzureAlias map[string]string `json:"azure_alias,omitempty"` diff --git a/models/record_test.go b/models/record_test.go index f4015197e..e654bd33a 100644 --- a/models/record_test.go +++ b/models/record_test.go @@ -88,7 +88,6 @@ func TestRecordConfig_Copy(t *testing.T) { TlsaUsage uint8 TlsaSelector uint8 TlsaMatchingType uint8 - TxtStrings []string R53Alias map[string]string AzureAlias map[string]string Original interface{} @@ -135,7 +134,6 @@ func TestRecordConfig_Copy(t *testing.T) { TlsaUsage: 1, TlsaSelector: 2, TlsaMatchingType: 3, - TxtStrings: []string{"one", "two", "three"}, R53Alias: map[string]string{"a": "eh", "b": "bee"}, AzureAlias: map[string]string{"az": "az", "ure": "your"}, //Original interface{}, @@ -174,7 +172,6 @@ func TestRecordConfig_Copy(t *testing.T) { TlsaUsage: 1, TlsaSelector: 2, TlsaMatchingType: 3, - TxtStrings: []string{"one", "two", "three"}, R53Alias: map[string]string{"a": "eh", "b": "bee"}, AzureAlias: map[string]string{"az": "az", "ure": "your"}, //Original interface{}, @@ -217,7 +214,6 @@ func TestRecordConfig_Copy(t *testing.T) { TlsaUsage: tt.fields.TlsaUsage, TlsaSelector: tt.fields.TlsaSelector, TlsaMatchingType: tt.fields.TlsaMatchingType, - TxtStrings: tt.fields.TxtStrings, R53Alias: tt.fields.R53Alias, AzureAlias: tt.fields.AzureAlias, Original: tt.fields.Original, diff --git a/models/t_txt.go b/models/t_txt.go index 4a39c06c4..6e59658a3 100644 --- a/models/t_txt.go +++ b/models/t_txt.go @@ -10,104 +10,18 @@ Sadly many providers handle TXT records in strange and non-compliant ways. DNSControl has to handle all of them. Over the years we've tried many things. This explain the current state of the code. -What are some of these variations? +DNSControl stores the TXT record target as a single string of any length. +Providers take care of any splitting, excaping, or quoting. -* The RFCs say that a TXT record is a series of strings, each 255-octets - or fewer. Yet, most provider APIs only support a single string which - is split into 255-octetl chunks behind the scenes. Some only support - a single string that is 255-octets or less. +NOTE: Older versions of DNSControl stored the TXT record as +represented by the provider, which could be a single string, a series +of smaller strings, or a single string that is quoted/escaped. This +created tons of edge-cases and other distractions. -* The RFCs don't say much about the content of the strings. Some - providers accept any octet, some only accept ASCII-printable chars, - some get confused by TXT records that include backticks, quotes, or - whitespace at the end of the string. - -DNSControl has tried many different ways to handle all these -variations over the years. This is what we found works best: - -Principle 1. Store the string as the user input it. - -DNSControl stores the string as the user specified in dnsconfig.js. -The user can specify a string of any length, or many individual -strings of any length. - -No matter how the user presented the data in dnsconfig.js, the data is -stored as a list of strings (RecordConfig.TxtStrings []string). If -they input 1 string, the list has one element. If the user input many -individual strings, the list is copied into .TxtStrings. - -When we store the data in .TxtStrings there is no length checking. The data is not manipulated. - -Principle 2. When downloading zone records, receive the data as appropriate. - -When the API returns a TXT record, the provider's code must properly -store it in the .TxtStrings field of RecordConfig. - -We've found most APIs return TXT strings in one of three ways: - - * The API returns a single string: use RecordConfig.SetTargetTXT(). - * The API returns multiple strings: use RecordConfig.SetTargetTXTs(). - * (THIS IS RARE) The API returns a single string that must be parsed - into multiple strings: The provider is responsible for the - parsing. However, usually the format is "quoted like in RFC 1035" - which is vague, but we've implemented it as - RecordConfig.SetTargetTXTfromRFC1035Quoted(). - -If the format is something else, please write the parser as a separate -function and write unit tests based on actual data received from the -API. - -Principle 3. When sending TXT records to the API, send what the API expects. - -The provider's code must decide how to take the list of strings in -.TxtStrings and present them to the API. - -Most providers fall into one of these categories: - - * If the API expects one long string, the provider code joins all - the smaller strings and sends one big string. Use the helper - function RecordConfig.GetTargetTXTJoined() - * If the API expects many strings of any size, the provider code - sends the individual strings. Those strings are accessed as - the array RecordConfig.TxtStrings - * (THIS IS RARE) If the API expects multiple strings to be sent as - one long string, quoted RFC 1025-style, call - RecordConfig.GetTargetRFC1035Quoted() and send that string. - -Note: If the API expects many strings, each 255-octets or smaller, the -provider code must split the longer strings into smaller strings. The -helper function txtutil.SplitSingleLongTxt(dc.Records) will iterate -over all TXT records and split out any strings longer than 255 octets. -Call this once in GetDomainCorrections(). (Yes, this violates -Principle 1, but we decided it is best to do it once, than provide a -getter that would re-split the strings on every call.) - -Principle 4. Providers can communicate back to DNSControl strings they can't handle. - -As mentioned before, some APIs reject TXT records for various reasons: -Illegal chars, whitespace at the end, etc. We can't make a flag for -every variation. Instead we call the provider's AuditRecords() -function and it reports if there are any records that it can't -process. - -We've provided many helper functions to make this easier. Look at any -of the providers/.../auditrecord.go` files for examples. - -The integration tests call AuditRecords() to skip any tests that we -know will fail. If one of the integration tests is failing, it is -often better to update AuditRecords() than to try to figure out why, -for example, the provider doesn't support backticks in strings. Don't -spend a lot of effort trying to fix situations that are rare or will -not appear in real-world situations. - -Companies do update their APIs occasionally. You might want to try -eliminating the checks one at a time to see if the API has improved. -Don't feel obligated to do this more than once a year. - -Conclusion: - -When we follow these 4 principles, and stick with the helper functions -provided, we're able to handle all the variations. +If a provider doesn't support certain charactors in a TXT record, use +the providers/$PROVIDER/auditrecords.go file to indicate this. +DNSControl uses this information to warn users of unsupporrted input, +and to skip related integration tests. */ @@ -128,8 +42,7 @@ func (rc *RecordConfig) SetTargetTXT(s string) error { panic("assertion failed: SetTargetTXT called when .Type is not TXT or compatible type") } - rc.TxtStrings = []string{s} - rc.SetTarget(rc.zoneFileQuoted()) + rc.SetTarget(s) return nil } @@ -142,14 +55,14 @@ func (rc *RecordConfig) SetTargetTXTs(s []string) error { panic("assertion failed: SetTargetTXTs called when .Type is not TXT or compatible type") } - rc.TxtStrings = s - rc.SetTarget(rc.zoneFileQuoted()) + rc.SetTarget(strings.Join(s)) return nil } // GetTargetTXTJoined returns the TXT target as one string. If it was stored as multiple strings, concatenate them. +// Deprecated: GetTargetTXTJoined is deprecated. Use GetTargetField() func (rc *RecordConfig) GetTargetTXTJoined() string { - return strings.Join(rc.TxtStrings, "") + return rc.GetTargetField() } // SetTargetTXTfromRFC1035Quoted parses a series of quoted strings @@ -161,6 +74,7 @@ func (rc *RecordConfig) GetTargetTXTJoined() string { // "foo bar" << 1 string // "foo" "bar" << 2 strings // foo << error. No quotes! Did you intend to use SetTargetTXT? +// Deprecated: GetTargetTXTJoined is deprecated. ...or should be. func (rc *RecordConfig) SetTargetTXTfromRFC1035Quoted(s string) error { if s != "" && s[0] != '"' { // If you get this error, it is likely that you should use diff --git a/models/target.go b/models/target.go index 6d3c38620..58e1b34ab 100644 --- a/models/target.go +++ b/models/target.go @@ -23,12 +23,6 @@ var debugWarnTxtField = false // GetTargetField returns the target. There may be other fields (for example // an MX record also has a .MxPreference field. func (rc *RecordConfig) GetTargetField() string { - if debugWarnTxtField { - if rc.Type == "TXT" { - fmt.Printf("DEBUG: WARNING: GetTargetField called on TXT record is frequently wrong: %q\n", rc.target) - //debug.PrintStack() - } - } return rc.target } diff --git a/pkg/txtutil/txtutil.go b/pkg/txtutil/txtutil.go index 5aa6b1d4b..a6caf5e05 100644 --- a/pkg/txtutil/txtutil.go +++ b/pkg/txtutil/txtutil.go @@ -1,20 +1,8 @@ package txtutil -import "github.com/StackExchange/dnscontrol/v4/models" - -// SplitSingleLongTxt finds TXT records with a single long string and splits it -// into 255-octet chunks. This is used by providers that, when a user specifies -// one long TXT string, split it into smaller strings behind the scenes. -// This should be called from GetZoneRecordsCorrections(). -func SplitSingleLongTxt(records []*models.RecordConfig) { - for _, rc := range records { - if rc.HasFormatIdenticalToTXT() { - s := rc.TxtStrings[0] - if len(rc.TxtStrings) == 1 && len(s) > 255 { - rc.SetTargetTXTs(splitChunks(s, 255)) - } - } - } +// ToChunks returns the string as chunks of 255-octet strings (the last string being the remainder). +func ToChunks(s string) []string { + return splitChunks(s, 255) } func splitChunks(buf string, lim int) []string {