Merge branch 'Kingdread/windows-drive-cwd'

qutebrowser/browser/commands.py

@@ -1337,6 +1337,9 @@ class CommandDispatcher:
                                       scope='window', window=self._win_id)
         target = None
         if dest is not None:
+            dest = downloads.transform_path(dest)
+            if dest is None:
+                raise cmdexc.CommandError("Invalid target filename")
             target = downloads.FileDownloadTarget(dest)
 
         tab = self._current_widget()

qutebrowser/browser/downloads.py

@@ -19,11 +19,13 @@
 
 """Shared QtWebKit/QtWebEngine code for downloads."""
 
+import re
 import sys
 import html
 import os.path
 import collections
 import functools
+import pathlib
 import tempfile
 
 import sip
@@ -161,6 +163,25 @@ def get_filename_question(*, suggested_filename, url, parent=None):
     return q
 
 
+def transform_path(path):
+    r"""Do platform-specific transformations, like changing E: to E:\.
+
+    Returns None if the path is invalid on the current platform.
+    """
+    if sys.platform != "win32":
+        return path
+    path = utils.expand_windows_drive(path)
+    # Drive dependent working directories are not supported, e.g.
+    # E:filename is invalid
+    if re.match(r'[A-Z]:[^\\]', path, re.IGNORECASE):
+        return None
+    # Paths like COM1, ...
+    # See https://github.com/qutebrowser/qutebrowser/issues/82
+    if pathlib.Path(path).is_reserved():
+        return None
+    return path
+
+
 class NoFilenameError(Exception):
 
     """Raised when we can't find out a filename in DownloadTarget."""

qutebrowser/mainwindow/prompt.py

@@ -25,9 +25,10 @@ import collections
 
 import sip
 from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QTimer, QDir, QModelIndex,
-                          QItemSelectionModel, QObject, QEventLoop)
+                          QItemSelectionModel, QObject, QEventLoop, QPoint)
 from PyQt5.QtWidgets import (QWidget, QGridLayout, QVBoxLayout, QLineEdit,
-                             QLabel, QFileSystemModel, QTreeView, QSizePolicy)
+                             QLabel, QFileSystemModel, QTreeView, QSizePolicy,
+                             QToolTip)
 
 from qutebrowser.browser import downloads
 from qutebrowser.config import style, config
@@ -642,8 +643,16 @@ class FilenamePrompt(_BasePrompt):
         self._file_model.directoryLoaded.connect(
             lambda: self._file_model.sort(0))
 
+    def _show_error(self, msg):
+        log.prompt.error(msg)
+        QToolTip.showText(self._lineedit.mapToGlobal(QPoint(0, 0)), msg)
+
     def accept(self, value=None):
         text = value if value is not None else self._lineedit.text()
+        text = downloads.transform_path(text)
+        if text is None:
+            self._show_error("Invalid filename")
+            return False
         self.question.answer = text
         return True
 
@@ -695,6 +704,10 @@ class DownloadFilenamePrompt(FilenamePrompt):
 
     def accept(self, value=None):
         text = value if value is not None else self._lineedit.text()
+        text = downloads.transform_path(text)
+        if text is None:
+            self._show_error("Invalid filename")
+            return False
         self.question.answer = downloads.FileDownloadTarget(text)
         return True
 

qutebrowser/utils/utils.py

@@ -20,6 +20,7 @@
 """Other utilities which don't fit anywhere else."""
 
 import io
+import re
 import sys
 import enum
 import json
@@ -845,3 +846,21 @@ def open_file(filename, cmdline=None):
 def unused(_arg):
     """Function which does nothing to avoid pylint complaining."""
     pass
+
+
+def expand_windows_drive(path):
+    r"""Expand a drive-path like E: into E:\.
+
+    Does nothing for other paths.
+
+    Args:
+        path: The path to expand.
+    """
+    # Usually, "E:" on Windows refers to the current working directory on drive
+    # E:\. The correct way to specifify drive E: is "E:\", but most users
+    # probably don't use the "multiple working directories" feature and expect
+    # "E:" and "E:\" to be equal.
+    if re.match(r'[A-Z]:$', path, re.IGNORECASE):
+        return path + "\\"
+    else:
+        return path

tests/end2end/features/downloads.feature

@@ -126,6 +126,34 @@ Feature: Downloading things from a website.
         Then the downloaded file ../foo should not exist
         And the downloaded file foo should exist
 
+    @windows
+    Scenario: Downloading a file to a reserved path
+        When I set storage -> prompt-download-directory to true
+        And I open data/downloads/download.bin
+        And I wait for "Asking question <qutebrowser.utils.usertypes.Question default='*' mode=<PromptMode.download: 5> text='Please enter a location for <b>http://localhost:*/data/downloads/download.bin</b>' title='Save file to:'>, *" in the log
+        And I run :prompt-accept COM1
+        And I run :leave-mode
+        Then "Invalid filename" should be logged
+
+    @windows
+    Scenario: Downloading a file to a drive-relative working directory
+        When I set storage -> prompt-download-directory to true
+        And I open data/downloads/download.bin
+        And I wait for "Asking question <qutebrowser.utils.usertypes.Question default='*' mode=<PromptMode.download: 5> text='Please enter a location for <b>http://localhost:*/data/downloads/download.bin</b>' title='Save file to:'>, *" in the log
+        And I run :prompt-accept C:foobar
+        And I run :leave-mode
+        Then "Invalid filename" should be logged
+
+    @windows
+    Scenario: Downloading a file to a reserved path with :download
+        When I run :download data/downloads/download.bin --dest=COM1
+        Then the error "Invalid target filename" should be shown
+
+    @windows
+    Scenario: Download a file to a drive-relative working directory with :download
+        When I run :download data/downloads/download.bin --dest=C:foobar
+        Then the error "Invalid target filename" should be shown
+
     ## :download-retry
 
     Scenario: Retrying a failed download

tests/unit/utils/test_utils.py

@@ -916,3 +916,16 @@ class TestOpenFile:
 
 def test_unused():
     utils.unused(None)
+
+
+@pytest.mark.parametrize('path, expected', [
+    ('E:', 'E:\\'),
+    ('e:', 'e:\\'),
+    ('E:foo', 'E:foo'),
+    ('E:\\', 'E:\\'),
+    ('E:\\foo', 'E:\\foo'),
+    ('foo:', 'foo:'),
+    ('foo:bar', 'foo:bar'),
+])
+def test_expand_windows_drive(path, expected):
+    assert utils.expand_windows_drive(path) == expected