Merge pull request #2354 from pallets/redirect-modern

modern redirect behavior

CHANGES.rst

@@ -90,6 +90,14 @@ Unreleased
     and ``CONTENT_TYPE``. :pr:`2348`
 -   ``append_slash_redirect`` handles ``PATH_INFO`` with internal
     slashes. :issue:`1972`, :pr:`2338`
+-   The default status code for ``append_slash_redirect`` is 308 instead
+    of 301. This preserves the request body, and matches a previous
+    change to ``strict_slashes`` in routing. :issue:`2351`
+-   Fix ``ValueError: I/O operation on closed file.`` with the test
+    client when following more than one redirect. :issue:`2353`
+-   ``Response.autocorrect_location_header`` is disabled by default.
+    The ``Location`` header URL will remain relative, and exclude the
+    scheme and domain, by default. :issue:`2352`
 
 
 Version 2.0.3

setup.cfg

@@ -43,6 +43,8 @@ where = src
 
 [tool:pytest]
 testpaths = tests
+filterwarnings =
+    error
 
 [coverage:run]
 branch = True

src/werkzeug/test.py

@@ -1091,7 +1091,10 @@ def open(
         redirects = set()
         history: t.List["TestResponse"] = []
 
-        while follow_redirects and response.status_code in {
+        if not follow_redirects:
+            return response
+
+        while response.status_code in {
             301,
             302,
             303,
@@ -1118,14 +1121,12 @@ def open(
             history.append(response)
             response = self.resolve_redirect(response, buffered=buffered)
         else:
-            # This is the final request after redirects, or not
-            # following redirects.
+            # This is the final request after redirects.
             response.history = tuple(history)
             # Close the input stream when closing the response, in case
             # the input is an open temporary file.
             response.call_on_close(request.input_stream.close)
-
-        return response
+            return response
 
     def get(self, *args: t.Any, **kw: t.Any) -> "TestResponse":
         """Call :meth:`open` with ``method`` set to ``GET``."""

src/werkzeug/utils.py

@@ -287,7 +287,7 @@ def redirect(
     return response
 
 
-def append_slash_redirect(environ: "WSGIEnvironment", code: int = 301) -> "Response":
+def append_slash_redirect(environ: "WSGIEnvironment", code: int = 308) -> "Response":
     """Redirect to the current URL with a slash appended.
 
     If the current URL is ``/user/42``, the redirect URL will be
@@ -304,6 +304,10 @@ def append_slash_redirect(environ: "WSGIEnvironment", code: int = 301) -> "Respo
     .. versionchanged:: 2.1
         Produce a relative URL that only modifies the last segment.
         Relevant when the current path has multiple segments.
+
+    .. versionchanged:: 2.1
+        The default status code is 308 instead of 301. This preserves
+        the request method and body.
     """
     tail = environ["PATH_INFO"].rpartition("/")[2]
 

src/werkzeug/wrappers/response.py

@@ -142,11 +142,15 @@ def application(environ, start_response):
     #:    your code to the name change.
     implicit_sequence_conversion = True
 
-    #: Should this response object correct the location header to be RFC
-    #: conformant?  This is true by default.
+    #: If a redirect ``Location`` header is a relative URL, make it an
+    #: absolute URL, including scheme and domain.
+    #:
+    #: .. versionchanged:: 2.1
+    #:     This is disabled by default, so responses will send relative
+    #:     redirects.
     #:
     #: .. versionadded:: 0.8
-    autocorrect_location_header = True
+    autocorrect_location_header = False
 
     #: Should this response object automatically set the content-length
     #: header if possible?  This is true by default.

tests/middleware/test_proxy_fix.py

@@ -7,6 +7,7 @@
 from werkzeug.test import create_environ
 from werkzeug.utils import redirect
 from werkzeug.wrappers import Request
+from werkzeug.wrappers import Response
 
 
 @pytest.mark.parametrize(
@@ -158,7 +159,9 @@
         ),
     ),
 )
-def test_proxy_fix(kwargs, base, url_root):
+def test_proxy_fix(monkeypatch, kwargs, base, url_root):
+    monkeypatch.setattr(Response, "autocorrect_location_header", True)
+
     @Request.application
     def app(request):
         # for header
@@ -173,7 +176,8 @@ def app(request):
         # match doesn't include prefix
         assert urls.match("/parrot")[0] == "parrot"
 
-        # location header will start with url_root
+        # With autocorrect_location_header enabled, location header will
+        # start with url_root
         return redirect(parrot_url)
 
     url_map = Map([Rule("/parrot", endpoint="parrot")])

tests/test_test.py

@@ -556,13 +556,13 @@ def app(request):
 
     assert response.history[-1].request.path == "/second"
     assert response.history[-1].status_code == 302
-    assert response.history[-1].location == "http://localhost/third"
+    assert response.history[-1].location == "/third"
     assert len(response.history[-1].history) == 1
     assert response.history[-1].history[-1] is response.history[-2]
 
     assert response.history[-2].request.path == "/first"
     assert response.history[-2].status_code == 302
-    assert response.history[-2].location == "http://localhost/second"
+    assert response.history[-2].location == "/second"
     assert len(response.history[-2].history) == 0
 
 

tests/test_utils.py

@@ -3,6 +3,7 @@
 
 import pytest
 
+from werkzeug import Request
 from werkzeug import utils
 from werkzeug.datastructures import Headers
 from werkzeug.http import http_date
@@ -237,7 +238,7 @@ def test_header_set_duplication_bug():
 
 
 @pytest.mark.parametrize(
-    "path, base_url, expected_location",
+    ("path", "base_url", "absolute_location"),
     [
         ("foo", "http://example.org/app", "http://example.org/app/foo/"),
         ("/foo", "http://example.org/app", "http://example.org/app/foo/"),
@@ -250,14 +251,22 @@ def test_header_set_duplication_bug():
         ("/", "http://example.org/app", "http://example.org/app/"),
     ],
 )
-def test_append_slash_redirect(path, base_url, expected_location):
-    def app(env, sr):
-        return utils.append_slash_redirect(env)(env, sr)
+@pytest.mark.parametrize("autocorrect", [False, True])
+def test_append_slash_redirect(autocorrect, path, base_url, absolute_location):
+    @Request.application
+    def app(request):
+        rv = utils.append_slash_redirect(request.environ)
+        rv.autocorrect_location_header = autocorrect
+        return rv
 
     client = Client(app)
     response = client.get(path, base_url=base_url)
-    assert response.status_code == 301
-    assert response.headers["Location"] == expected_location
+    assert response.status_code == 308
+
+    if not autocorrect:
+        assert response.headers["Location"].count("/") == 1
+    else:
+        assert response.headers["Location"] == absolute_location
 
 
 def test_cached_property_doc():