Fix parsing of TXT responses

[jwakefieldau]

For TXT responses, RDATA consists of character strings, of which the first byte is the length of the string. Currently this is returned along with the actual character data. This patch will return the concatenation of all character strings, minus length metadata, found in RDATA

[michalc]

Hi @jwakefieldau,

Thanks for the PR. I'm actually a bit torn about having this merged in: this was never meant to be tool for fetching all sorts of DNS records. It is just for resolving a host to an IP address. For example:

• It always follows CNAMEs, with no option of returning the actual CNAME record itself.
• How I'm using this now, the added code parsing TXT records won't ever be used, and I have no plan to use it. It would be more unused code.

(I know I added TYPES.TXT... but that was more of a just in case thing... and I was considering removing it...)

So I would suggest two options

• Run a hard fork of this project that you can add the TXT parsing behaviour in, and any other behaviour you would like
• Parse the data in BytesExpiresAt after it is returned from the resolve function [I think I'm happy to support that minor bit of functionality of allowing non A/AAAA requests, but passing through any parsing responsibility to clients].

Thanks,

Michal

[jwakefieldau]

Hi Michal,

First, thanks for developing this module, it fits nicely with a requirement we had at work for resolving DNS en masse.

As you might guess, our tool requires specifically resolving TXT queries. While we could in theory have the caller parse out the length fields from the responses, conversion of response data to lowercase would break our code, because some of those TXT records contain base64-encoded public keys and certificates that we need to be able to validate.

So if I'm understanding your comment correctly, in this case, it makes more sense to not merge this PR, and maintain a separate fork with the additional TXT record support.

[michalc]

conversion of response data to lowercase would break our code

Oh! I didn't realise/remember it did this. Yeah... I don't think aiodnsresolver should lowercase the raw bytes of the response... it feels pretty broken/unexpected. I would be pro changing this.

[jwakefieldau]

In that case, I think what I might do is start a new branch that fixes the lowercase issue only, create a PR for that, and then close this one, as I'd like to retain the behaviour I've added for my own fork. I can see lowercasing being potentially useful for NS or CNAME lookups, would you want that? Or moreso, "raw" should be "raw" and don't change the response at all?

[michalc]

In that case, I think what I might do is start a new branch that fixes the lowercase issue only, create a PR for that

Have now done this in https://github.com/michalc/aiodnsresolver/pull/57

In terms of better parsing the txt record or other records, I'm good for it to be out of scope.