classification
Title: Add ipaddress property to get name of reverse DNS PTR record
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: eric.smith, leonn, ncoghlan, pitrou, pmoody, python-dev
Priority: normal Keywords: patch

Created on 2014-02-02 01:22 by leonn, last changed 2014-04-14 16:59 by eric.smith. This issue is now closed.

Files
File name Uploaded Description Edit
ipaddress_reverse_names.patch leonn, 2014-02-02 01:22 patch implementing the feature review
ipaddress_reverse_names_v2.patch leonn, 2014-02-02 02:13 patch adding the reverse_pointer property review
ipaddress_reverse_names_v3.patch leonn, 2014-02-02 12:54 patch with updated documentation review
ipaddress_reverse_names_v3_alt.patch leonn, 2014-02-02 16:31 alternative patch without trailing dots review
ipaddress_reverse_names_v4.patch leonn, 2014-02-06 12:24 no trailing dots, with consistent variable naming review
Messages (23)
msg209933 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 01:22
I was missing a method to compute the reverse DNS name for an IP address, and I felt this is something that would belong in the ipaddress module; so here’s a patch for the ipaddress module adding a reverse_name property to IPv?Address.

This is an example:

>>> ipaddress.ip_address("127.0.0.1").reverse_name
'1.0.0.127.in-addr.arpa.'
>>> ipaddress.ip_address("2001:db8::1").reverse_name
'1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.'

Would this be an acceptable feature for inclusion in the ipaddress module?
msg209935 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-02 01:33
I think the functionality is reasonable for this module.

When I originally read the bug title, I was concerned that it was actually doing a reverse DNS lookup, which would not be appropriate. But now I realize it's just computing the name that would be used for the reverse lookup.

I'm not sure "reverse_name" is the best name for this, but I don't have any better suggestions, and I could live with this name.

Changing version to 3.5, since that's the first version this could be added to.
msg209936 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-02-02 01:47
Heh, my initial reaction based on the issue title was the same as Eric's, but yes, I agree the pure text manipulation proposed in the patch is actually a good fit.

Rather than "reverse_name" (which I feel is ambiguous about whether or not it does the DNS lookup to resolve back to the canonical name for the address), I would suggest the attribute name "reverse_pointer" or "reverse_record" (with a slight preference for the former, as indicated in the updated issue title).
msg209937 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-02 01:55
Thanks for the patch, Leon. Is the trailing dot actually desired?

>>> ipaddress.ip_address("127.0.0.1").reverse_name
'1.0.0.127.in-addr.arpa.'

Also, to accept your contribution, we will need you to fill a contributor's agreement. See http://www.python.org/psf/contrib/
msg209938 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 02:13
Thanks for the feedback, I agree "reverse_pointer" is a better, less ambiguous name for the property. I’ve amended the patch to reflect this suggestion.

Regarding the trailing dot, I felt it more appropriate to have it that to leave it out, but I don’t have a strong opinion on this. It has to be there in the DNS query, but most tools will automatically add it to the query if it’s not specified. The "host" tool from bind-utils is undecided as well when printing the output, it includes the trailing dot when querying IPv4 addresses, but not for IPv6 addresses.

Including the trailing dot has the advantage that the output could be directly fed into other code that strictly requires it, like the dnspython module.

