Work around ingress glitch with 304 responses#63355
Conversation
|
Hey there @home-assistant/supervisor, mind taking a look at this pull request as it has been labeled with an integration ( |
|
It seems a zero length chunk is indicative of end of stream. So it's expected to close the connection after this has been received. The client should re-open a new connection on the next request. I wonder what is getting confused by this closed connection. |
|
I think this is correct, we should however also add 204 "no content" responses. Both 304 and 204 are expected empty responses, that will "break" chunked encoding. |
|
As you say, closing the connection may well be correct behavior. (I got so caught up in the weeds I probably lost sight of that). Why it breaks Safari is a mystery -- perhaps there's some tiny out-of-spec implementation detail that can be found with more detailed inspection. All I can say is that Safari does something weird under these conditions. |
|
Tagged it for the milestone (although... it does affect Ingress, so we should be careful) |
Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>
| hdrs.CONTENT_LENGTH in result.headers | ||
| and int(result.headers.get(hdrs.CONTENT_LENGTH, 0)) < 4194000 | ||
| ): | ||
| ) or result.status in (204, 304): |
There was a problem hiding this comment.
This should use (HTTPStatus.NO_CONTENT, HTTPStatus.NOT_MODIFIED) and should have a test that proves that these 2 statuses return Response and not StreamResponse
Proposed change
This is a small workaround for an issue I noticed when using the ESPHome add-on (though it is not specific to that add-on). The ingress proxy rewrites "304 Not Modified" responses into chunked transfer-encoding, which triggers a series of weird browser behaviors. I don't know exactly why, but I wrote up the observed symptoms and in detail in esphome/issues#2908. This one-line change makes the problem go away.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: