diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 0de8cf068..45c2a7b9f 100644 --- a/qutebrowser/browser/commands.py +++ b/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() diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 51ccdbfeb..f9b246d3a 100644 --- a/qutebrowser/browser/downloads.py +++ b/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.""" diff --git a/qutebrowser/mainwindow/prompt.py b/qutebrowser/mainwindow/prompt.py index afe8a72dd..8a469cb00 100644 --- a/qutebrowser/mainwindow/prompt.py +++ b/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 diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index fd4466e2a..2aca78168 100644 --- a/qutebrowser/utils/utils.py +++ b/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 diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 444cf8721..1e4587740 100644 --- a/tests/end2end/features/downloads.feature +++ b/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 text='Please enter a location for http://localhost:*/data/downloads/download.bin' 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 text='Please enter a location for http://localhost:*/data/downloads/download.bin' 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 diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 12330aa73..c60b640d6 100644 --- a/tests/unit/utils/test_utils.py +++ b/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