Error when retrieving binary data in TXT records
maelp opened this issue · 8 comments
The RFC1035 indicates that TXT records are "" which are to be interpreted as binary data of length at most 255, prefixed with a length character.
If I'm packing a TXT record containing a null byte, it is not correctly parsed by the library and the parsed output contains an empty buffer
Do you know if c-area is capa of parsing those? Do you have a sample record I could query?
I'm not sure whether c-ares is doing it as it should or not, but I guess it should be supposed to because the RFC indicates that's how it should be done
For instance I'm trying to use this byte string as a TXT record (I'm using dnslib to pack the data, which works as expected when I inspect the packet, and aiodns to receive and parse the packet, which shows an empty record):
b'\x00\x08\xa7\xe4\xca\x88\x06\x12\x02\x12'
and (perhaps because the first byte is null) it outputs as an empty string
The bug is most likely here: https://github.com/saghul/pycares/blob/4e6e36f839255ebef05e0682b98cbee1533805ce/src/pycares/__init__.py#L749-L764 -- the txt reply structures do have a length field I'm not using, I'm relying on strlen really.
A patch would be most welcome!
I guess it is this indeed https://github.com/saghul/pycares/blob/4e6e36f839255ebef05e0682b98cbee1533805ce/src/pycares/utils.py#L21
it should not necessarily force a decoding as ascii, it should be a "bytes" response (but this might break existing code as it means that the result of the resp.text is "bytes" rather than "str", and it is to each user to know how to interpret it, although this would be the correct way to do it)
We are already returning bytes sometimes, and str just when the response is strictly ascii. Can't we keep that behavior?
So I guess the real "error" is there, trying to cast to a _ffi.string when it should stay as a byte array (but not sure how to do this using the lib?) https://github.com/saghul/pycares/blob/4e6e36f839255ebef05e0682b98cbee1533805ce/src/pycares/__init__.py#L763
The correct way seems to be
self.text = bytes(_ffi.buffer(txt.txt, txt.length))
This PR fixes the bug saghul/pycares#160