Use Response instead of BaseResponse

[maximino-dev]

In the new werkzeug version released on 2022 - 03 - 28, the BaseResponse class has been removed (because it's deprecated) and replaced by Response. Using the Response class in the core.py file will now probably resolve incompatibility issues. Fixes #673

[moy]

You may want to add Fixes #673 somewhere in the commit message so that merging the PR fixes the bug.

With your patch, httpbin would have a strong dependency on werkzeug. I don't know what's httpbin policy on dependencies, but I guess you need to either:

• Update Pipfile.lock and Pipfile to reflect this new dependency
• or write conditional code so that httpbin works with both old and new versions (if Response is not found, import BaseResponse and do Response = BaseResponse)

[florimondmanca]

Agreed, we might want to use something like this:

try:
    from werkzeug.wrappers import Response
except ImportError:  # werkzeug < 2.1
    from werkzeug.wrappers import BaseResponse as Response

[maximino-dev]

The Response class is written even in the old werzeug versions, but in the version 1.x and older, there's not the autocorrect_location_header which is modified in httpbin, so there will be an error if we write this conditionnal code. Importing Response in any of the 2.x werkzeug versions will normally work. So I think that the best way is to update the pipfiles.

[hemberger]

I agree with @maximino-dev that updating the pipfiles to a >=2.0 constraint on werkzeug and exclusively using Response is probably the best progressive solution. However, if the higher priority is to retain compatibility with old versions of werkzeug, then what about this variation of @florimondmanca's suggestion:

try:
    from werkzeug.wrappers import BaseResponse as Response
except ImportError:  # werkzeug >= 2.1
    from werkzeug.wrappers import Response

This will use Response for werkzeug >= 2.1 and will use BaseResponse for < 2.1. In all cases, the class being imported will have the autocorrect_location_header property.

[moy]

FYI, Werkzeug 2.0 was released on May 11th, 2021 (i.e. not "very recent" but not so old either), so it may be nice to support older versions if there's a simple way to do it. No strong opinion here, and I leave it to the httpbin maintainers anyway to decide.

[yan12125]

However, if the higher priority is to retain compatibility with old versions of werkzeug, then what about this variation of @florimondmanca's suggestion:

As per https://github.com/postmanlabs/httpbin/pull/649, for werkzeug >= 2.0 (note that the boundary is not 2.1), autocorrect_location_header should be set on Response instead of BaseResponse. As a result, @florimondmanca's approach in https://github.com/postmanlabs/httpbin/pull/674#issuecomment-1088503584 is better.

[maximino-dev]

So we can maybe try something like this :

try:
        from werkzeug.wrappers import Response
        from werkzeug.wrappers import BaseResponse
except ImportError: # werkzeug >= 2.1.0
        from werkzeug.wrappers import Response

. . .

try:
        Response.autocorrect_location_header = False
except NameError: # werkzeug < 2.0.0
        BaseResponse.autocorrect_location_header = False

It imports Response and BaseResponse for werkzeug < 2.1.0 (otherwise just Response for werkzeug >= 2.1.0) and changes the autocorrect_location_header attribute in Response, or in BaseResponse if werkzeug < 2.0.0.

[hemberger]

I think @maximino-dev's solution covers all the bases. However, it seems a shame to encode the complicated history of werkzeug into this library. Perhaps we could just add a werkzeug 2.0+ requirement to this PR and call it a day?

[yan12125]

Perhaps we could just add a werkzeug 2.0+ requirement to this PR and call it a day?

Definitely fine for me! Anyway, that's up to postmanlabs developers. Maybe @MikeRalphson (reviewer of #649) can give some suggestions? Sorry for bothering if you no longer work on this.

[evilham]

Hey, FWIW this is the open issue on FreeBSD's ports tree: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263650 And this is the patch being applied:

 from werkzeug.datastructures import WWWAuthenticate, MultiDict
 from werkzeug.http import http_date
-from werkzeug.wrappers import BaseResponse
 from werkzeug.http import parse_authorization_header
 from raven.contrib.flask import Sentry
+# https://github.com/postmanlabs/httpbin/pull/674
+try:
+    from werkzeug.wrappers import BaseResponse as Response
+except ImportError: # werkzeug >= 2.1.0
+    from werkzeug.wrappers import Response
 
 from . import filters
 from .helpers import get_headers, status_code, get_dict, get_request_range, check_basic_auth, check_digest_auth, \
@@ -48,7 +52,7 @@ def jsonify(*args, **kwargs):
     return response
 
 # Prevent WSGI from correcting the casing of the Location header
-BaseResponse.autocorrect_location_header = False
+Response.autocorrect_location_header = False
 
 # Find the correct template folder when running from a different location
 tmpl_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'templates')

[AdamWill]

All of these versions of the patch ignore something rather important - core.py also does:

from flask import Flask, Response,...

and then later does stuff like return Response('Invalid status code', status=400), which it's clearly expecting to use flask's Response, not werkzeug's Response. We can't import werkzeug's thing as Response, that's a naming collision.

In my version downstream ATM I do this:

from werkzeug.wrappers import Response as WzResponse ... WzResponse.autocorrect_location_header = False

[hemberger]

Yes, the import will need to be aliased. @maximino-dev would you be willing to update your PR to address this point? Thanks!

[maximino-dev]

You're right, I updated it !

[moy]

The PR looks good to me, and takes all comments into account. I'm not familiar enough with the codebase to claim an official "approve" in a review, but I believe the PR can be merged. It's rather important since the tool is actually broken without this PR with the latest werkzeug.