Fix FileResponse sending empty chunked body on 304 #2144
Conversation
| assert 304 == resp.status | ||
| assert b'' == body |
There was a problem hiding this comment.
Hello @es1024 ,
According to what I read on the RFC:
- http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.3
- http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.5
The Content-Length header should not even be there. Can you add an assert to enforce it?
There was a problem hiding this comment.
Thx!
This habit of so many dev to always use .get() 😕 This is so much more readable (and slightly more correct):
assert "Content-Length" not in resp.headers(Don't bother though, not important)
aiohttp/web_response.py
Outdated
| @@ -362,7 +362,8 @@ def _start(self, request, | |||
| del headers[CONTENT_LENGTH] | |||
| elif self._length_check: | |||
| writer.length = self.content_length | |||
| if writer.length is None and version >= HttpVersion11: | |||
| if (writer.length is None and version >= HttpVersion11 and | |||
There was a problem hiding this comment.
I think fix should be applied to web_fileresponse.py instead of base StreamResponse class.
Codecov Report
@@ Coverage Diff @@
## 2.2 #2144 +/- ##
==========================================
+ Coverage 97.03% 97.03% +<.01%
==========================================
Files 38 38
Lines 7684 7685 +1
Branches 1341 1341
==========================================
+ Hits 7456 7457 +1
Misses 104 104
Partials 124 124
Continue to review full report at Codecov.
|
|
pretty sure |
| @@ -179,6 +179,7 @@ def prepare(self, request): | |||
| modsince = request.if_modified_since | |||
| if modsince is not None and st.st_mtime <= modsince.timestamp(): | |||
| self.set_status(HTTPNotModified.status_code) | |||
| self._length_check = False | |||
There was a problem hiding this comment.
This line looks hacky but I could live with it.
Looks like StreamResponse is too coupled with Response class, let's left it for other PR
|
@asvetlov can I do the merge? (I will drop the bin don't worry) |
|
@cecton please do |
|
Fixed by cc614ca |
What do these changes do?
Fixes a bug where FileResponse/add_static will include a non-empty body and use Transfer-Encoding: chunked for 304 responses.
Are there changes in behavior for the user?
FileResponse/add_static now won't give malformed 304 responses with bodies.
Related issue number
This fixes #2143.
Checklist
CONTRIBUTORS.txtchangesfolder<issue_id>.<type>for example (588.bug)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.