Issue658327
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2002-12-24 21:00 by exarkun, last changed 2022-04-10 16:06 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
inet_pton.diff | exarkun, 2002-12-24 21:05 | |||
inet_pton.diff | exarkun, 2002-12-26 18:32 | |||
doctestnews.patch | exarkun, 2002-12-31 16:54 | Documentation, unit test, and Misc/NEWS update | ||
inet_pton.diff | nnorwitz, 2003-01-11 03:49 | updated inet_pton.diff - nn 1 | ||
ntop.diff | nnorwitz, 2003-02-05 02:40 | has_ipv6 nn patch 1 (hopefully complete) | ||
pton.diff | exarkun, 2003-04-19 17:31 | Doc/Code/Tests, changes discussed through 4/19/03 | ||
inet_pton.patch | exarkun, 2003-04-19 18:16 |
Messages (21) | |||
---|---|---|---|
msg42076 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2002-12-24 21:00 | |
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. |
|||
msg42077 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2002-12-24 21:05 | |
Logged In: YES user_id=366566 Sourceforge decided not to attach the file the first time... Here it is. |
|||
msg42078 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2002-12-26 18:10 | |
Logged In: YES user_id=33168 Next time, please use context or unified diff. -c or -u option to cvs diff: cvs diff -c ... |
|||
msg42079 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2002-12-26 18:32 | |
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. |
|||
msg42080 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2002-12-27 10:23 | |
Logged In: YES user_id=21627 What is the rationale for providing this functionality? |
|||
msg42081 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2002-12-27 15:39 | |
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. |
|||
msg42082 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2002-12-31 00:11 | |
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? |
|||
msg42083 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2002-12-31 13:17 | |
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. |
|||
msg42084 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2002-12-31 16:52 | |
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. |
|||
msg42085 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2003-01-11 03:49 | |
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! |
|||
msg42086 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2003-01-11 17:04 | |
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 :) |
|||
msg42087 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2003-02-05 02:40 | |
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. |
|||
msg42088 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2003-03-03 10:15 | |
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. |
|||
msg42089 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2003-03-03 22:25 | |
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. :-( |
|||
msg42090 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2003-03-04 03:41 | |
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 |
|||
msg42091 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2003-03-04 07:05 | |
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. |
|||
msg42092 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2003-04-19 17:28 | |
Logged In: YES user_id=366566 Based on the discussion, I've uploaded a new version of the patch. This one does the following: if HAVE_INET_PTON is defined, has_ipv6 is added to the socket module as True, socket_inet_pton and socket_inet_ntop are defined and added to the module, and use the underlying functions to perform the work, only AF_INET and AF_INET6 are supported and where appropriate, input lengths are checked against in_addr and in6_addr. Otherwise, has_ipv6 is added to the socket module as False and no other changes are made. No checks against IPV6_ENABLED are made, because AF_INET6 is always defined and only the platform functions are used to handle values passed to the functions. The patch also updates libsocket.tex, ACKS, NEWS, the socket.py module docstring, and adds tests for inet_pton, inet_ntop, inet_ntoa, and inet_aton to test_socket. Guido, I'm assigning this to you as per your suggestions on python-dev, and because Neal has told me in private correspondance that he doesn't think he'll have a chance to get to it before 2.3b1 comes out. |
|||
msg42093 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2003-04-19 17:53 | |
Logged In: YES user_id=6380 I reject this patch on account of the fact that it clearly hasn't been tested: there's a call to memset() with 2 instead of 3 arguments. Given this lack of care I'm lowering the priority. Also note that there was a patch to test_heapq.py in the patch set. Poster is free to submit a new patch, but I can't guarantee that it'll make it into 2.3b1. |
|||
msg42094 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2003-04-19 18:14 | |
Logged In: YES user_id=366566 Once again I've screwed up and uploaded the wrong .diff. Here is the correct one, which I just applied to a clean CVS checkout and ran the regression tests for, just to be sure. One test does fail (test_tzset), but also fails on a clean CVS checkout. Additionally, as a correct to my last post, has_ipv6 is set based on the definition of IPV6_ENABLED, not HAVE_INET_PTON. My apologies for your wasted time. |
|||
msg42095 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2003-04-23 19:09 | |
Logged In: YES user_id=6380 Close. The tests however fail on platforms where inet_pton etc. aren't defined. The docs should say that this is not available on all platforms (and in particular not on Windows). |
|||
msg42096 - (view) | Author: Guido van Rossum (gvanrossum) * | Date: 2003-04-25 14:23 | |
Logged In: YES user_id=6380 Checked in; now fixing the fallout for Windows. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-10 16:06:03 | admin | set | github: 37645 |
2002-12-24 21:00:22 | exarkun | create |