Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ares_query_txt_result_chunk to return bytes rather than str #160

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

maelp
Copy link

@maelp maelp commented Aug 10, 2021

The RFC1035 specification says that TXT records are allowed to contain data made of arbitrary binary data, but right now aiodns is parsing it as a str, and therefore will stop on a null byte, this PR fixes this

The RFC1035 specification says that TXT records are allowed to contain <character-string> data made of arbitrary binary data, but right now aiodns is parsing it as a str, and therefore will stop on a null byte, this PR fixes this
@saghul
Copy link
Owner

saghul commented Aug 11, 2021

I think we need to apply the same to ares_query_txt_result, don't we? Also, I wonder why the GH action with the tests didn't run...

@maelp
Copy link
Author

maelp commented Aug 11, 2021 via email

@saghul
Copy link
Owner

saghul commented Aug 11, 2021

Sure thing, no rush, and thanks for helping out with this!

ares_query_txt_result returns bytes so that it stays coherent
@maelp
Copy link
Author

maelp commented Aug 14, 2021

@saghul I guess that now that both return bytes it should work as expected

maelp added 3 commits August 14, 2021 22:42
Free the hostent in both cases
Reorder to be consistent with initial code
@maelp
Copy link
Author

maelp commented Aug 17, 2021

It seems that the test_query_txt_multiple_chunked_with_non_ascii_content test fails because it is checking the server txt-non-ascii.dns-test.hmnid.ru which does not respond

https://mxtoolbox.com/SuperTool.aspx?action=txt%3atxt-non-ascii.dns-test.hmnid.ru&run=toolpage

@maelp
Copy link
Author

maelp commented Aug 17, 2021

Also the test_idna_encoding_query_a test seems to fail because the A query to españa.icom.museum seems to respond with a NXDOMAIN and SOA response (non-existent domain)

; <<>> DiG 9.10.6 <<>> españa.icom.museum
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 15201
;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 1, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 4096
;; QUESTION SECTION:
;espa\195\177a.icom.museum.     IN      A

;; AUTHORITY SECTION:
icom.museum.            600     IN      SOA     ns.oxymium.net. hostmaster.oxymium.net. 2021072901 28800 3600 604800 7200

;; Query time: 432 msec
;; SERVER: 172.20.10.1#53(172.20.10.1)
;; WHEN: Tue Aug 17 14:33:51 CEST 2021
;; MSG SIZE  rcvd: 109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants