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

Nicer handling of PTR queries #50

Closed
Daniel15 opened this issue Feb 5, 2017 · 6 comments · Fixed by #52
Closed

Nicer handling of PTR queries #50

Daniel15 opened this issue Feb 5, 2017 · 6 comments · Fixed by #52
Assignees

Comments

@Daniel15
Copy link

Daniel15 commented Feb 5, 2017

amphp DNS doesn't handle calls like this:

yield Amp\Dns\resolve('209.141.56.29', $options = ['types' => Amp\Dns\Record::PTR])

Currently, you need to manually convert the IP address to the right PTR query:

yield Amp\Dns\resolve('29.56.141.209.in-addr.arpa', $options = ['types' => Amp\Dns\Record::PTR])

The legacy Net_DNS library has some code to handle PTR queries correctly, when the input looks like an IP address:

        /*
         * If the name looks like an IP address then do an appropriate
         * PTR query.
         */
        if (preg_match('/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/', $name, $regs)) {
            $name = $regs[4].'.'.$regs[3].'.'.$regs[2].'.'.$regs[1].'.in-addr.arpa.';
            $type = 'PTR';
        }

https://github.com/pear/Net_DNS/blob/5786b8c6f21c3cc8915fe251dac1571fa524a417/Net/DNS/Resolver.php#L552-L559

It would be nice for amphp to do this too.

@kelunik
Copy link
Member

kelunik commented Feb 5, 2017

You should use Amp\Dns\query for everything that doesn't resolve to an IP address. So in this case yield Amp\Dns\query('8.8.8.8.in-addr.arpa', Amp\Dns\Record::PTR); But you're right, we could add auto-rewrite here.

@bwoebi Didn't we have a protection that restricts resolve to A and AAAA queries?

@Daniel15
Copy link
Author

Daniel15 commented Feb 5, 2017

Amp\Dns\resolve actually seems to work properly, Amp\Dns\resolve('29.56.141.209.in-addr.arpa', ['types' => Amp\Dns\Record::PTR]) gives me the right result.

@kelunik
Copy link
Member

kelunik commented Feb 5, 2017

Yes, I know that it works, but it's not supposed to work. I think we had a restriction in there before.

@kelunik kelunik self-assigned this Feb 5, 2017
@kelunik
Copy link
Member

kelunik commented Feb 5, 2017

Do you have any suggestion for a better function name? I'm not sure what to do with an invalid IP, probably throw an exception, but not sure which one. InvalidAddressException might make sense, but it should probably not extend the ResolutionException in this case.

function ip2arpa($ip) {
    $packedIp = inet_pton($ip);

    if ($packedIp === false) {
        return null; // FIXME
    }

    if (isset($packedIp[5])) { // IPv6
        return wordwrap(strrev(bin2hex($packedIp)), 1, ".", true) . ".ip6.arpa";
    }

    return inet_ntop(strrev($packedIp)) . ".in-addr.arpa";
}

@kelunik
Copy link
Member

kelunik commented Feb 5, 2017

I created a PR now. It doesn't use a separate function, see #52.

@bwoebi bwoebi closed this as completed in #52 Feb 5, 2017
@kelunik
Copy link
Member

kelunik commented Feb 6, 2017

It's now included in https://github.com/amphp/dns/releases/tag/v0.8.14.

peter279k pushed a commit to peter279k/dns that referenced this issue Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants