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

@@ -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,
+        widget.hintmanager.follow_prevnext(frame, self._tabs.current_url(),
-                                           newtab)
+                                           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 |
+        urlstr = self._tabs.current_url().toString(
-                                              QUrl.RemovePassword)
+            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):
+    def spawn(self, *args):
-        """Spawn a command in a shell. {} gets replaced by the current URL.
+        """Spawn a command in a shell.
 
-        The URL will already be quoted correctly, so there's no need to do
+        Note the {url} variable which gets replaced by the current URL might be
-        that.
+        useful here.
-
-        The command will be run in a shell, so you can use shell features like
-        redirections.
 
         //
 
-        We use subprocess rather than Qt's QProcess here because of it's
+        We use subprocess rather than Qt's QProcess here because we really
-        shell=True argument and because we really don't care about the process
+        don't care about the process anymore as soon as it's spawned.
-        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 |
+        log.procs.debug("Executing: {}".format(args))
-                                              QUrl.RemovePassword)
+        subprocess.Popen(args)
-        cmd = cmd.replace('{}', shell_escape(urlstr))
-        log.procs.debug("Executing: {}".format(cmd))
-        subprocess.Popen(cmd, shell=True)
 
     @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)

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)

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^d<e>f%", "a^&b^|c^^d^<e^>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.

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

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.