From 9973cd503782e5019dc4621b53f04b5cdb0f1824 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 00:46:59 +0200 Subject: [PATCH 1/6] downloads: don't crash on OSError in open-download Fixes the crash in #1725, but does not provide a solution. The browser won't crash, but the file won't be downloaded and opened either. --- qutebrowser/browser/webkit/downloads.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 87e038841..474f8d3e2 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -947,7 +947,13 @@ class DownloadManager(QAbstractListModel): download.set_filename(target.filename) elif isinstance(target, usertypes.OpenFileDownloadTarget): tmp_manager = objreg.get('temporary-downloads') - fobj = tmp_manager.get_tmpfile(suggested_filename) + try: + fobj = tmp_manager.get_tmpfile(suggested_filename) + except OSError as exc: + msg = "Download error: {}".format(exc) + message.error(self._win_id, msg) + download.cancel() + return download.finished.connect(download.open_file) download.autoclose = True download.set_fileobj(fobj) From abcdaa9ccedb01c5e9eae998e51f29cd083df676 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 00:57:22 +0200 Subject: [PATCH 2/6] open-download: make sure the name is not too long Fixes #1725. Make sure that the temporary filename is not too long by restricting the suggested part to 20 characters. --- qutebrowser/browser/webkit/downloads.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 474f8d3e2..f51dd81b7 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -1322,6 +1322,9 @@ class TempDownloadManager(QObject): A tempfile.NamedTemporaryFile that should be used to save the file. """ tmpdir = self._get_tmpdir() + # Make sure that the filename is not too long + if len(suggested_name) > 20: + suggested_name = suggested_name[:10] + '...' + suggested_name[-10:] fobj = tempfile.NamedTemporaryFile(dir=tmpdir.name, delete=False, suffix=suggested_name) self.files.append(fobj) From b187b680cb5a376de02ed518ac1f72e7810ca742 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 01:00:57 +0200 Subject: [PATCH 3/6] open-download: force encoding for filename Fixes #1726. --- qutebrowser/browser/webkit/downloads.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index f51dd81b7..47d264d37 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -1322,6 +1322,8 @@ class TempDownloadManager(QObject): A tempfile.NamedTemporaryFile that should be used to save the file. """ tmpdir = self._get_tmpdir() + encoding = sys.getfilesystemencoding() + suggested_name = utils.force_encoding(suggested_name, encoding) # Make sure that the filename is not too long if len(suggested_name) > 20: suggested_name = suggested_name[:10] + '...' + suggested_name[-10:] From 57ceaeeb557dbb6f4cad78ea5dcdcdd7fb2e755c Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 01:10:40 +0200 Subject: [PATCH 4/6] open-download: don't crash on download cancel Fixes #1728. --- qutebrowser/browser/webkit/downloads.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 47d264d37..723edebb8 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -954,12 +954,21 @@ class DownloadManager(QAbstractListModel): message.error(self._win_id, msg) download.cancel() return - download.finished.connect(download.open_file) + download.finished.connect( + functools.partial(self._open_download, download)) download.autoclose = True download.set_fileobj(fobj) else: log.downloads.error("Unknown download target: {}".format(target)) + def _open_download(self, download): + """Open the given download but only if it was successful.""" + if download.successful: + download.open_file() + else: + log.downloads.debug("{} finished but not successful, not opening!" + .format(download)) + def raise_no_download(self, count): """Raise an exception that the download doesn't exist. From a3047008ddee7b83a8cb8914686651928f16423e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 2 Aug 2016 16:12:09 +0200 Subject: [PATCH 5/6] Bump up filename length limit to 50 The usual limit seems to be 255 bytes, so even when assuming 5-byte UTF-8 chars for every letter, 50 should be fine. http://serverfault.com/questions/9546/filename-length-limits-on-linux/9548#9548 --- qutebrowser/browser/webkit/downloads.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 723edebb8..e5f9e8709 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -1334,8 +1334,8 @@ class TempDownloadManager(QObject): encoding = sys.getfilesystemencoding() suggested_name = utils.force_encoding(suggested_name, encoding) # Make sure that the filename is not too long - if len(suggested_name) > 20: - suggested_name = suggested_name[:10] + '...' + suggested_name[-10:] + if len(suggested_name) > 50: + suggested_name = suggested_name[:25] + '...' + suggested_name[-25:] fobj = tempfile.NamedTemporaryFile(dir=tmpdir.name, delete=False, suffix=suggested_name) self.files.append(fobj) From 48067f0c7656f240022fc69f1d402c8e5cf498b3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 2 Aug 2016 16:15:30 +0200 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.asciidoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 2669428ef..d08cabaf9 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -53,6 +53,12 @@ Fixed relative path with `--basedir`. - Fixed crash when deleting a quickmark with Ctrl-D - Fixed HTML5 video playback on Windows +- Fixed crash when using `:prompt-open-download` with a file with chars not + encodable with the OS' filesystem encoding (e.g. with `LC_ALL=C`) +- Fixed `:prompt-open-download` with a too long filename (< 255 bytes) +- Fixed crash when cancelling a download after doing `:prompt-open-download` +- Fixed crash when writing a download to disk fails with + `:prompt-open-download`. v0.8.1 ------