Merge remote-tracking branch 'origin/pr/4133'

qutebrowser/browser/commands.py

@@ -2109,7 +2109,10 @@ class CommandDispatcher:
                 raise cmdexc.CommandError(str(e))
 
         widget = self._current_widget()
-        widget.run_js_async(js_code, callback=jseval_cb, world=world)
+        try:
+            widget.run_js_async(js_code, callback=jseval_cb, world=world)
+        except browsertab.WebTabError as e:
+            raise cmdexc.CommandError(str(e))
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
     def fake_key(self, keystring, global_=False):

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,7 +248,8 @@ class GreasemonkeyManager(QObject):
                 continue
             script_path = os.path.join(scripts_dir, script_filename)
             with open(script_path, encoding='utf-8-sig') as script_file:
-                script = GreasemonkeyScript.parse(script_file.read())
+                script = GreasemonkeyScript.parse(script_file.read(),
+                                                  script_filename)
                 if not script.name:
                     script.name = script_filename
                 self.add_script(script, force)

qutebrowser/browser/webengine/webenginetab.py

@@ -950,6 +950,15 @@ class _WebEngineScripts(QObject):
         scripts = self._greasemonkey.all_scripts()
         self._inject_greasemonkey_scripts(scripts)
 
+    def _remove_all_greasemonkey_scripts(self):
+        page_scripts = self._widget.page().scripts()
+        for script in page_scripts.toList():
+            if script.name().startswith("GM-"):
+                log.greasemonkey.debug('Removing script: {}'
+                                       .format(script.name()))
+                removed = page_scripts.remove(script)
+                assert removed, script.name()
+
     def _inject_greasemonkey_scripts(self, scripts=None, injection_point=None,
                                      remove_first=True):
         """Register user JavaScript files with the current tab.
@@ -973,12 +982,7 @@ class _WebEngineScripts(QObject):
         # have been added elsewhere, like the one for stylesheets.
         page_scripts = self._widget.page().scripts()
         if remove_first:
-            for script in page_scripts.toList():
+            self._remove_all_greasemonkey_scripts()
-                if script.name().startswith("GM-"):
-                    log.greasemonkey.debug('Removing script: {}'
-                                           .format(script.name()))
-                    removed = page_scripts.remove(script)
-                    assert removed, script.name()
 
         if not scripts:
             return
@@ -987,6 +991,15 @@ class _WebEngineScripts(QObject):
             new_script = QWebEngineScript()
             try:
                 world = int(script.jsworld)
+                if not 0 <= world <= qtutils.MAX_WORLD_ID:
+                    log.greasemonkey.error(
+                        "script {} has invalid value for '@qute-js-world'"
+                        ": {}, should be between 0 and {}"
+                        .format(
+                            script.name,
+                            script.jsworld,
+                            qtutils.MAX_WORLD_ID))
+                    continue
             except ValueError:
                 try:
                     world = _JS_WORLD_MAP[usertypes.JsWorld[
@@ -1104,6 +1117,10 @@ class WebEngineTab(browsertab.AbstractTab):
             world_id = QWebEngineScript.ApplicationWorld
         elif isinstance(world, int):
             world_id = world
+            if not 0 <= world_id <= qtutils.MAX_WORLD_ID:
+                raise browsertab.WebTabError(
+                    "World ID should be between 0 and {}"
+                    .format(qtutils.MAX_WORLD_ID))
         else:
             world_id = _JS_WORLD_MAP[world]
 

qutebrowser/utils/qtutils.py

@@ -24,6 +24,7 @@ Module attributes:
              value.
     MINVALS: A dictionary of C/Qt types (as string) mapped to their minimum
              value.
+    MAX_WORLD_ID: The highest world ID allowed in this version of QtWebEngine.
 """
 
 
@@ -98,6 +99,10 @@ def version_check(version, exact=False, compiled=True):
     return result
 
 
+# WORKAROUND for https://bugreports.qt.io/browse/QTBUG-69904
+MAX_WORLD_ID = 256 if version_check('5.11.2') else 11
+
+
 def is_new_qtwebkit():
     """Check if the given version is a new QtWebKit."""
     assert qWebKitVersion is not None

tests/end2end/features/misc.feature

@@ -118,6 +118,26 @@ Feature: Various utility commands.
         Then the javascript message "Hello from the page!" should be logged
         And "No output or error" should be logged
 
+    @qtwebkit_skip @qt>=5.11.2
+    Scenario: :jseval using too high of a world id in Qt versions bigger than 5.11.2
+        When I run :jseval --world=257 console.log("Hello from JS!");
+        Then the error "World ID should be between 0 and 256" should be shown
+
+    @qtwebkit_skip @qt<5.11.2
+    Scenario: :jseval using too high of a world id in Qt versions smaller than 5.11.2
+        When I run :jseval --world=12 console.log("Hello from JS!");
+        Then the error "World ID should be between 0 and 11" should be shown
+
+    @qtwebkit_skip @qt>=5.11.2
+    Scenario: :jseval using a negative world id in Qt versions bigger than 5.11.2
+        When I run :jseval --world=-1 console.log("Hello from JS!");
+        Then the error "World ID should be between 0 and 256" should be shown
+
+    @qtwebkit_skip @qt<5.11.2
+    Scenario: :jseval using a negative world id in Qt versions smaller than 5.11.2
+        When I run :jseval --world=-1 console.log("Hello from JS!");
+        Then the error "World ID should be between 0 and 11" should be shown
+
     Scenario: :jseval --file using a file that exists as js-code
         When I run :jseval --file (testdata)/misc/jseval_file.js
         Then the javascript message "Hello from JS!" should be logged

tests/unit/browser/webengine/test_webenginetab.py

@@ -0,0 +1,107 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2018 Florian Bruhin (The Compiler) <mail@qutebrowser.org>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test webenginetab."""
+
+from unittest import mock
+import logging
+
+from PyQt5.QtCore import QObject
+import pytest
+QtWebEngineWidgets = pytest.importorskip("PyQt5.QtWebEngineWidgets")
+QWebEnginePage = QtWebEngineWidgets.QWebEnginePage
+QWebEngineScriptCollection = QtWebEngineWidgets.QWebEngineScriptCollection
+
+from qutebrowser.browser.webengine import webenginetab
+from qutebrowser.browser import greasemonkey
+
+pytestmark = pytest.mark.usefixtures('greasemonkey_manager')
+
+
+class TestWebengineScripts:
+    """Test the _WebEngineScripts utility class."""
+
+    class FakeWidget(QObject):
+        """Fake widget for _WebengineScripts to use."""
+
+        def __init__(self, *args, **kwargs):
+            super().__init__(*args, **kwargs)
+            self.scripts = []
+            self.page = mock.create_autospec(QWebEnginePage)
+            self.scripts_mock = mock.create_autospec(
+                QWebEngineScriptCollection
+            )
+            self.scripts_mock.toList.return_value = self.scripts
+            self.page.return_value.scripts.return_value = self.scripts_mock
+
+    @pytest.fixture
+    def mocked_scripts(self, fake_web_tab):
+        scripts = webenginetab._WebEngineScripts(fake_web_tab)
+        scripts._widget = self.FakeWidget()
+        return scripts
+
+    def test_greasemonkey_undefined_world(self, mocked_scripts, caplog):
+        """Make sure scripts with non-existent worlds are rejected."""
+        scripts = [
+            greasemonkey.GreasemonkeyScript(
+                [('qute-js-world', 'Mars'), ('name', 'test')], None)
+        ]
+
+        with caplog.at_level(logging.ERROR, 'greasemonkey'):
+            mocked_scripts._inject_greasemonkey_scripts(scripts)
+
+        assert len(caplog.records) == 1
+        msg = caplog.records[0].message
+        assert "has invalid value for '@qute-js-world': Mars" in msg
+        mocked_scripts._widget.scripts_mock.insert.assert_not_called()
+
+    @pytest.mark.parametrize("worldid", [-1, 257])
+    def test_greasemonkey_out_of_range_world(self, worldid,
+                                             mocked_scripts, caplog):
+        """Make sure scripts with out-of-range worlds are rejected."""
+        scripts = [
+            greasemonkey.GreasemonkeyScript(
+                [('qute-js-world', worldid), ('name', 'test')], None)
+        ]
+
+        with caplog.at_level(logging.ERROR, 'greasemonkey'):
+            mocked_scripts._inject_greasemonkey_scripts(scripts)
+
+        assert len(caplog.records) == 1
+        msg = caplog.records[0].message
+        assert "has invalid value for '@qute-js-world': " in msg
+        assert "should be between 0 and" in msg
+        mocked_scripts._widget.scripts_mock.insert.assert_not_called()
+
+    @pytest.mark.parametrize("worldid", [0, 10])
+    def test_greasemonkey_good_worlds_are_passed(self, worldid,
+                                                 mocked_scripts, caplog):
+        """Make sure scripts with valid worlds have it set."""
+        scripts = [
+            greasemonkey.GreasemonkeyScript(
+                [('name', 'foo'), ('qute-js-world', worldid)], None
+            )
+        ]
+
+        with caplog.at_level(logging.ERROR, 'greasemonkey'):
+            mocked_scripts._inject_greasemonkey_scripts(scripts)
+
+        calls = mocked_scripts._widget.scripts_mock.insert.call_args_list
+        assert len(calls) == 1
+        assert calls[0][0][0].worldId() == worldid

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