diff --git a/src/flask/app.py b/src/flask/app.py index 3a4df3665e..348bc7f74f 100644 --- a/src/flask/app.py +++ b/src/flask/app.py @@ -1457,17 +1457,26 @@ def log_exception( ) 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 diff --git a/src/flask/debughelpers.py b/src/flask/debughelpers.py index 212f7d7ee8..27d378c24a 100644 --- a/src/flask/debughelpers.py +++ b/src/flask/debughelpers.py @@ -41,53 +41,55 @@ def __str__(self): 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__ diff --git a/src/flask/wrappers.py b/src/flask/wrappers.py index 47dbe5c8d8..9a1611c44a 100644 --- a/src/flask/wrappers.py +++ b/src/flask/wrappers.py @@ -9,7 +9,6 @@ from .helpers import _split_blueprint_path if t.TYPE_CHECKING: - import typing_extensions as te from werkzeug.routing import Rule @@ -110,7 +109,7 @@ def blueprints(self) -> t.List[str]: 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 @@ def _load_form_data(self) -> None: 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): diff --git a/tests/test_basic.py b/tests/test_basic.py index c5b83c7c03..2a177e9a70 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -15,6 +15,7 @@ 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 get_and_assert(): 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 - ) + app.debug = False + 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" + app.debug = True - app.debug = False 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):