diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index a0fc3a67b..a3f301ae5 100644 --- a/qutebrowser/browser/commands.py +++ b/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): diff --git a/qutebrowser/browser/greasemonkey.py b/qutebrowser/browser/greasemonkey.py index d48e6db1f..a2ae73aab 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[^\s]+)\s*(?P.*)' @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) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index b00587bf6..7afbd409e 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/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(): - if script.name().startswith("GM-"): - log.greasemonkey.debug('Removing script: {}' - .format(script.name())) - removed = page_scripts.remove(script) - assert removed, script.name() + self._remove_all_greasemonkey_scripts() 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] diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index 8a42fb073..c634eb95f 100644 --- a/qutebrowser/utils/qtutils.py +++ b/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 diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index 5f0035b8b..6893e143a 100644 --- a/tests/end2end/features/misc.feature +++ b/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 diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py new file mode 100644 index 000000000..1828d9c47 --- /dev/null +++ b/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) +# +# 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 . + +"""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 diff --git a/tests/unit/javascript/test_greasemonkey.py b/tests/unit/javascript/test_greasemonkey.py index 03a914878..f5f3d7972 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')