-
-
Notifications
You must be signed in to change notification settings - Fork 16.2k
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
jsonify no longer causes Content-Length to be set on returned Response object #1877
Comments
This is unintended behavior. |
From a first look the best fix is to explicitly set content-length in jsonify. |
I do not have a good grasp of the Encoding Implications, but would it not help to just concat the newline onto the generated json string so we trigger the intended behaviour in After all setting somewhat "special" headers might introduce other undesired side-effects over time. |
I'm not sure about the performance implications of concatenating strings like that. We already are |
something like this could work (you know |
The point by @mbra was that if the behavior of Also we need to be careful here: |
ah damn.. json dumps still returns a unicode string at least in python3. not sure if it'll contain any non-ascii chars though. AFAIK it escapes them all but yeah, might be a bit fragile nonetheless edit: just checked the docs:
So I guess it'd be safe since it's documented behavior to always return plain ascii |
Well, I did not consider the performance penalty, the reallocations from the string concatenation could really be harmful, for large responses. Perhaps calling Also, the bad feeling I have about setting the Content-Length is that I do not know if and where a decision if made to use |
Mhh, after reading up on chunked Transfer and WSGI, this seems not to be an issue... |
Chunked encoding doesn't make much sense since we're not streaming. Content-Length is required by some clients, if it is suddenly missing I'm sure at least some code will break. |
The only alternative I could see involves StringIO, and calling @mitsuhiko Do you think this is breakage? |
Mhh, perhaps this should be handled by werkzeug. The docs for Perhaps I am missing something again, but calculating from a tuple should always be possible, right? |
Not sure when this got fixed, but I can't reproduce this. Examining the response from a client shows the correct content length. ( |
I am not sure if this was an intentional change, as it is only triggered by
jsonify
by passing an iterable toflask.wrappers.Response
/werkzeug.wrappers.Response
.In commit d9402fc a newline was added to the json response, as per a request from the httpbin project.
When
werkzeug.wrappers.Response
is passed a non text_type iterable, theself.set_data
codepath, responsible for setting the Content-Length header, is not used anymore.Ironcially this breaks httpbins "response-headers" route that uses the response object in a somewhat
creative way and breaks while trying to access
response.headers['Content-Length']
If this considered a backwards incompatible change that might need to be undone, or is this considered new and intended behaviour?
The text was updated successfully, but these errors were encountered: