dnscontrol: MAINT: PopulateFromString should use SetTargetTXT, not SetTargetTXTString
[NOTE: This is not urgent. I’m asking that everyone check their code in the next 4-5 weeks: I’d like to merge this on Aug 1, 2022]
The good news: PopulateFromString is very useful and a lot of people use it.
The bad news: PopulateFromString has a bug, and I need to check with everyone to make sure they aren’t affected.
WHAT IS THE PROBLEM?
PopulateFromString calls SetTargetTXTString, which is almost never the right thing to do. It should use SetTargetTXT() instead.
SetTargetTXTString parses the string for quotes… incorrectly. Generally provider code receives a big TXT string that doesn’t need to be parsed.
WHY CHANGE?
The integration tests do not detect if the incorrect SetTargetTXT* function is called. I can’t figure out a test that would do that. Therefore, we have to make this change carefully.
Here is a checklist of who uses this function:
- akamaiedgedns @svernick
- autodns @arnoschoon
- cloudflare @tresni
- desec @D3luxee
- dnsimple @aeden
- dnsmadeeasy @vojtad
- exoscale pierre-emmanuelJ
- gandiv5 @TomOnTime
- gcloud @riyadhalnur
- hedns @rblenkinsopp
- hetzner @das7pad
- hexonet @papakai
- hostingde @juliusrickert
- inwx @svenpeter42
- linode @koesie10
- namecheap @captncraig
- namedotcom
- ns1 @costasd
- oracle @kallsyms
- ovh @masterzen
- powerdns @jpbede
- route53 @tresni
- transip @blackshadev
- vultr @pgaskin
WHAT SHOULD I DO?
Git clone this and run your integration tests.
If your integration tests work, let me know.
if the integration tests fail, I’d be glad to help you fix them.
I’ve written a longer explanation of how TXT records are handled in models/t_txt.go
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 33 (30 by maintainers)
Commits related to this issue
- Fix population of txt records from TransIP (#1581) As requested by tlim, the PopulateFromString has changed and thus some providers need to handle TXT records differently. See issue linked below fo... — committed to StackExchange/dnscontrol by blackshadev 2 years ago
- HOSTINGDE: Use QuoteEscapedFields for TXT records Re: #1568 — committed to juliusrickert/dnscontrol by juliusrickert 2 years ago
- HOSTINGDE: Use QuoteEscapedFields for TXT records (#1590) Re: #1568 — committed to StackExchange/dnscontrol by juliusrickert 2 years ago
Hey folks! I’m back from my travel.
First, I had a realization while traveling: I could have made this a lot easier for people if I had just renamed the old function to be
PopulateFromStringOld()and asked people to submit individual PRs at your own pace. The way I did this, requiring everyone to comment on one big PR, was not the best way to do it. I apologize for making this so complex. Often the only way to see the right way is to do it the wrong way first. Mea culpa! I’ll do better next time!Second, I’ve added some helper functions so you don’t have to call
decode.Parse*()directly.Thanks! I’ve merged them.
@riyadhalnur Excellent!