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

Default Content-Type header re-applied when using test client #2450

Closed
Tom-Brouwer opened this issue Jul 8, 2022 · 2 comments · Fixed by #2459
Closed

Default Content-Type header re-applied when using test client #2450

Tom-Brouwer opened this issue Jul 8, 2022 · 2 comments · Fixed by #2459
Milestone

Comments

@Tom-Brouwer
Copy link

When using 'make_response' in a view function, default headers can be cleared, such as the default 'Content-Type' header. This works fine when using flask run, but when using the test client, the default 'Content-Type' header is re-applied, causing the test response to be different from the actual server response.

Current Behaviour

Given the following flask app (test_app.py):

from flask import Flask, make_response

app = Flask(__name__)


@app.route("/")
def hello_world():
    response = make_response("<p>Hello, World!</p>")
    response.headers.clear()
    return response


if __name__ == '__main__':
    client = app.test_client()
    response = client.get('/')
    print(response.headers)

When you do flask run in one terminal window, and curl in another, the output is as expected (No Content-Type header):

>> curl -i http://127.0.0.1:5000/
HTTP/1.1 200 OK
Server: Werkzeug/2.1.2 Python/3.10.5
Date: Fri, 08 Jul 2022 12:58:44 GMT
Content-Length: 20
Connection: close

When you run the application standalone, so the test client is triggered however, the default flask 'Content-Type' header is applied:

>> python test_app.py
Content-Length: 20
Content-Type: text/html; charset=utf-8

Expected behavior

I expect, if default headers are cleared in the view function, that they are not re-applied by the test client, especially since this doesn't match the actual response of the server.

Environment:

Python 3.10.5
Flask 2.1.2
Werkzeug 2.1.2

Cause

The test_client method initializes the FlaskClient class with self.response_class (flask.wrappers.Response), which in term passes this as a 'response_wrapper' to the werkzeug.test.Client __init__ method. Since flask.wrappers.Response.default_mimetype has the value seen above, this is always applied to a test_client response in case there's no 'Content-Type' header.

@davidism davidism transferred this issue from pallets/flask Jul 8, 2022
@davidism
Copy link
Member

davidism commented Jul 8, 2022

Seems like setting TestResponse.default_mimetype = None should do the trick, happy to review a PR. Note that this was moved to Werkzeug, which is where TestResponse is defined.

@davidism davidism added this to the 2.2.0 milestone Jul 8, 2022
@Tom-Brouwer
Copy link
Author

Tom-Brouwer commented Jul 15, 2022

@davidism I'm happy to submit a PR, but as far as I can see (On the latest 'main' branch for flask), the 'app.test_client' defined here still passes the Flask.Response as second argument to the FlaskClient, which in term passes the 'Flask.Response' as the second argument to werkzeug.test.Client, meaning the Flask.Response is the second base class when the 'WrapperTestResponse' is constructed, in term the default_mimetype from Flask.Response is used for the WrapperTestResponse (see here), or am I missing something?

If the above is the case, the fix should be inside the 'flask' repo, e.g. by changing line 1067 - 1069 in app.test_client() to:

return cls(  # type: ignore
    self, type('AppTestResponse', (self.response_class,), {'default_mimetype': None}), use_cookies=use_cookies, **kwargs
)

Please let me know your thoughts on this!

Note that this issue is about the way it's used by Flask. As you mention, the same issue may occur when using purely WerkZeug, and initializing the test.client with response_wrapper = None. In that case the TestResponse class from Werkzeug is used, which also has a default mimetype, but then set to text/plain, rather than the text/html that I'm seeing...

pgjones added a commit to pgjones/werkzeug that referenced this issue Jul 22, 2022
The TestResponse should have a mimetype as defined by the response
itself. If the response is missing a mimetype then it shouldn't assume
one.

See pallets#2450
pgjones added a commit to pgjones/werkzeug that referenced this issue Jul 22, 2022
The TestResponse should have a mimetype as defined by the response
itself. If the response is missing a mimetype then it shouldn't assume
one.

See pallets#2450
pgjones added a commit to pgjones/werkzeug that referenced this issue Jul 22, 2022
The TestResponse should have a mimetype as defined by the response
itself. If the response is missing a mimetype then it shouldn't assume
one.

See pallets#2450
pgjones added a commit to pgjones/werkzeug that referenced this issue Jul 22, 2022
The TestResponse should have a mimetype as defined by the response
itself. If the response is missing a mimetype then it shouldn't assume
one.

See pallets#2450
pgjones added a commit that referenced this issue Jul 23, 2022
The TestResponse should have a mimetype as defined by the response
itself. If the response is missing a mimetype then it shouldn't assume
one.

See #2450
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants