Merge branch 'pdfjs-fix' of https://github.com/Kingdread/qutebrowser into Kingdread-pdfjs-fix

qutebrowser/browser/network/qutescheme.py

@@ -58,6 +58,25 @@ def add_handler(name):
     return namedecorator
 
 
+class QuteSchemeError(Exception):
+
+    """Exception to signal that a handler should return an ErrorReply.
+
+    Attributes correspond to the arguments in
+    networkreply.ErrorNetworkReply.
+
+    Attributes:
+        errorstring: Error string to print.
+        error: Numerical error value.
+    """
+
+    def __init__(self, errorstring, error):
+        """Constructor."""
+        self.errorstring = errorstring
+        self.error = error
+        super().__init__(errorstring)
+
+
 class QuteSchemeHandler(schemehandler.SchemeHandler):
 
     """Scheme handler for qute: URLs."""
@@ -95,6 +114,9 @@ class QuteSchemeHandler(schemehandler.SchemeHandler):
             return networkreply.ErrorNetworkReply(
                 request, str(e), QNetworkReply.ContentNotFoundError,
                 self.parent())
+        except QuteSchemeError as e:
+            return networkreply.ErrorNetworkReply(
+                request, e.errorstring, e.error, self.parent())
         mimetype, _encoding = mimetypes.guess_type(request.url().fileName())
         if mimetype is None:
             mimetype = 'text/html'
@@ -212,4 +234,13 @@ def qute_settings(win_id, _request):
 def qute_pdfjs(_win_id, request):
     """Handler for qute://pdfjs. Return the pdf.js viewer."""
     urlpath = request.url().path()
-    return pdfjs.get_pdfjs_res(urlpath)
+    try:
+        return pdfjs.get_pdfjs_res(urlpath)
+    except pdfjs.PDFJSNotFound as e:
+        # Logging as the error might get lost otherwise since we're not showing
+        # the error page if a single asset is missing. This way we don't lose
+        # information, as the failed pdfjs requests are still in the log.
+        log.misc.warning(
+            "pdfjs resource requested but not found: {}".format(e.path))
+        raise QuteSchemeError("Can't find pdfjs resource '{}'".format(e.path),
+                QNetworkReply.ContentNotFoundError)

qutebrowser/browser/pdfjs.py

@@ -29,9 +29,16 @@ from qutebrowser.utils import utils
 
 class PDFJSNotFound(Exception):
 
-    """Raised when no pdf.js installation is found."""
+    """Raised when no pdf.js installation is found.
 
-    pass
+    Attributes:
+        path: path of the file that was requested but not found.
+    """
+
+    def __init__(self, path):
+        self.path = path
+        message = "Path '{}' not found".format(path)
+        super().__init__(message)
 
 
 def generate_pdfjs_page(url):
@@ -70,21 +77,21 @@ def fix_urls(asset):
     Args:
         asset: js file or html page as string.
     """
-    new_urls = {
-        'viewer.css': 'qute://pdfjs/web/viewer.css',
-        'compatibility.js': 'qute://pdfjs/web/compatibility.js',
-        'locale/locale.properties':
-            'qute://pdfjs/web/locale/locale.properties',
-        'l10n.js': 'qute://pdfjs/web/l10n.js',
-        '../build/pdf.js': 'qute://pdfjs/build/pdf.js',
-        'debugger.js': 'qute://pdfjs/web/debugger.js',
-        'viewer.js': 'qute://pdfjs/web/viewer.js',
-        'compressed.tracemonkey-pldi-09.pdf': '',
-        './images/': 'qute://pdfjs/web/images/',
-        '../build/pdf.worker.js': 'qute://pdfjs/build/pdf.worker.js',
-        '../web/cmaps/': 'qute://pdfjs/web/cmaps/',
-    }
-    for original, new in new_urls.items():
+    new_urls = [
+        ('viewer.css', 'qute://pdfjs/web/viewer.css'),
+        ('compatibility.js', 'qute://pdfjs/web/compatibility.js'),
+        ('locale/locale.properties',
+            'qute,//pdfjs/web/locale/locale.properties'),
+        ('l10n.js', 'qute://pdfjs/web/l10n.js'),
+        ('../build/pdf.js', 'qute://pdfjs/build/pdf.js'),
+        ('debugger.js', 'qute://pdfjs/web/debugger.js'),
+        ('viewer.js', 'qute://pdfjs/web/viewer.js'),
+        ('compressed.tracemonkey-pldi-09.pdf', ''),
+        ('./images/', 'qute://pdfjs/web/images/'),
+        ('../build/pdf.worker.js', 'qute://pdfjs/build/pdf.worker.js'),
+        ('../web/cmaps/', 'qute://pdfjs/web/cmaps/'),
+    ]
+    for original, new in new_urls:
         asset = asset.replace(original, new)
     return asset
 
@@ -129,7 +136,7 @@ def get_pdfjs_res_and_path(path):
         try:
             content = utils.read_file(res_path, binary=True)
         except FileNotFoundError:
-            raise PDFJSNotFound
+            raise PDFJSNotFound(path) from None
 
     try:
         # Might be script/html or might be binary

tests/unit/browser/network/test_qutescheme.py

@@ -0,0 +1,66 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2016 Daniel Schadt
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests for qutebrowser.browser.network.qutescheme."""
+
+import pytest
+import logging
+
+from PyQt5.QtCore import QUrl
+from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply
+
+from qutebrowser.browser import pdfjs
+from qutebrowser.browser.network import qutescheme
+
+
+@pytest.fixture
+def handler():
+    return qutescheme.QuteSchemeHandler(win_id=0)
+
+
+class TestPDFJSHandler:
+
+    """Test the qute://pdfjs endpoint."""
+
+    @pytest.fixture(autouse=True)
+    def fake_pdfjs(self, monkeypatch):
+
+        def get_pdfjs_res(path):
+            if path == '/existing/file':
+                return b'foobar'
+            raise pdfjs.PDFJSNotFound(path)
+
+        monkeypatch.setattr('qutebrowser.browser.pdfjs.get_pdfjs_res',
+            get_pdfjs_res)
+
+    def test_existing_resource(self, handler):
+        """Test with a resource that exists."""
+        req = QNetworkRequest(QUrl('qute://pdfjs/existing/file'))
+        reply = handler.createRequest(None, req, None)
+        assert reply.readAll() == b'foobar'
+
+    def test_nonexisting_resource(self, handler, caplog):
+        """Test with a resource that does not exist."""
+        req = QNetworkRequest(QUrl('qute://pdfjs/no/file'))
+        with caplog.at_level(logging.WARNING, 'misc'):
+            reply = handler.createRequest(None, req, None)
+        assert reply.error() == QNetworkReply.ContentNotFoundError
+        assert len(caplog.records) == 1
+        assert (caplog.records[0].message ==
+                'pdfjs resource requested but not found: /no/file')

tests/unit/utils/test_version.py

@@ -536,7 +536,7 @@ class TestPDFJSVersion:
 
     def test_not_found(self, mocker):
         mocker.patch('qutebrowser.utils.version.pdfjs.get_pdfjs_res_and_path',
-                     side_effect=pdfjs.PDFJSNotFound)
+                     side_effect=pdfjs.PDFJSNotFound('/build/pdf.js'))
         assert version._pdfjs_version() == 'no'
 
     def test_unknown(self, monkeypatch):