I’ll sign and submit the contributor’s agreement as soon as I’m within reach of a printer (likely within the next 24 hours).
msg209939 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-02-02 02:18
Gah, we still haven't fixed the contributor license docs on the main
CLA page (hopefully we'll finally have that sorted later this month).
In the meantime, if you go directly to
http://www.python.org/psf/contrib/contrib-form/ it should give you the
option to sign electronically:
http://www.python.org/psf/contrib/contrib-form/
msg209941 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 02:28
Oh nice, then fewer trees have to die. I’ve now signed the contributor’s agreement.
msg209960 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-02 08:07
I'd prefer to leave the trailing dots. It's the technically correct thing to do.

And as they say, technically correct is the best kind of correct!
msg209962 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-02 08:25
The patch looks good to me, save for a versionadded tag in the docs. I've mentioned it on Rietveld.

Not to get in to bikeshedding on the name, but I'd prefer something with PTR in the name. I'm an old-timer, and think of these as "PTR lookups" (because they are!). If I were looking for it, that's what I'd search for.

And if not in the property name, then at least the documentation should mention it.
msg209963 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-02 08:26
Or, now that I think about it some more, maybe leave the name as reverse_pointer and just mention PTR records in the documentation.
msg209977 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-02 12:49
> I'd prefer to leave the trailing dots. It's the technically correct thing to do.

It depends what we're talking about. Hostnames (in the general sense)
don't carry a trailing dot except in DNS queries. So I'd argue the
trailing dot is more part of the DNS protocol than of the hostname
itself.

As for what the "host" command does, it doesn't add any trailing dots
here:

$ host 127.0.0.1
1.0.0.127.in-addr.arpa domain name pointer XXX.
$ host ::1
1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa domain name pointer XXX.

(this is bind9-host from Ubuntu)

The Wikipedia page doesn't use trailing dots:
http://en.wikipedia.org/wiki/Reverse_DNS_lookup

Neither do the examples from the RIPE itself:
http://www.ripe.net/data-tools/dns/reverse-dns/how-to-set-up-reverse-delegation
msg209980 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 12:54
I’ve changed the wording in the documentation a bit and added an explanatory sentence, how about this?

I think this should make pretty clear what it does and does not, and also easy to find in the documentation when searching for “reverse”, “PTR” and “pointer” (which is what I can think of what one would be looking for).

(Not sure whether to add further patches in roundup or rietveld, but a quick google search reveals patches are periodically added to rietveld automatically, so I’ll just add it here. Please let me know if this is not correct :))
msg209981 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 13:12
> As for what the "host" command does, it doesn't add any trailing dots
> here:

Oh, interesting. It does on Fedora 19 with version 9.9.3, but not on my Ubuntu installation with version 9.8.1. So it seems this was added between 9.8.1 and 9.9.3.
msg209982 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-02 13:32
> Oh, interesting. It does on Fedora 19 with version 9.9.3, but not on
> my Ubuntu installation with version 9.8.1. So it seems this was added
> between 9.8.1 and 9.9.3.

According to apt-cache, the version I tried with is
1:9.9.3.dfsg.P2-4ubuntu1
msg209986 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 14:02
> According to apt-cache, the version I tried with is
> 1:9.9.3.dfsg.P2-4ubuntu1

Ok, then it depends on something else apparently. It’s not so relevant anyway because even with the trailing dot, “host” is inconsistent between IPv4 and IPv6, and we should choose a consistent variant.

I’m still slightly in favour of having the trailing dot, because it is the correct representation of the DNS name. RFC1034 section 3.1 explicitly states that “Since a complete domain name ends with the root label, this leads to a printed form which ends in a dot.”. It goes on to explain that relative names (names not ending in a dot) “appear mostly at the user interface, where their interpretation varies from implementation to implementation” and that the most common interpretation is relative to the root zone.

So leaving the dot off is merely a convention, which is good enough for Wikipedia and RIPE documents; however I think in an API we should implement the more strict option. Also, I don’t see anything breaking with the dot, while without the dot, a user needs to take extra care when feeding the name to dnspython’s dns.resolver.query(), which would do undesired things without the trailing dot (append a search domain if there is one in /etc/resolv.conf).

Any other opinions?
msg209987 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-02 14:09
Well that convention is also used in existing APIs:

>>> socket.gethostbyaddr("8.8.8.8")
('google-public-dns-a.google.com', [], ['8.8.8.8'])

(note that this merely reflects a POSIX API)
msg209995 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-02 15:51
I concede the point on the trailing dot.

