ipc: Add username to hash instead.

qutebrowser/misc/ipc.py

@@ -40,19 +40,26 @@ READ_TIMEOUT = 5000
 PROTOCOL_VERSION = 1
 
 
+def _get_socketname_legacy(basedir):
+    """Legacy implementation of _get_socketname."""
+    parts = ['qutebrowser', getpass.getuser()]
+    if basedir is not None:
+        md5 = hashlib.md5(basedir.encode('utf-8')).hexdigest()
+        parts.append(md5)
+    return '-'.join(parts)
+
+
 def _get_socketname(basedir, legacy=False):
     """Get a socketname to use."""
-    if basedir is None:
-        basedir_md5 = None
-    else:
-        md5 = hashlib.md5(basedir.encode('utf-8'))
-        basedir_md5 = md5.hexdigest()
-
     if legacy or os.name == 'nt':
-        parts = ['qutebrowser', getpass.getuser()]
+        return _get_socketname_legacy(basedir)
-        if basedir_md5 is not None:
+
-            parts.append(basedir_md5)
+    parts_to_hash = [getpass.getuser()]
-        return '-'.join(parts)
+    if basedir is not None:
+        parts_to_hash.append(basedir)
+
+    data_to_hash = '-'.join(parts_to_hash).encode('utf-8')
+    md5 = hashlib.md5(data_to_hash).hexdigest()
 
     if sys.platform.startswith('linux'):
         target_dir = standarddir.runtime()
@@ -61,8 +68,7 @@ def _get_socketname(basedir, legacy=False):
         target_dir = standarddir.temp()
 
     parts = ['ipc']
-    if basedir_md5 is not None:
+    parts.append(md5)
-        parts.append(basedir_md5)
     return os.path.join(target_dir, '-'.join(parts))
 
 

qutebrowser/utils/standarddir.py

@@ -21,7 +21,6 @@
 
 import os
 import os.path
-import getpass
 
 from PyQt5.QtCore import QCoreApplication, QStandardPaths
 
@@ -151,7 +150,10 @@ def _get(typ):
         elif typ == QStandardPaths.TempLocation:
             # "The returned value might be application-specific, shared among
             # other applications for this user, or even system-wide."
-            subdir = '{}-{}'.format(appname, getpass.getuser())
+            #
+            # Unfortunately this path could get too long for IPC sockets, so we
+            # don't add the username here...
+            subdir = appname
             path = os.path.join(path, subdir)
         elif typ == QStandardPaths.RuntimeLocation:
             # This is generic, but per-user.

tests/unit/misc/test_ipc.py

@@ -25,6 +25,7 @@ import getpass
 import collections
 import logging
 import json
+import hashlib
 from unittest import mock
 
 import pytest
@@ -167,20 +168,27 @@ def test_getpass_getuser():
     assert getpass.getuser()
 
 
+def md5(inp):
+    return hashlib.md5(inp.encode('utf-8')).hexdigest()
+
+
 class TestSocketName:
 
-    MD5 = 'cc8755609ad61864910f145119713de9'  # The MD5 of 'x'
-
     LEGACY_TESTS = [
-        (None, 'qutebrowser-{}'.format(getpass.getuser())),
+        (None, 'qutebrowser-testusername'),
-        ('/x', 'qutebrowser-{}-{}'.format(getpass.getuser(), MD5)),
+        ('/x', 'qutebrowser-testusername-{}'.format(md5('/x'))),
     ]
 
     POSIX_TESTS = [
-        (None, 'ipc'),
+        (None, 'ipc-{}'.format(md5('testusername'))),
-        ('/x', 'ipc-{}'.format(MD5)),
+        ('/x', 'ipc-{}'.format(md5('testusername-/x'))),
     ]
 
+    @pytest.fixture(autouse=True)
+    def patch_user(self, monkeypatch):
+        monkeypatch.setattr('qutebrowser.misc.ipc.getpass.getuser',
+                            lambda: 'testusername')
+
     @pytest.mark.parametrize('basedir, expected', LEGACY_TESTS)
     def test_legacy(self, basedir, expected):
         socketname = ipc._get_socketname(basedir, legacy=True)
@@ -744,11 +752,13 @@ class TestSendOrListen:
 def test_long_username(monkeypatch):
     """See https://github.com/The-Compiler/qutebrowser/issues/888."""
     username = 'alexandercogneau'
+    basedir = '/this_is_a_long_basedir'
     monkeypatch.setattr('qutebrowser.misc.ipc.standarddir.getpass.getuser',
                         lambda: username)
-    name = ipc._get_socketname(basedir='/foo')
+    name = ipc._get_socketname(basedir=basedir)
     server = ipc.IPCServer(name)
-    assert username in server._socketname
+    expected_md5 = md5('{}-{}'.format(username, basedir))
+    assert expected_md5 in server._socketname
     try:
         server.listen()
     finally:

tests/unit/utils/test_standarddir.py

@@ -58,12 +58,6 @@ def reset_standarddir():
     standarddir.init(None)
 
 
-@pytest.fixture(autouse=True)
-def patch_username(monkeypatch):
-    monkeypatch.setattr('qutebrowser.utils.standarddir.getpass.getuser',
-                        lambda: 'user')
-
-
 @pytest.mark.parametrize('data_subdir, config_subdir, expected', [
     ('foo', 'foo', 'foo/data'),
     ('foo', 'bar', 'foo'),
@@ -127,7 +121,7 @@ class TestGetStandardDirLinux:
     def test_temp_explicit(self, monkeypatch, tmpdir):
         """Test temp dir with TMPDIR explicitly set."""
         monkeypatch.setenv('TMPDIR', str(tmpdir))
-        assert standarddir.temp() == str(tmpdir / 'qute_test-user')
+        assert standarddir.temp() == str(tmpdir / 'qute_test')
 
     def test_data(self, monkeypatch, tmpdir):
         """Test data dir with XDG_DATA_HOME not set."""
@@ -153,7 +147,7 @@ class TestGetStandardDirLinux:
     def test_temp(self, monkeypatch, tmpdir):
         """Test temp dir with TMPDIR not set."""
         monkeypatch.delenv('TMPDIR', raising=False)
-        assert standarddir.temp().split(os.sep)[-1] == 'qute_test-user'
+        assert standarddir.temp().split(os.sep)[-1] == 'qute_test'
 
 
 @pytest.mark.windows
@@ -177,7 +171,7 @@ class TestGetStandardDirWindows:
         assert standarddir.cache().split(os.sep)[-2:] == expected
 
     def test_temp(self):
-        assert standarddir.temp().split(os.sep)[-1] == 'qute_test-user'
+        assert standarddir.temp().split(os.sep)[-1] == 'qute_test'
 
 
 DirArgTest = collections.namedtuple('DirArgTest', 'arg, expected')