From 5252541fe340628300a2c934a8956291d890b76a Mon Sep 17 00:00:00 2001
From: Jimmy <jimmy@spalge.com>
Date: Sun, 9 Sep 2018 14:27:37 +1200
Subject: [PATCH] greasemonkey: better handle scripts without metadata

Previously calling `script.code()` would fail if the script didn't have
a `name`. This wasn't being hit in practice because the only place that
constructs GreasemonkeyScripts was checking for that condition and add
the filename there as a fallback.

This change make the `name` attribute more explicitly mandatory by
failing with a `ValueError` if it is not provided and make it still
possible to use the filename fallback in that case by adding a
`filename` keyward argument to `__init__()`.

Additionally where `script_meta` is used in `script.code()` a fallback
to and emptry string was added so it doesn't fail for raw javascript
files without greasemonkey metadata.
---
 qutebrowser/browser/greasemonkey.py        | 28 ++++++++++++++++------
 tests/unit/javascript/test_greasemonkey.py | 15 ++++++++++++
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/qutebrowser/browser/greasemonkey.py b/qutebrowser/browser/greasemonkey.py
index 7e5ff7f5b..7606e6d64 100644
--- a/qutebrowser/browser/greasemonkey.py
+++ b/qutebrowser/browser/greasemonkey.py
@@ -46,7 +46,8 @@ class GreasemonkeyScript:
 
     """Container class for userscripts, parses metadata blocks."""
 
-    def __init__(self, properties, code):
+    def __init__(self, properties, code,  # noqa: C901 pragma: no mccabe
+                 filename=None):
         self._code = code
         self.includes = []
         self.matches = []
@@ -81,11 +82,19 @@ class GreasemonkeyScript:
             elif name == 'qute-js-world':
                 self.jsworld = value
 
+        if not self.name:
+            if filename:
+                self.name = filename
+            else:
+                raise ValueError(
+                    "@name key required or pass filename to init."
+                )
+
     HEADER_REGEX = r'// ==UserScript==|\n+// ==/UserScript==\n'
     PROPS_REGEX = r'// @(?P<prop>[^\s]+)\s*(?P<val>.*)'
 
     @classmethod
-    def parse(cls, source):
+    def parse(cls, source, filename=None):
         """GreasemonkeyScript factory.
 
         Takes a userscript source and returns a GreasemonkeyScript.
@@ -97,7 +106,11 @@ class GreasemonkeyScript:
             _head, props, _code = matches
         except ValueError:
             props = ""
-        script = cls(re.findall(cls.PROPS_REGEX, props), source)
+        script = cls(
+            re.findall(cls.PROPS_REGEX, props),
+            source,
+            filename=filename
+        )
         script.script_meta = props
         if not script.includes and not script.matches:
             script.includes = ['*']
@@ -121,7 +134,7 @@ class GreasemonkeyScript:
             scriptName=javascript.string_escape(
                 "/".join([self.namespace or '', self.name])),
             scriptInfo=self._meta_json(),
-            scriptMeta=javascript.string_escape(self.script_meta),
+            scriptMeta=javascript.string_escape(self.script_meta or ''),
             scriptSource=self._code,
             use_proxy=use_proxy)
 
@@ -235,9 +248,10 @@ class GreasemonkeyManager(QObject):
                 continue
             script_path = os.path.join(scripts_dir, script_filename)
             with open(script_path, encoding='utf-8') as script_file:
-                script = GreasemonkeyScript.parse(script_file.read())
-                if not script.name:
-                    script.name = script_filename
+                script = GreasemonkeyScript.parse(
+                    script_file.read(),
+                    filename=script_filename,
+                )
                 self.add_script(script, force)
         self.scripts_reloaded.emit()
 
diff --git a/tests/unit/javascript/test_greasemonkey.py b/tests/unit/javascript/test_greasemonkey.py
index 15fa4321e..e85edd15b 100644
--- a/tests/unit/javascript/test_greasemonkey.py
+++ b/tests/unit/javascript/test_greasemonkey.py
@@ -113,6 +113,21 @@ def test_no_metadata(caplog):
     assert len(scripts.end) == 1
 
 
+def test_no_name():
+    """Ensure that GreaseMonkeyScripts must have a name."""
+    msg = "@name key required or pass filename to init."
+    with pytest.raises(ValueError, match=msg):
+        greasemonkey.GreasemonkeyScript([("something", "else")], "")
+
+
+def test_no_name_with_fallback():
+    """Ensure that script's name can fallback to the provided filename."""
+    script = greasemonkey.GreasemonkeyScript(
+        [("something", "else")], "", filename=r"C:\COM1")
+    assert script
+    assert script.name == r"C:\COM1"
+
+
 def test_bad_scheme(caplog):
     """qute:// isn't in the list of allowed schemes."""
     _save_script("var nothing = true;\n", 'nothing.user.js')