From dcac832f5e81858a774d45c07a99845827aa71c2 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 5 May 2016 00:04:58 +0200 Subject: [PATCH 1/3] netmanager: fix crash when asking with no tab_id Issue 1413 This happens when the networkmanager is used by something that has no tab_id, like the generic DownloadManager. In this case, we should just skip the webview connection (as it makes no sense) instead of crashing (which is the last thing we want to do). --- qutebrowser/browser/network/networkmanager.py | 10 +++++++--- tests/integration/features/downloads.feature | 10 +++++++++- tests/integration/features/test_downloads.py | 6 ++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/network/networkmanager.py b/qutebrowser/browser/network/networkmanager.py index 89b566875..4945959cd 100644 --- a/qutebrowser/browser/network/networkmanager.py +++ b/qutebrowser/browser/network/networkmanager.py @@ -226,9 +226,13 @@ class NetworkManager(QNetworkAccessManager): self.shutting_down.connect(q.abort) if owner is not None: owner.destroyed.connect(q.abort) - webview = objreg.get('webview', scope='tab', window=self._win_id, - tab=self._tab_id) - webview.loadStarted.connect(q.abort) + + # This might be a generic network manager, e.g. one belonging to a + # DownloadManager. In this case, just skip the webview thing. + if self._tab_id is not None: + webview = objreg.get('webview', scope='tab', window=self._win_id, + tab=self._tab_id) + webview.loadStarted.connect(q.abort) bridge = objreg.get('message-bridge', scope='window', window=self._win_id) bridge.ask(q, blocking=True) diff --git a/tests/integration/features/downloads.feature b/tests/integration/features/downloads.feature index 5e6a90252..8509a435c 100644 --- a/tests/integration/features/downloads.feature +++ b/tests/integration/features/downloads.feature @@ -44,6 +44,14 @@ Feature: Downloading things from a website. And I run :leave-mode Then no crash should happen + Scenario: Downloading with SSL errors (issue 1413) + When I run :debug-clear-ssl-errors + And I set network -> ssl-strict to ask + And I download a SSL page + And I wait for "Entering mode KeyMode.* (reason: question asked)" in the log + And I run :prompt-accept + Then the error "Download error: SSL handshake failed" should be shown + Scenario: Retrying a failed download When I run :download http://localhost:(port)/does-not-exist And I wait for the error "Download error: * - server replied: NOT FOUND" @@ -162,7 +170,7 @@ Feature: Downloading things from a website. When I run :download http://localhost:(port)/drip?numbytes=128&duration=5 And I run :download-open with count 1 Then the error "Download 1 is not done!" should be shown - + ## completion -> download-path-suggestion Scenario: completion -> download-path-suggestion = path diff --git a/tests/integration/features/test_downloads.py b/tests/integration/features/test_downloads.py index 36e32a758..1f3783543 100644 --- a/tests/integration/features/test_downloads.py +++ b/tests/integration/features/test_downloads.py @@ -41,6 +41,12 @@ def wait_for_download_finished(quteproc): quteproc.wait_for(category='downloads', message='Download finished') +@bdd.when("I download a SSL page") +def download_ssl_page(quteproc, ssl_server): + quteproc.send_cmd(':download https://localhost:{}/' + .format(ssl_server.port)) + + @bdd.then(bdd.parsers.parse("The downloaded file {filename} should not exist")) def download_should_not_exist(filename, tmpdir): path = tmpdir / filename From 2918c5cd57be08b1f1544d802fc225ef84ac35be Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Fri, 6 May 2016 18:01:45 +0200 Subject: [PATCH 2/3] downloads: close fileobject in DownloadItem._die Otherwise we will get a unclosed resource warning. --- qutebrowser/browser/downloads.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 85c957fb9..6cbeff434 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -412,6 +412,8 @@ class DownloadItem(QObject): self.reply = None self.done = True self.data_changed.emit() + if self.fileobj is not None: + self.fileobj.close() def init_reply(self, reply): """Set a new reply and connect its signals. From e81c13cf35332697fb71526257e730552c123b40 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 6 May 2016 18:23:16 +0200 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.asciidoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 228932f85..a75a543ce 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -45,6 +45,12 @@ Changed versions those already all are disabled, but with older versions they might not be. +Fixed +----- + +- Fixed crash when downloading from an URL with SSL errors +- Close file handles correctly when a download failed + v0.6.2 ------