classification
Title: checkRecvmsgAddress wrong in test_socket.py (AIX failures)
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.3, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: David.Edelsohn, neologix, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2013-06-26 15:37 by David.Edelsohn, last changed 2013-07-09 17:19 by neologix. This issue is now closed.

Files
File name Uploaded Description Edit
scope_id.patch vstinner, 2013-07-03 21:53 review
test_socket_scope_id.diff neologix, 2013-07-06 10:24 review
Messages (9)
msg191908 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2013-06-26 15:37
The recvmsg tests in test_socket.py check that the address returned by recvmsg matches the original address to which the socket was bound. For IPv6, sockaddr includes sin6_scope_id, in addition to the address and port.

The test connects to host "::1", which is loopback, but is an under-specified address because the link scope is left ambiguous.

The scope_id in the original bind call defaults to "0", which represents an ambiguous scoped address and allows the IPV6 protocol and implementation to choose the interface or site identifier.  The recvmsg call returns the actual scope_id.

The test incorrectly checks that the full sockaddr matches. sin6_scope_id may not match for IPv6 addresses.  This generates bogus failures on AIX.

(Microsoft has a good description about scope_id:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms739166%28v=vs.85%29.aspx)
msg191920 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2013-06-26 22:18
The current arguments to checkRecvmsgAddress() are the sockaddrs, so it does not know the protocol family. One potential patch to infer the family and apply the correct test is:

diff -r 035d8fed07ad Lib/test/test_socket.py
--- a/Lib/test/test_socket.py   Tue Jun 25 22:54:35 2013 +0200
+++ b/Lib/test/test_socket.py   Wed Jun 26 15:16:31 2013 -0700
@@ -1809,7 +1809,10 @@
     def checkRecvmsgAddress(self, addr1, addr2):
         # Called to compare the received address with the address of
         # the peer.
-        self.assertEqual(addr1, addr2)
+        if len(addr1) > 3 or len(addr2) > 3:
+            self.assertEqual(addr1[:-1], addr2[:-1])
+        else:
+            self.assertEqual(addr1, addr2)
 
     # Flags that are normally unset in msg_flags
     msg_flags_common_unset = 0
msg192259 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-07-03 21:53
"The scope_id in the original bind call defaults to "0", which represents an ambiguous scoped address and allows the IPV6 protocol and implementation to choose the interface or site identifier."

Ok, so here is a patch using scope_id=1 to get a reliable IPv6 address. Can you please try it on AIX?
msg192264 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2013-07-04 02:13
The patch in msg192259 probably will work on the particular AIX system runnnig the buildbot, but I do not believe that the patch is the correct solution for the problem.

scope_id 1 is not necessarily the correct link for IPv6 address ::1. The scope_id values are system dependent and the result could be any value depending on the interfaces available on the particular system running the test. scope_id 1 might not exist on the system, and forcing that value would generate an error.

I think the safest solution is not to compare scope_id when comparing addresses.
msg192405 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-07-06 10:24
> I think the safest solution is not to compare scope_id when comparing
> addresses.

Agreed.

However, it might be simpler to special-case the IPv6 addresses
comparison by overriding it in the IPv6 sendmsg base test.

Could you try the patch attached?
msg192432 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-07-06 12:46
"However, it might be simpler to special-case the IPv6 addresses
comparison by overriding it in the IPv6 sendmsg base test."

Yes, this is why I proposed another approach. (But David convinced me
that it is not the right approach.)

Victor
msg192467 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2013-07-06 17:05
The patch in msg192405 works and fixes that error on AIX. That is exactly what I had in mind, but I incorrectly had been looking higher up the class hierarchy to override the method. Thanks!
msg192767 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-09 17:17
New changeset 330c7aa2922b by Charles-François Natali in branch '3.3':
Issue #18308: don't take the scope ID into account when comparing IPv6
http://hg.python.org/cpython/rev/330c7aa2922b

New changeset b44749cee660 by Charles-François Natali in branch 'default':
Issue #18308: don't take the scope ID into account when comparing IPv6
http://hg.python.org/cpython/rev/b44749cee660
msg192768 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-07-09 17:19
Fixed, thanks!
History
Date User Action Args
2013-07-09 17:19:46neologixsetstatus: open -> closed
resolution: fixed
messages: + msg192768

stage: resolved
2013-07-09 17:17:28python-devsetnosy: + python-dev
messages: + msg192767
2013-07-06 17:05:22David.Edelsohnsetmessages: + msg192467
2013-07-06 12:46:07vstinnersetmessages: + msg192432
2013-07-06 10:24:24neologixsetfiles: + test_socket_scope_id.diff

messages: + msg192405
2013-07-04 02:13:42David.Edelsohnsetmessages: + msg192264
2013-07-03 21:53:23vstinnersetfiles: + scope_id.patch

nosy: + vstinner
messages: + msg192259

keywords: + patch
2013-06-28 09:32:25pitrousetnosy: + neologix
2013-06-26 22:18:44David.Edelsohnsetmessages: + msg191920
2013-06-26 15:37:10David.Edelsohncreate