The reason we used to always use it is that it prevented the local resolver from first trying to append the host's default domain name (as, for example, defined in /etc/resolv.conf as "search"). I have no idea if resolvers still do that. Some quick research shows that that's probably not their behavior any more, at least if the domain name contains at least one dot.
msg209996 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-02 15:52
The documentation change looks good to me.
msg210000 - (view) Author: Leon Weber (leonn) * Date: 2014-02-02 16:31
I can live without the trailing dot, although I’d find it “more correct” to have it.

I’ve attached an alternative patch that doesn’t return a trailing dot, it’s up to you guys to decide which one you prefer.
msg210049 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-02-02 22:59
From a "principle of least surprise" point of view, non-specialists would
likely be puzzled by a trailing dot and wonder why it was there. By
contrast, specialists that know the trailing dot is technically *supposed*
to be there are already going to be accustomed to seeing it left out
(because it is already left out in so many other contexts).

So I'm +1 for leaving out the trailing dot, as that likely won't surprise
*any* users.
msg210378 - (view) Author: Leon Weber (leonn) * Date: 2014-02-06 12:24
I noticed an inconsistency between IPv4 and IPv6 in my temporary variable names when looking at the patch again after a few days. Here’s an updated patch. Not that it’s important, but I sleep better that way :)

As consensus seems to have settled on leaving out the trailing dot, I only updated the version without the dot.
msg210379 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2014-02-06 12:29
Looks good to me. Once 3.5 is open for new features, I'll commit this. (But feel free to ping me if you think it's lingering!)

Thanks!
msg216112 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-14 16:58
New changeset 5992d2c9522c by Eric V. Smith in branch 'default':
Issue #20480: Add ipaddress.reverse_pointer. Patch by Leon Weber.
http://hg.python.org/cpython/rev/5992d2c9522c
History
Date User Action Args
2014-04-14 16:59:19eric.smithsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2014-04-14 16:58:30python-devsetnosy: + python-dev
messages: + msg216112
2014-02-06 12:29:18eric.smithsetassignee: eric.smith
messages: + msg210379
2014-02-06 12:24:32leonnsetfiles: + ipaddress_reverse_names_v4.patch

messages: + msg210378
2014-02-02 22:59:04ncoghlansetmessages: + msg210049
2014-02-02 16:31:43leonnsetfiles: + ipaddress_reverse_names_v3_alt.patch

messages: + msg210000
2014-02-02 15:52:53eric.smithsetmessages: + msg209996
2014-02-02 15:51:58eric.smithsetmessages: + msg209995
2014-02-02 14:09:47pitrousetmessages: + msg209987
2014-02-02 14:02:09leonnsetmessages: + msg209986
2014-02-02 13:32:56pitrousetmessages: + msg209982
2014-02-02 13:12:46leonnsetmessages: + msg209981
2014-02-02 12:54:20leonnsetfiles: + ipaddress_reverse_names_v3.patch

messages: + msg209980
2014-02-02 12:49:35pitrousetmessages: + msg209977
2014-02-02 08:26:57eric.smithsetmessages: + msg209963
2014-02-02 08:25:50eric.smithsetmessages: + msg209962
2014-02-02 08:07:34eric.smithsetmessages: + msg209960
title: Add ipaddress property to get name of reverse DNS pointer -> Add ipaddress property to get name of reverse DNS PTR record
2014-02-02 02:28:46leonnsetmessages: + msg209941
2014-02-02 02:18:50ncoghlansetmessages: + msg209939
2014-02-02 02:13:20leonnsetfiles: + ipaddress_reverse_names_v2.patch

messages: + msg209938
2014-02-02 01:55:07pitrousetnosy: + pitrou

messages: + msg209937
stage: patch review
2014-02-02 01:47:50ncoghlansetmessages: + msg209936
title: Add ipaddress property to get reverse DNS name -> Add ipaddress property to get name of reverse DNS pointer
2014-02-02 01:33:20eric.smithsetnosy: + eric.smith

messages: + msg209935
versions: + Python 3.5, - Python 3.4
2014-02-02 01:22:28leonncreate