search

openresty / lua-resty-dns

DNS resolver for the nginx lua module
324 stars 107 forks source link

bugfix: fix parsing state after SOA record #31

Closed Lekensteyn closed 7 years ago

Lekensteyn commented 7 years ago

Correct parsing of Additional Records failed due to a bad parsing state after processing a SOA record in the Authorative nameservers section.

There is no regression test for this "trivial" fix since the only way I know of is to do a query against an authorative DNS server, e.g.:

dig SOA google.com @ns1.google.com

however, the current tests seems designed such that it can work without relying on an external DNS server (by setting TEST_NGINX_RESOLVER). Hope this is OK?

(and yet another option is to do an ANY query, e.g. dig ANY example.com, but this type has been deprecated...)

agentzh commented 7 years ago

@Lekensteyn We can mock it up in the existing t/mock.t test file. We'd better cover this fix in the test suite otherwise it might accidentally get broken again in the future.

And it's also fine to use google or openresty's real authoritative severs in the test suite.

Lekensteyn commented 7 years ago

Just added some mock tests. It is also possible to add sanity test, but that would be the first to query external DNS servers, is that OK?

(The real DNS response actually compresses names which is not done by TestDNS.pm, but it should be good enough I think?)

agentzh commented 7 years ago

@Lekensteyn Merged. Thanks! And sorry for the delay on my side.

Lekensteyn commented 7 years ago

It was actually me that was did not quickly follow up on your latest comment. Thanks for taking this and rolling out a release, that was actually fast!

agentzh commented 7 years ago

@Lekensteyn Thanks for your nice words!