test_socket fails if /proc/modules is existent but not readable

[patrila]

BPO	28815
Nosy	@vadmium, @serhiy-storchaka
Files	test_socket_isTipcAvailable_proc_modules.patch: Patch for test_socket.isTipcAvailable() [obsolete] test_socket_isTipcAvailable_proc_modules.patch: Patch for test_socket.isTipcAvailable()

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-01-08.05:24:27.101>
created_at = <Date 2016-11-27.16:16:18.912>
labels = ['3.7', 'type-bug', 'tests']
title = 'test_socket fails if /proc/modules is existent but not readable'
updated_at = <Date 2017-01-08.05:24:27.100>
user = 'https://bugs.python.org/patrila'

bugs.python.org fields:

activity = <Date 2017-01-08.05:24:27.100>
actor = 'martin.panter'
assignee = 'none'
closed = True
closed_date = <Date 2017-01-08.05:24:27.101>
closer = 'martin.panter'
components = ['Tests']
creation = <Date 2016-11-27.16:16:18.912>
creator = 'patrila'
dependencies = []
files = ['45666', '45834']
hgrepos = []
issue_num = 28815
keywords = ['patch']
message_count = 8.0
messages = ['281828', '282126', '282849', '283943', '283946', '283951', '284379', '284953']
nosy_count = 4.0
nosy_names = ['python-dev', 'martin.panter', 'serhiy.storchaka', 'patrila']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28815'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']

[patrila]

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(),

[vadmium]

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.

[patrila]

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).

[python-dev]

New changeset 7889d7a771c7 by Martin Panter in branch '3.5':
Issue bpo-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 bpo-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 bpo-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 bpo-28815: Skip TIPC tests if /proc/modules is not readable
https://hg.python.org/cpython/rev/41a99a2a7198

[serhiy-storchaka]

Wouldn't be more pythonic to catch (FileNotFoundError, IsADirectoryError, PermissionError) instead of checking errno?

[vadmium]

That would be possible in Python 3, not Python 2 though.

[patrila]

@martin Panter: Thanks for your support and merging.

[python-dev]

New changeset 5a417e160b34 by Martin Panter in branch '3.5':
Issue bpo-28815: Use new exception subclasses
https://hg.python.org/cpython/rev/5a417e160b34

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

New changeset 82fb37281954 by Martin Panter in branch 'default':
Issue bpo-28815: Merge test tweak from 3.6
https://hg.python.org/cpython/rev/82fb37281954