Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modern redirect behavior #2354

Merged
merged 4 commits into from Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGES.rst
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions setup.cfg
Expand Up @@ -43,6 +43,8 @@ where = src

[tool:pytest]
testpaths = tests
filterwarnings =
error

[coverage:run]
branch = True
Expand Down
11 changes: 6 additions & 5 deletions src/werkzeug/test.py
Expand Up @@ -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,
Expand All @@ -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``."""
Expand Down
6 changes: 5 additions & 1 deletion src/werkzeug/utils.py
Expand Up @@ -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
Expand All @@ -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]

Expand Down
10 changes: 7 additions & 3 deletions src/werkzeug/wrappers/response.py
Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions tests/middleware/test_proxy_fix.py
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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")])
Expand Down
4 changes: 2 additions & 2 deletions tests/test_test.py
Expand Up @@ -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


Expand Down
21 changes: 15 additions & 6 deletions tests/test_utils.py
Expand Up @@ -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
Expand Down Expand Up @@ -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/"),
Expand All @@ -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():
Expand Down