[Patches] [ python-Patches-658327 ] Add inet_pton and inet_ntop to socket

SourceForge.net noreply@sourceforge.net
Sat, 08 Mar 2003 04:06:27 -0800


Patches item #658327, was opened at 2002-12-24 22:00
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=658327&group_id=5470

Category: Core (C code)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Jp Calderone (kuran)
>Assigned to: Neal Norwitz (nnorwitz)
Summary: Add inet_pton and inet_ntop to socket

Initial Comment:
Patch is against current CVS and adds two socket module
functions, inet_pton and inet_ntop.  Both of these
should be available on all platforms (because of other
dependancies in the code) so I don't think portability
is a problem.  inet_ntop converts a packed IP address
to a human-readable '.' or ':' separated string
representation of the IP.  inet_pton performs the
reverse operation.

(Potential) problems: inet_pton sets errno to ENOSPC,
which may lead to a confusing error message.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2003-03-04 08:05

Message:
Logged In: YES 
user_id=21627

My two suggestions aren't exclusive: If you have the native
inet_pton, you can *always* support IPv6 addresses with
that, regardless of whether --enable-ipv6 was passed to
configure or not.

If that is done, it will be a legitime test failure for
inet_pton not to support IPv6 - after all, the primary
reason to define this function was to support IPv6, so if
the native function fails to do so, there is clearly a bug
in the system.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-03-04 04:41

Message:
Logged In: YES 
user_id=33168

I added the #ifdef, but that doesn't address the testing
problem.  If the platform has inet_pton, but doesn't have
IPv6 ENABLED.  The inet_pton will be exported, but there's
no good way to tell if you can pass an IPv6 address.  The
only way to test if IPv6 is enabled would be to call
inet_pton with AF_INET6, catch a socket.error and check if
the exception message is "unknown address family".  Since
this is really a testing issue, perhaps that's best after all?

Do you agree this should be done?
 * Remove has_ipv6
 * Export inet_pton & inet_ntop only if defined for platform
 * Only try to test inet_pton/ntop if defined for platform
 * Modify the tests to pass a valid IPv6 test, catch
socket.error, if the error message is "unknown address
family", don't test ipv6 any further, if the error message
is different, raise TestFailed, if no exception, test all
IPv6 addresses

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-03-03 23:25

Message:
Logged In: YES 
user_id=33168

As I recall, yes, has_ipv6 is only for tests.  There was no
way to distinguish if python was built with IPv6 support,
since AF_INET6 was always defined.

Your second approach sounds like it will work.  I need to
review the code, though.  I've forgotten how it works. :-(

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2003-03-03 11:15

Message:
Logged In: YES 
user_id=21627

The has_ipv6 test is only there for the tests? In that case,
drop it, and just perform AF_INET6 conversions unconditionally.

OTOH, I think we should not expose the emulated inet_pton:
it doesn't set errno correctly, and offers no advantage over
inet_addr. So wrap the entire code with HAVE_INET_PTON, and
only perform the tests if the function is supported.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-02-05 03:40

Message:
Logged In: YES 
user_id=33168

I was just about to check this in, but then I ran into a
problem.  IPv6 may not be enabled, even if the constant
AF_INET6 exists.  The cleanest way I saw to address this in
the test was to add a has_ipv6 boolean constant to the
socket module.  Martin, do you think this is acceptable?

Attached is a complete patch which should be safe (based on
the discussion below), includes tests and doc changes.

----------------------------------------------------------------------

Comment By: Jp Calderone (kuran)
Date: 2003-01-11 18:04

Message:
Logged In: YES 
user_id=366566

Yea, testing for the proper input length is definitely
something that should be done.  The patch looks good, but
for one thing.  If the specified address family is neither
AF_INET nor AF_INET6, the length won't be tested and the
underlying inet_ntop will be called.  This isn't a problem
now (afaik) because only those two address families are
support, but in a future libc version with more supported
address families, it might open a similar hole to the one
you've fixed.  Perhaps the

+       } else {
+               PyErr_SetString(socket_error, "unknown
address family");
+               return NULL;
+       }

should be moved up from the second if-grouping to follow the
first if-grouping.  Everything else looks good to me. 
Thanks for taking the time to look at this :)



----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-01-11 04:49

Message:
Logged In: YES 
user_id=33168

JP, do you agree with my comment on 2002-12-30 about the
checks?  I have attached an updated patch.  Please review
and verify this is correct.

Thank you for the additional tests.  Feel free to submit
patches with additional tests for any and all modules!

----------------------------------------------------------------------

Comment By: Jp Calderone (kuran)
Date: 2002-12-31 17:52

Message:
Logged In: YES 
user_id=366566

Doc, NEWS, and test_socket patch attached.  I didn't notice
any inet_aton/inet_ntoa tests in the module so I added a
couple for those as well (I excluded a test for
inet_ntoa('255.255.255.255') ;) Also included are a couple
IPv6 tests.  I'm not sure if these are appropriate, since
many systems may still lack the required support for them to
pass.  I'll leave it up to you to decide whether they should
be commented out or removed or whatever.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-31 14:17

Message:
Logged In: YES 
user_id=21627

I agree that such a change should be added. Neal, you have
given this patch more attention than I did - please check it
in when you consider it complete. I just like to point out
that it is missing documentation changes (libsocket.tex), a
NEWS entry, and a test case. kuran, please provide those as
a single patch file.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2002-12-31 01:11

Message:
Logged In: YES 
user_id=33168

ISTM that in socket_inet_ntop() you need to verify the size
of the packed value passed in.  If the user passes an empty
string, inet_ntop() could read beyond the buffer passed in,
potentially causing a core dump.

The checks could be something like this:

  if (af == AF_INET && len != sizeof(struct in_addr))
  else if (af == AF_INET6 && len != sizeof(struct in6_addr))

Do this make sense?

----------------------------------------------------------------------

Comment By: Jp Calderone (kuran)
Date: 2002-12-27 16:39

Message:
Logged In: YES 
user_id=366566

The use case I have for it at the moment is a DNS server
(Twisted.names).  inet_pton allows me to handle IPv6
addresses, so it allows me to support AAAA and A6 records. 
I believe an IPv6 capable socks proxy would find this useful
as well.  Basically, low level network stuff.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-27 11:23

Message:
Logged In: YES 
user_id=21627

What is the rationale for providing this functionality?

----------------------------------------------------------------------

Comment By: Jp Calderone (kuran)
Date: 2002-12-26 19:32

Message:
Logged In: YES 
user_id=366566

Ooops, I made two, and uploaded the wrong one >:O  Sorry. 
Dunno if it's still helpful, but here's the unified diff.


----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2002-12-26 19:10

Message:
Logged In: YES 
user_id=33168

Next time, please use context or unified diff.  -c or -u
option to cvs diff:  cvs diff -c ...

----------------------------------------------------------------------

Comment By: Jp Calderone (kuran)
Date: 2002-12-24 22:05

Message:
Logged In: YES 
user_id=366566

Sourceforge decided not to attach the file the first time...
 Here it is.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=658327&group_id=5470