Merge pull request #4492 from pallets/debug-messages

update some debug message behavior

src/flask/app.py

@@ -1457,17 +1457,26 @@ class Flask(Scaffold):
         )
 
     def raise_routing_exception(self, request: Request) -> "te.NoReturn":
-        """Exceptions that are recording during routing are reraised with
-        this method.  During debug we are not reraising redirect requests
-        for non ``GET``, ``HEAD``, or ``OPTIONS`` requests and we're raising
-        a different error instead to help debug situations.
+        """Intercept routing exceptions and possibly do something else.
 
+        In debug mode, intercept a routing redirect and replace it with
+        an error if the body will be discarded.
+
+        With modern Werkzeug this shouldn't occur, since it now uses a
+        308 status which tells the browser to resend the method and
+        body.
+
+        .. versionchanged:: 2.1
+            Don't intercept 307 and 308 redirects.
+
+        :meta private:
         :internal:
         """
         if (
             not self.debug
             or not isinstance(request.routing_exception, RequestRedirect)
-            or request.method in ("GET", "HEAD", "OPTIONS")
+            or request.routing_exception.code in {307, 308}
+            or request.method in {"GET", "HEAD", "OPTIONS"}
         ):
             raise request.routing_exception  # type: ignore
 

src/flask/debughelpers.py

@@ -41,53 +41,55 @@ class DebugFilesKeyError(KeyError, AssertionError):
 
 
 class FormDataRoutingRedirect(AssertionError):
-    """This exception is raised by Flask in debug mode if it detects a
-    redirect caused by the routing system when the request method is not
-    GET, HEAD or OPTIONS.  Reasoning: form data will be dropped.
+    """This exception is raised in debug mode if a routing redirect
+    would cause the browser to drop the method or body. This happens
+    when method is not GET, HEAD or OPTIONS and the status code is not
+    307 or 308.
     """
 
     def __init__(self, request):
         exc = request.routing_exception
         buf = [
-            f"A request was sent to this URL ({request.url}) but a"
-            " redirect was issued automatically by the routing system"
-            f" to {exc.new_url!r}."
+            f"A request was sent to '{request.url}', but routing issued"
+            f" a redirect to the canonical URL '{exc.new_url}'."
         ]
 
-        # In case just a slash was appended we can be extra helpful
-        if f"{request.base_url}/" == exc.new_url.split("?")[0]:
+        if f"{request.base_url}/" == exc.new_url.partition("?")[0]:
             buf.append(
-                "  The URL was defined with a trailing slash so Flask"
-                " will automatically redirect to the URL with the"
-                " trailing slash if it was accessed without one."
+                " The URL was defined with a trailing slash. Flask"
+                " will redirect to the URL with a trailing slash if it"
+                " was accessed without one."
             )
 
         buf.append(
-            "  Make sure to directly send your"
-            f" {request.method}-request to this URL since we can't make"
-            " browsers or HTTP clients redirect with form data reliably"
-            " or without user interaction."
+            " Send requests to the canonical URL, or use 307 or 308 for"
+            " routing redirects. Otherwise, browsers will drop form"
+            " data.\n\n"
+            "This exception is only raised in debug mode."
         )
-        buf.append("\n\nNote: this exception is only raised in debug mode")
-        AssertionError.__init__(self, "".join(buf).encode("utf-8"))
+        super().__init__("".join(buf))
 
 
 def attach_enctype_error_multidict(request):
-    """Since Flask 0.8 we're monkeypatching the files object in case a
-    request is detected that does not use multipart form data but the files
-    object is accessed.
+    """Patch ``request.files.__getitem__`` to raise a descriptive error
+    about ``enctype=multipart/form-data``.
+
+    :param request: The request to patch.
+    :meta private:
     """
     oldcls = request.files.__class__
 
     class newcls(oldcls):
         def __getitem__(self, key):
             try:
-                return oldcls.__getitem__(self, key)
+                return super().__getitem__(key)
             except KeyError as e:
                 if key not in request.form:
                     raise
 
-                raise DebugFilesKeyError(request, key) from e
+                raise DebugFilesKeyError(request, key).with_traceback(
+                    e.__traceback__
+                ) from None
 
     newcls.__name__ = oldcls.__name__
     newcls.__module__ = oldcls.__module__

src/flask/wrappers.py

@@ -9,7 +9,6 @@ from .globals import current_app
 from .helpers import _split_blueprint_path
 
 if t.TYPE_CHECKING:
-    import typing_extensions as te
     from werkzeug.routing import Rule
 
 
@@ -110,7 +109,7 @@ class Request(RequestBase):
         return _split_blueprint_path(name)
 
     def _load_form_data(self) -> None:
-        RequestBase._load_form_data(self)
+        super()._load_form_data()
 
         # In debug mode we're replacing the files multidict with an ad-hoc
         # subclass that raises a different error for key errors.
@@ -124,11 +123,14 @@ class Request(RequestBase):
 
             attach_enctype_error_multidict(self)
 
-    def on_json_loading_failed(self, e: Exception) -> "te.NoReturn":
-        if current_app and current_app.debug:
-            raise BadRequest(f"Failed to decode JSON object: {e}")
+    def on_json_loading_failed(self, e: ValueError) -> t.Any:
+        try:
+            return super().on_json_loading_failed(e)
+        except BadRequest as e:
+            if current_app and current_app.debug:
+                raise
 
-        raise BadRequest()
+            raise BadRequest() from e
 
 
 class Response(ResponseBase):

tests/test_basic.py

@@ -15,6 +15,7 @@ from werkzeug.exceptions import Forbidden
 from werkzeug.exceptions import NotFound
 from werkzeug.http import parse_date
 from werkzeug.routing import BuildError
+from werkzeug.routing import RequestRedirect
 
 import flask
 
@@ -1724,28 +1725,29 @@ def test_before_first_request_functions_concurrent(app, client):
     assert app.got_first_request
 
 
-def test_routing_redirect_debugging(app, client):
-    app.debug = True
-
+def test_routing_redirect_debugging(monkeypatch, app, client):
     @app.route("/foo/", methods=["GET", "POST"])
     def foo():
         return "success"
 
-    with client:
-        with pytest.raises(AssertionError) as e:
-            client.post("/foo", data={})
-        assert "http://localhost/foo/" in str(e.value)
-        assert "Make sure to directly send your POST-request to this URL" in str(
-            e.value
-        )
-
-        rv = client.get("/foo", data={}, follow_redirects=True)
-        assert rv.data == b"success"
-
     app.debug = False
+    rv = client.post("/foo", data={}, follow_redirects=True)
+    assert rv.data == b"success"
+
+    app.debug = True
+
     with client:
         rv = client.post("/foo", data={}, follow_redirects=True)
         assert rv.data == b"success"
+        rv = client.get("/foo", data={}, follow_redirects=True)
+        assert rv.data == b"success"
+
+    monkeypatch.setattr(RequestRedirect, "code", 301)
+
+    with client, pytest.raises(AssertionError) as e:
+        client.post("/foo", data={})
+
+    assert "canonical URL 'http://localhost/foo/'" in str(e.value)
 
 
 def test_route_decorator_custom_endpoint(app, client):