interceptor: Make sure redirect target URL is valid

Catches issues with invalid URLs early instead of later in the code (e.g. when
the brave adblocker runs on the URL).

Hopefully helps catch issues with broken config.py hacks calling redirect().

2 files changed, 99 insertions, 6 deletions

diff --git a/qutebrowser/browser/webengine/interceptor.py b/qutebrowser/browser/webengine/interceptor.py
index ac0795803..161f5ffab 100644
--- a/qutebrowser/browser/webengine/interceptor.py
+++ b/qutebrowser/browser/webengine/interceptor.py
@@ -10,7 +10,7 @@ from qutebrowser.qt.webenginecore import (QWebEngineUrlRequestInterceptor,
 
 from qutebrowser.config import websettings, config
 from qutebrowser.browser import shared
-from qutebrowser.utils import debug, log
+from qutebrowser.utils import debug, log, qtutils
 from qutebrowser.extensions import interceptors
 from qutebrowser.misc import objects
 
@@ -35,6 +35,11 @@ class WebEngineRequest(interceptors.Request):
         if self._webengine_info is None:
             raise interceptors.RedirectException("Request improperly initialized.")
 
+        try:
+            qtutils.ensure_valid(url)
+        except qtutils.QtValueError as e:
+            raise interceptors.RedirectException(f"Redirect to invalid URL: {e}")
+
         # Redirecting a request that contains payload data is not allowed.
         # To be safe, abort on any request not in a whitelist.
         verb = self._webengine_info.requestMethod()
diff --git a/tests/unit/browser/webengine/test_webengineinterceptor.py b/tests/unit/browser/webengine/test_webengineinterceptor.py
index d579d60f7..506e65284 100644
--- a/tests/unit/browser/webengine/test_webengineinterceptor.py
+++ b/tests/unit/browser/webengine/test_webengineinterceptor.py
@@ -6,12 +6,15 @@
 
 
 import pytest
+import pytest_mock
 
-pytest.importorskip('qutebrowser.qt.webenginecore')
+pytest.importorskip("qutebrowser.qt.webenginecore")
 
+from qutebrowser.qt.core import QUrl, QByteArray
 from qutebrowser.qt.webenginecore import QWebEngineUrlRequestInfo
 
 from qutebrowser.browser.webengine import interceptor
+from qutebrowser.extensions import interceptors
 from qutebrowser.utils import qtutils
 from helpers import testutils
 
@@ -19,10 +22,12 @@ from helpers import testutils
 def test_no_missing_resource_types():
     request_interceptor = interceptor.RequestInterceptor()
     qb_keys = set(request_interceptor._resource_types.keys())
-    qt_keys = set(testutils.enum_members(
-        QWebEngineUrlRequestInfo,
-        QWebEngineUrlRequestInfo.ResourceType,
-    ).values())
+    qt_keys = set(
+        testutils.enum_members(
+            QWebEngineUrlRequestInfo,
+            QWebEngineUrlRequestInfo.ResourceType,
+        ).values()
+    )
     assert qt_keys == qb_keys
 
 
@@ -30,3 +35,86 @@ def test_resource_type_values():
     request_interceptor = interceptor.RequestInterceptor()
     for qt_value, qb_item in request_interceptor._resource_types.items():
         assert qtutils.extract_enum_val(qt_value) == qb_item.value
+
+
+@pytest.fixture
+def we_request(  # a shrubbery!
+    mocker: pytest_mock.MockerFixture,
+) -> interceptor.WebEngineRequest:
+    qt_info = mocker.Mock(spec=QWebEngineUrlRequestInfo)
+    qt_info.requestMethod.return_value = QByteArray(b"GET")
+    first_party_url = QUrl("https://firstparty.example.org/")
+    request_url = QUrl("https://request.example.org/")
+    return interceptor.WebEngineRequest(
+        first_party_url=first_party_url,
+        request_url=request_url,
+        webengine_info=qt_info,
+    )
+
+
+def test_block(we_request: interceptor.WebEngineRequest):
+    assert not we_request.is_blocked
+    we_request.block()
+    assert we_request.is_blocked
+
+
+class TestRedirect:
+
+    REDIRECT_URL = QUrl("https://redirect.example.com/")
+
+    def test_redirect(self, we_request: interceptor.WebEngineRequest):
+        assert not we_request._redirected
+        we_request.redirect(self.REDIRECT_URL)
+        assert we_request._redirected
+        we_request._webengine_info.redirect.assert_called_once_with(self.REDIRECT_URL)
+
+
+    def test_twice(self, we_request: interceptor.WebEngineRequest):
+        we_request.redirect(self.REDIRECT_URL)
+        with pytest.raises(
+            interceptors.RedirectException,
+            match=r"Request already redirected.",
+        ):
+            we_request.redirect(self.REDIRECT_URL)
+        we_request._webengine_info.redirect.assert_called_once_with(self.REDIRECT_URL)
+
+
+    def test_invalid_method(self, we_request: interceptor.WebEngineRequest):
+        we_request._webengine_info.requestMethod.return_value = QByteArray(b"POST")
+        with pytest.raises(
+            interceptors.RedirectException,
+            match=(
+                r"Request method b'POST' for https://request.example.org/ does not "
+                r"support redirection."
+            ),
+        ):
+            we_request.redirect(self.REDIRECT_URL)
+        assert not we_request._webengine_info.redirect.called
+
+
+    def test_invalid_method_ignore_unsupported(self, we_request: interceptor.WebEngineRequest, caplog: pytest.LogCaptureFixture):
+        we_request._webengine_info.requestMethod.return_value = QByteArray(b"POST")
+        we_request.redirect(self.REDIRECT_URL, ignore_unsupported=True)
+        assert caplog.messages == [
+            "Request method b'POST' for https://request.example.org/ does not support "
+            "redirection."
+        ]
+        assert not we_request._webengine_info.redirect.called
+
+
+    def test_improperly_initialized(self, we_request: interceptor.WebEngineRequest):
+        we_request._webengine_info = None
+        with pytest.raises(
+            interceptors.RedirectException,
+            match=r"Request improperly initialized.",
+        ):
+            we_request.redirect(self.REDIRECT_URL)
+
+    def test_invalid_url(self, we_request: interceptor.WebEngineRequest):
+        url = QUrl()
+        assert not url.isValid()
+        with pytest.raises(
+            interceptors.RedirectException,
+            match=r"Redirect to invalid URL: PyQt6\.QtCore\.QUrl\(''\) is not valid",
+        ):
+            we_request.redirect(url)