MaddieM4 / pymads

A fork of the pymds authoritative DNS server, designed for asynchronous lookup without domain restrictions.
GNU Lesser General Public License v3.0
4 stars 2 forks source link

For recursive queries, use the same question type and class as original request ($10) #33

Closed MaddieM4 closed 11 years ago

MaddieM4 commented 11 years ago

Pymads currently isn't clever about this. If you run:

dig @localhost -t AAAA ipv6.google.com
dig @8.8.8.8 -t AAAA ipv6.google.com

You'll see that the results differ. This is because Pymads effectively sends out this recursive request:

dig @8.8.8.8 -t A ipv6.google.com

Which reflects the original request in the default case, but not when you ask for anything special.

This issue is fixed when the results of those top two commands are the same, and the recursive request's qtype and qclass are the same as the original request's.

MaddieM4 commented 11 years ago

Bounty is $10.

http://www.freedomsponsors.org/core/issue/298/for-recursive-queries-use-the-same-question-type-and-class-as-original-request


(Copied from acceptance criteria)

Edit the DnsSource class so that it uses the same qtype and qclass as the original request when formulating recursive requests.

Another really simple issue that I'm only bountying to get people involved.

clehner commented 11 years ago

The DnsSource has to get passed the qtype and qclass, so I changed it to take the whole request object from consumer. But there is other test code that gives DnsSource only a domain name string as a request, so I changed DnsSource and DictSource to take either a domain name string or request object. Also DJSource has to be similarly changed. Do you see a better way of doing this?

MaddieM4 commented 11 years ago

@clehner Thanks for working on this!

I don't really like the idea of that conditional logic having to be duplicated everywhere. Since this is a somewhat breaking change anyways, I'd like to code in the flexibility at the Chain level, and have the source objects only considered with handling Request objects in self.get().

If it looks useful for testing purposes, create a base class for the various sources, so that they all have a get_domain_string() function that takes a domain name string, creates a Request from it, and returns self.get(locally_generated_request).

I'll handle the necessary changes in DJDNS, so you don't have to worry about that.

clehner commented 11 years ago

Ok, will do.

clehner commented 11 years ago

Added and it works on my local server with some modifications to python-djdns which I will push.

MaddieM4 commented 11 years ago

It does break a bunch of tests still, according to Travis-CI. Let me know when it passes the tests and is ready for final review. Looks good so far, though.

On Mon, Jul 15, 2013 at 9:33 PM, Charles Lehner notifications@github.comwrote:

Added and it works on my local server with some modifications to python-djdns which I will push.

— Reply to this email directly or view it on GitHubhttps://github.com/campadrenalin/pymads/issues/33#issuecomment-21020668 .

clehner commented 11 years ago

I got a few more tests to pass. The others I still have to spend some more time on.

clehner commented 11 years ago

Tests are passing.

MaddieM4 commented 11 years ago

That's fantastic. I'm still waist-deep in my day job at the moment, but I'm really looking forward to reviewing and merging this pull request.

clehner commented 11 years ago

Great!