From e0bd89b7625eb560059e841d85a45296404bf0aa Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 28 Jul 2014 01:40:25 +0200 Subject: [PATCH] Add an {url} variable for commands. Note this also means we don't support :spawn running in a shell anymore, as {url} is evaluated earlier. However this should be fine, as there's no really important use case for that anyways, and shell escaping on Windows is rather unmaintainable. --- qutebrowser/browser/commands.py | 62 ++++++++++----------------- qutebrowser/commands/command.py | 16 +++++-- qutebrowser/test/utils/test_misc.py | 63 ---------------------------- qutebrowser/utils/misc.py | 23 ---------- qutebrowser/widgets/tabbedbrowser.py | 22 ++++++++++ 5 files changed, 56 insertions(+), 130 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index e0d8690f3..e69f39e43 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -37,7 +37,6 @@ import qutebrowser.utils.webelem as webelem import qutebrowser.browser.quickmarks as quickmarks import qutebrowser.utils.log as log import qutebrowser.utils.url as urlutils -from qutebrowser.utils.misc import shell_escape from qutebrowser.utils.qt import (check_overflow, check_print_compat, qt_ensure_valid, QtValueError) from qutebrowser.utils.editor import ExternalEditor @@ -71,19 +70,6 @@ class CommandDispatcher: self._tabs = parent self._editor = None - def _current_url(self): - """Get the URL of the current tab.""" - url = self._tabs.currentWidget().url() - try: - qt_ensure_valid(url) - except QtValueError as e: - msg = "Current URL is invalid" - if e.reason: - msg += " ({})".format(e.reason) - msg += "!" - raise CommandError(msg) - return url - def _scroll_percent(self, perc=None, count=None, orientation=None): """Inner logic for scroll_percent_(x|y). @@ -111,8 +97,8 @@ class CommandDispatcher: frame = widget.page().currentFrame() if frame is None: raise CommandError("No frame focused!") - widget.hintmanager.follow_prevnext(frame, self._current_url(), prev, - newtab) + widget.hintmanager.follow_prevnext(frame, self._tabs.current_url(), + prev, newtab) def _tab_move_absolute(self, idx): """Get an index for moving a tab absolutely. @@ -285,7 +271,8 @@ class CommandDispatcher: target = getattr(hints.Target, targetstr.replace('-', '_')) except AttributeError: raise CommandError("Unknown hinting target {}!".format(targetstr)) - widget.hintmanager.start(frame, self._current_url(), group, target) + widget.hintmanager.start(frame, self._tabs.current_url(), group, + target) @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True) def follow_hint(self): @@ -372,8 +359,8 @@ class CommandDispatcher: sel: True to use primary selection, False to use clipboard """ clipboard = QApplication.clipboard() - urlstr = self._current_url().toString(QUrl.FullyEncoded | - QUrl.RemovePassword) + urlstr = self._tabs.current_url().toString( + QUrl.FullyEncoded | QUrl.RemovePassword) if sel and clipboard.supportsSelection(): mode = QClipboard.Selection target = "primary selection" @@ -467,19 +454,19 @@ class CommandDispatcher: @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True) def open_tab_cur(self): """Set the statusbar to :tabopen and the current URL.""" - urlstr = self._current_url().toDisplayString(QUrl.FullyEncoded) + urlstr = self._tabs.current_url().toDisplayString(QUrl.FullyEncoded) message.set_cmd_text(':open-tab ' + urlstr) @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True) def open_cur(self): """Set the statusbar to :open and the current URL.""" - urlstr = self._current_url().toDisplayString(QUrl.FullyEncoded) + urlstr = self._tabs.current_url().toDisplayString(QUrl.FullyEncoded) message.set_cmd_text(':open ' + urlstr) @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True) def open_tab_bg_cur(self): """Set the statusbar to :tabopen-bg and the current URL.""" - urlstr = self._current_url().toDisplayString(QUrl.FullyEncoded) + urlstr = self._tabs.current_url().toDisplayString(QUrl.FullyEncoded) message.set_cmd_text(':open-tab-bg ' + urlstr) @cmdutils.register(instance='mainwindow.tabs.cmd') @@ -610,29 +597,22 @@ class CommandDispatcher: self._tabs.setCurrentIndex(idx) @cmdutils.register(instance='mainwindow.tabs.cmd', split=False) - def spawn(self, cmd): - """Spawn a command in a shell. {} gets replaced by the current URL. + def spawn(self, *args): + """Spawn a command in a shell. - The URL will already be quoted correctly, so there's no need to do - that. - - The command will be run in a shell, so you can use shell features like - redirections. + Note the {url} variable which gets replaced by the current URL might be + useful here. // - We use subprocess rather than Qt's QProcess here because of it's - shell=True argument and because we really don't care about the process - anymore as soon as it's spawned. + We use subprocess rather than Qt's QProcess here because we really + don't care about the process anymore as soon as it's spawned. Args: - cmd: The command to execute. + *args: The commandline to execute """ - urlstr = self._current_url().toString(QUrl.FullyEncoded | - QUrl.RemovePassword) - cmd = cmd.replace('{}', shell_escape(urlstr)) - log.procs.debug("Executing: {}".format(cmd)) - subprocess.Popen(cmd, shell=True) + log.procs.debug("Executing: {}".format(args)) + subprocess.Popen(args) @cmdutils.register(instance='mainwindow.tabs.cmd') def home(self): @@ -644,7 +624,7 @@ class CommandDispatcher: """Run an userscript given as argument.""" # We don't remove the password in the URL here, as it's probably safe # to pass via env variable. - urlstr = self._current_url().toString(QUrl.FullyEncoded) + urlstr = self._tabs.current_url().toString(QUrl.FullyEncoded) runner = UserscriptRunner(self._tabs) runner.got_cmd.connect(self._tabs.got_cmd) runner.run(cmd, *args, env={'QUTE_URL': urlstr}) @@ -653,7 +633,7 @@ class CommandDispatcher: @cmdutils.register(instance='mainwindow.tabs.cmd') def quickmark_save(self): """Save the current page as a quickmark.""" - quickmarks.prompt_save(self._current_url()) + quickmarks.prompt_save(self._tabs.current_url()) @cmdutils.register(instance='mainwindow.tabs.cmd') def quickmark_load(self, name): @@ -701,7 +681,7 @@ class CommandDispatcher: def download_page(self): """Download the current page.""" page = self._tabs.currentWidget().page() - self._tabs.download_get.emit(self._current_url(), page) + self._tabs.download_get.emit(self._tabs.current_url(), page) @cmdutils.register(instance='mainwindow.tabs.cmd', modes=['insert'], hide=True) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index 6310b95a6..928a99704 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -19,7 +19,7 @@ """Contains the Command class, a skeleton for a command.""" -from PyQt5.QtCore import QCoreApplication +from PyQt5.QtCore import QCoreApplication, QUrl from PyQt5.QtWebKit import QWebSettings from qutebrowser.commands.exceptions import (ArgumentCountError, @@ -127,15 +127,25 @@ class Command: kwargs = {} app = QCoreApplication.instance() + # Replace variables (currently only {url}) + new_args = [] + for arg in args: + if arg == '{url}': + urlstr = app.mainwindow.tabs.current_url().toString( + QUrl.FullyEncoded | QUrl.RemovePassword) + new_args.append(urlstr) + else: + new_args.append(arg) + if self.instance is not None: # Add the 'self' parameter. if self.instance == '': obj = app else: obj = dotted_getattr(app, self.instance) - args.insert(0, obj) + new_args.insert(0, obj) if count is not None and self.count: kwargs = {'count': count} - self.handler(*args, **kwargs) + self.handler(*new_args, **kwargs) diff --git a/qutebrowser/test/utils/test_misc.py b/qutebrowser/test/utils/test_misc.py index 799459e4a..2d3ee8d93 100644 --- a/qutebrowser/test/utils/test_misc.py +++ b/qutebrowser/test/utils/test_misc.py @@ -155,69 +155,6 @@ class SafeShlexSplitTests(unittest.TestCase): self.assertEqual(items, ['one', 'two\\']) -class ShellEscapeTests(unittest.TestCase): - - """Tests for shell_escape. - - Class attributes: - TEXTS_LINUX: A list of (input, output) of expected texts for Linux. - TEXTS_WINDOWS: A list of (input, output) of expected texts for Windows. - - Attributes: - platform: The saved sys.platform value. - """ - - TEXTS_LINUX = ( - ('', "''"), - ('foo%bar+baz', 'foo%bar+baz'), - ('foo$bar', "'foo$bar'"), - ("$'b", """'$'"'"'b'"""), - ) - - TEXTS_WINDOWS = ( - ('', '""'), - ('foo*bar?baz', 'foo*bar?baz'), - ("a&b|c^df%", "a^&b^|c^^d^f^%"), - ('foo"bar', 'foo"""bar'), - ) - - def setUp(self): - self.platform = sys.platform - - def test_fake_linux(self): - """Fake test which simply checks if the escaped string looks right.""" - sys.platform = 'linux' - for (orig, escaped) in self.TEXTS_LINUX: - self.assertEqual(utils.shell_escape(orig), escaped) - - def test_fake_windows(self): - """Fake test which simply checks if the escaped string looks right.""" - sys.platform = 'win32' - for (orig, escaped) in self.TEXTS_WINDOWS: - self.assertEqual(utils.shell_escape(orig), escaped) - - @unittest.skipUnless(sys.platform.startswith("linux"), "requires Linux") - def test_real_linux(self): - """Real test which prints an escaped string via python.""" - for (orig, _escaped) in self.TEXTS_LINUX: - cmd = ("python -c 'import sys; print(sys.argv[1], end=\"\")' " - "{}".format(utils.shell_escape(orig))) - out = subprocess.check_output(cmd, shell=True).decode('ASCII') - self.assertEqual(out, orig, cmd) - - @unittest.skipUnless(sys.platform.startswith("win"), "requires Windows") - def test_real_windows(self): - """Real test which prints an escaped string via python.""" - for (orig, _escaped) in self.TEXTS_WINDOWS: - cmd = ('python -c "import sys; print(sys.argv[1], end=\'\')" ' - '{}'.format(utils.shell_escape(orig))) - out = subprocess.check_output(cmd, shell=True).decode('ASCII') - self.assertEqual(out, orig, cmd) - - def tearDown(self): - sys.platform = self.platform - - class GetStandardDirLinuxTests(unittest.TestCase): """Tests for get_standard_dir under Linux. diff --git a/qutebrowser/utils/misc.py b/qutebrowser/utils/misc.py index 735e78bc7..27b19fade 100644 --- a/qutebrowser/utils/misc.py +++ b/qutebrowser/utils/misc.py @@ -127,29 +127,6 @@ def safe_shlex_split(s): raise -def shell_escape(s): - """Escape a string so it's safe to pass to a shell.""" - if sys.platform.startswith('win'): - # Oh dear flying sphagetti monster please kill me now... - if not s: - # Is this an empty argument or a literal ""? It seems to depend on - # something magical. - return '""' - # We could also use \", but what do we do for a literal \" then? It - # seems \\\". But \\ anywhere else is a literal \\. Because that makes - # sense. Totally NOT. Using """ also seems to yield " and work in some - # kind-of-safe manner. - s = s.replace('"', '"""') - # Some places suggest we use %% to escape %, but actually ^% seems to - # work better (compare echo %%DATE%% and echo ^%DATE^%) - s = re.sub(r'[&|^><%]', r'^\g<0>', s) - # Is everything escaped now? Maybe. I don't know. I don't *get* the - # black magic Microshit is doing here. - return s - else: - return shlex.quote(s) - - def pastebin(text): """Paste the text into a pastebin and return the URL.""" api_url = 'http://paste.the-compiler.org/api/' diff --git a/qutebrowser/widgets/tabbedbrowser.py b/qutebrowser/widgets/tabbedbrowser.py index 16b62305f..671b1c58b 100644 --- a/qutebrowser/widgets/tabbedbrowser.py +++ b/qutebrowser/widgets/tabbedbrowser.py @@ -207,6 +207,28 @@ class TabbedBrowser(TabWidget): else: return None + def current_url(self): + """Get the URL of the current tab. + + Intended to be used from command handlers. + + Return: + The current URL as QUrl. + + Raise: + CommandError if the current URL is invalid. + """ + url = self.currentWidget().url() + try: + qt_ensure_valid(url) + except QtValueError as e: + msg = "Current URL is invalid" + if e.reason: + msg += " ({})".format(e.reason) + msg += "!" + raise CommandError(msg) + return url + def shutdown(self): """Try to shut down all tabs cleanly.