Tests are failing with the latest werkzeug 2.0.1

[frenzymadness]

/usr/bin/python3 test_httpbin.py
....................................FFFFF......................
======================================================================
FAIL: test_redirect_n_equals_to_1 (__main__.HttpbinTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/httpbin-0.7.0/test_httpbin.py", line 549, in test_redirect_n_equals_to_1
    self.assertEqual(
AssertionError: 'http://localhost/get' != '/get'
- http://localhost/get
+ /get


======================================================================
FAIL: test_redirect_n_higher_than_1 (__main__.HttpbinTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/httpbin-0.7.0/test_httpbin.py", line 527, in test_redirect_n_higher_than_1
    self.assertEqual(
AssertionError: 'http://localhost/relative-redirect/4' != '/relative-redirect/4'
- http://localhost/relative-redirect/4
+ /relative-redirect/4


======================================================================
FAIL: test_redirect_to_post (__main__.HttpbinTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/httpbin-0.7.0/test_httpbin.py", line 536, in test_redirect_to_post
    self.assertEqual(
AssertionError: 'http://localhost/post' != '/post'
- http://localhost/post
+ /post


======================================================================
FAIL: test_relative_redirect_n_equals_to_1 (__main__.HttpbinTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/httpbin-0.7.0/test_httpbin.py", line 555, in test_relative_redirect_n_equals_to_1
    self.assertEqual(
AssertionError: 'http://localhost/get' != '/get'
- http://localhost/get
+ /get


======================================================================
FAIL: test_relative_redirect_n_higher_than_1 (__main__.HttpbinTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/httpbin-0.7.0/test_httpbin.py", line 562, in test_relative_redirect_n_higher_than_1
    self.assertEqual(
AssertionError: 'http://localhost/relative-redirect/6' != '/relative-redirect/6'
- http://localhost/relative-redirect/6
+ /relative-redirect/6


----------------------------------------------------------------------
Ran 63 tests in 6.087s

FAILED (failures=5)

[AdamWill]

sorry, previous comment was wrong. This is caused by a change in werkzeug. I bisected it, and the problematic commit I think is pallets/werkzeug@2cd4fa9 - this was a little hard to debug because there's a commit in the history that seems to break werkzeug entirely with Python 3.10 at least, I had to use an 'alternative history' branch where I fixed the bug in that commit to do a successful bisect.

As best I can tell, something somewhere in there is messing with responses before they're returned. for e.g. the failure in test_redirect_to_post - that test is backed by redirect_to in httpbin/core.py. If I log what the "Location" header in the response is right before redirect_to returns, it's correct - it's just /post. So something else between that function returning and the response actually getting back to test_redirect_to_post is messing with the response and making the "Location" absolute.

[AdamWill]

@davidism ahoy! Hope you don't mind the ping. Any idea what's going on here, as you wrote the commit that seems to have caused it? It's a big commit and I'm struggling to pinpoint the issue. The stuff about autocorrect_location_header obviously looks like a suspect, but I can't see that that code actually changed in the commit, so much as just moved...

[davidism]

As far as I understand it, autocorrect_location_header has always produced full URLs, not just paths. If you don't want that behavior, you can set Response.autocorrect_location_header = False, but I'm not sure what exactly that implies. I'm not sure why the tests were seeing path-only URLs before.

To debug this, I'd set a breakpoint in the if self.autocorrect_location_header block and see what it's doing differently in 1.x and 2.x.

[AdamWill]

yeah, I'm not saying that part actually is the problem, it's just the most obvious thing I saw on a brief look through the commit.

Most of the tests are of httpbin code that very explicitly intends to get relative URLs. redirect_to doesn't, in particular, but relative_redirect_n_times for instance has a bit of an obvious clue in the name, and does this:

response.headers["Location"] = url_for("relative_redirect_n_times", n=n - 1, _external=False)

where using _external=False is specifically ensuring it gets a relative path, not an absolute one. So httpbin is definitely specifically trying to get/return relative Locations in several of the tests, and previously was getting them, but after that commit it is ultimately producing absolute ones.

Your suggested next step was probably what I was gonna do next too, but I figured instead of flailing around blindly at "likely suspects" it'd be an idea to see if the person who wrote it could spot the issue :)

[AdamWill]

OK, so I added some debug prints and indeed the issue is autocorrect_location_header, somehow (I still don't have the how figured out). Before 2cd4fa9 , somehow, we do not enter the if self.autocorrect_location_header: block during the affected tests. After 2cd4fa9 , we do enter that block. Trying to figure out why now.

[AdamWill]

Ah. Heh. Turns out it's really pretty simple, and I dunno why I didn't notice it before...httpbin's core.py has this:

# Prevent WSGI from correcting the casing of the Location header
BaseResponse.autocorrect_location_header = False

which obviously stopped 'working' when the logic moved to Response. That should be fairly easy to fix.