classification
Title: test_socket fails if /proc/modules is existent but not readable
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: martin.panter, patrila, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-11-27 16:16 by patrila, last changed 2017-01-08 05:24 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
test_socket_isTipcAvailable_proc_modules.patch patrila, 2016-11-27 16:16 Patch for test_socket.isTipcAvailable() [obsolete] review
test_socket_isTipcAvailable_proc_modules.patch patrila, 2016-12-10 12:59 Patch for test_socket.isTipcAvailable() review
Messages (8)
msg281828 - (view) Author: (patrila) Date: 2016-11-27 16:16
Dear Python developers

The test_socket test fails if /proc/modules is existent but not readable by the user (this is for example the case with the grsecurity patchset of the kernel).

The method reading /proc/modules is isTipcAvailable(), which is not a test but a guard for other tests.
It seems reasonable to return False in the case that /proc/modules is not readable (but existent).
The method isTipcAvailable() already returns False if /proc/modules is non existent (but fails to return False if it's not readable).

Attached a proposed test. Feel free to remove the EISDIR in case you feel uncomfortable and want a it be a "real" error.
The patch should be applied to both Python-2.7 and Python-3.x.

Kind regards

Here is the inline version of the patch; it's also attached.

diff -r 876bee0bd0ba Lib/test/test_socket.py
--- a/Lib/test/test_socket.py   Sat Nov 26 14:04:40 2016 -0800
+++ b/Lib/test/test_socket.py   Sun Nov 27 17:00:55 2016 +0100
@@ -4779,12 +4779,21 @@
     """ 
     if not hasattr(socket, "AF_TIPC"):
         return False
-    if not os.path.isfile("/proc/modules"):
-        return False
-    with open("/proc/modules") as f:
-        for line in f:
-            if line.startswith("tipc "):
-                return True
+    try:
+        f = open("/proc/modules")
+    except IOError as e:
+        # It's ok if the file does not exist, is a directory or if we
+        # have not the permission to read it. In any other case it's a
+        # real error, so raise it again.
+        if e.errno in (ENOENT, EISDIR, EACCES):
+            return False
+        else:
+            raise
+    else:
+        with f:
+            for line in f:
+                if line.startswith("tipc "):
+                    return True
     return False

 @unittest.skipUnless(isTipcAvailable(),
msg282126 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-12-01 02:12
This seems reasonable in general.

Did you test this exact patch Patrila? It looks to me like you need to change ENOENT → errno.ENOENT, etc.
msg282849 - (view) Author: (patrila) Date: 2016-12-10 12:59
Oops, sorry that was the draft version of the patch.
Attached the correct version. It was tested against "default" branch (but not against 2.7).
msg283943 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-24 11:38
New changeset 7889d7a771c7 by Martin Panter in branch '3.5':
Issue #28815: Skip TIPC tests if /proc/modules is not readable
https://hg.python.org/cpython/rev/7889d7a771c7

New changeset 48b9d9cdfe3b by Martin Panter in branch '3.6':
Issue #28815: Merge test_socket fix from 3.5
https://hg.python.org/cpython/rev/48b9d9cdfe3b

New changeset 7ceacac48cd2 by Martin Panter in branch 'default':
Issue #28815: Merge test_socket fix from 3.6
https://hg.python.org/cpython/rev/7ceacac48cd2

New changeset 41a99a2a7198 by Martin Panter in branch '2.7':
Issue #28815: Skip TIPC tests if /proc/modules is not readable
https://hg.python.org/cpython/rev/41a99a2a7198
msg283946 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-24 12:23
Wouldn't be more pythonic to catch (FileNotFoundError, IsADirectoryError, PermissionError) instead of checking errno?
msg283951 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-12-24 13:40
That would be possible in Python 3, not Python 2 though.
msg284379 - (view) Author: (patrila) Date: 2016-12-31 08:32
@Martin Panter: Thanks for your support and merging.
msg284953 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-08 02:35
New changeset 5a417e160b34 by Martin Panter in branch '3.5':
Issue #28815: Use new exception subclasses
https://hg.python.org/cpython/rev/5a417e160b34

New changeset 401e70317976 by Martin Panter in branch '3.6':
Issue #28815: Merge test tweak from 3.5
https://hg.python.org/cpython/rev/401e70317976

New changeset 82fb37281954 by Martin Panter in branch 'default':
Issue #28815: Merge test tweak from 3.6
https://hg.python.org/cpython/rev/82fb37281954
History
Date User Action Args
2017-01-08 05:24:27martin.pantersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-01-08 02:35:10python-devsetmessages: + msg284953
2016-12-31 08:32:59patrilasetmessages: + msg284379
2016-12-24 13:40:56martin.pantersetmessages: + msg283951
2016-12-24 12:23:40serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg283946
2016-12-24 11:38:41python-devsetnosy: + python-dev
messages: + msg283943
2016-12-10 12:59:13patrilasetfiles: + test_socket_isTipcAvailable_proc_modules.patch

messages: + msg282849
2016-12-01 02:12:44martin.pantersettype: behavior
components: + Tests
versions: - Python 3.3, Python 3.4
nosy: + martin.panter

messages: + msg282126
stage: patch review
2016-11-27 16:16:18patrilacreate