Greasemonkey: Make sure script names are unique

Fixes #6353

5 files changed, 80 insertions, 1 deletions

diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc
index 46c6c283a..96e7cb66c 100644
--- a/doc/changelog.asciidoc
+++ b/doc/changelog.asciidoc
@@ -32,6 +32,12 @@ Changed
 - The `:greasemonkey-reload` command now shows a list of loaded scripts and has
   a new `--quiet` switch to suppress that message.
 
+Fixed
+~~~~~
+
+- Crash when two Greasemonkey scripts have the same name (usually happening
+  because the same file is in both the data and the config directory).
+
 [[v2.2.3]]
 v2.2.3 (2021-06-01)
 -------------------
diff --git a/qutebrowser/browser/greasemonkey.py b/qutebrowser/browser/greasemonkey.py
index 03db3be0c..7fea5820c 100644
--- a/qutebrowser/browser/greasemonkey.py
+++ b/qutebrowser/browser/greasemonkey.py
@@ -67,6 +67,7 @@ class GreasemonkeyScript:
         self.runs_on_sub_frames = True
         self.jsworld = "main"
         self.name = ''
+        self.dedup_suffix = 1
 
         for name, value in properties:
             if name == 'name':
@@ -104,6 +105,20 @@ class GreasemonkeyScript:
     def __str__(self):
         return self.name
 
+    def full_name(self) -> str:
+        """Get the full name of this script.
+
+        This includes a GM- prefix, its namespace (if any) and deduplication
+        counter suffix, if set.
+        """
+        parts = ['GM-']
+        if self.namespace is not None:
+            parts += [self.namespace, '/']
+        parts.append(self.name)
+        if self.dedup_suffix > 1:
+            parts.append(f"-{self.dedup_suffix}")
+        return ''.join(parts)
+
     @classmethod
     def parse(cls, source, filename=None):
         """GreasemonkeyScript factory.
diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py
index f9b636bde..b2ca42cbe 100644
--- a/qutebrowser/browser/webengine/webenginetab.py
+++ b/qutebrowser/browser/webengine/webenginetab.py
@@ -1112,7 +1112,12 @@ class _WebEngineScripts(QObject):
         page_scripts = self._widget.page().scripts()
         self._remove_all_greasemonkey_scripts()
 
+        seen_names = set()
         for script in scripts:
+            while script.full_name() in seen_names:
+                script.dedup_suffix += 1
+            seen_names.add(script.full_name())
+
             new_script = QWebEngineScript()
 
             try:
@@ -1144,7 +1149,7 @@ class _WebEngineScripts(QObject):
             new_script.setInjectionPoint(QWebEngineScript.DocumentReady)
 
             new_script.setSourceCode(script.code())
-            new_script.setName(f"GM-{script.name}")
+            new_script.setName(script.full_name())
             new_script.setRunsOnSubFrames(script.runs_on_sub_frames)
 
             if script.needs_document_end_workaround():
diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py
index 274e216ba..3d8eec663 100644
--- a/tests/unit/browser/webengine/test_webenginetab.py
+++ b/tests/unit/browser/webengine/test_webenginetab.py
@@ -164,6 +164,45 @@ class TestWebengineScripts:
 
         assert scripts_helper.get_script().injectionPoint() == expected
 
+    @pytest.mark.parametrize('header1, header2, expected_names', [
+        (
+            ["// @namespace ns1", "// @name same"],
+            ["// @namespace ns2", "// @name same"],
+            ['GM-ns1/same', 'GM-ns2/same'],
+        ),
+        (
+            ["// @name same"],
+            ["// @name same"],
+            ['GM-same', 'GM-same-2'],
+        ),
+        (
+            ["// @name same"],
+            ["// @name sam"],
+            ['GM-same', 'GM-sam'],
+        ),
+    ])
+    def test_greasemonkey_duplicate_name(self, scripts_helper,
+                                         header1, header2, expected_names):
+        template = """
+            // ==UserScript==
+            {header}
+            // ==/UserScript==
+        """
+        template = textwrap.dedent(template.lstrip('\n'))
+
+        source1 = template.format(header="\n".join(header1))
+        script1 = greasemonkey.GreasemonkeyScript.parse(source1)
+        source2 = template.format(header="\n".join(header2))
+        script2 = greasemonkey.GreasemonkeyScript.parse(source2)
+        scripts_helper.inject([script1, script2])
+
+        names = [script.name() for script in scripts_helper.get_scripts()]
+        assert names == expected_names
+
+        source3 = textwrap.dedent(template.lstrip('\n')).format(header="// @name other")
+        script3 = greasemonkey.GreasemonkeyScript.parse(source3)
+        scripts_helper.inject([script3])
+
 
 def test_notification_permission_workaround():
     """Make sure the value for QWebEnginePage::Notifications is correct."""
diff --git a/tests/unit/javascript/test_greasemonkey.py b/tests/unit/javascript/test_greasemonkey.py
index 2bfb9ca83..17847816d 100644
--- a/tests/unit/javascript/test_greasemonkey.py
+++ b/tests/unit/javascript/test_greasemonkey.py
@@ -131,6 +131,20 @@ def test_no_name_with_fallback():
     assert script.name == r"C:\COM1"
 
 
+@pytest.mark.parametrize('properties, inc_counter, expected', [
+    ([("name", "gorilla")], False, "GM-gorilla"),
+    ([("namespace", "apes"), ("name", "gorilla")], False, "GM-apes/gorilla"),
+
+    ([("name", "gorilla")], True, "GM-gorilla-2"),
+    ([("namespace", "apes"), ("name", "gorilla")], True, "GM-apes/gorilla-2"),
+])
+def test_full_name(properties, inc_counter, expected):
+    script = greasemonkey.GreasemonkeyScript(properties, code="")
+    if inc_counter:
+        script.dedup_suffix += 1
+    assert script.full_name() == expected
+
+
 def test_bad_scheme(caplog):
     """qute:// isn't in the list of allowed schemes."""
     _save_script("var nothing = true;\n", 'nothing.user.js')