From 8f9707699f4809bac9e7c10484829ecd35de1b84 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Thu, 13 Jan 2022 11:55:42 -0800 Subject: [PATCH] Return a content-length on HTTP204 to prevent client failures It turns out that eventlet has been injecting a ``Transfer-Encoding`` header as of recent into WSGI application response headers. The result of this ultimately depends on how the HTTP client which is passing the request to the server is written to handle data. Apache, for example, will return that an invalid response was received. In part because it sees the request end, with an HTTP 204 response code, but also an encoding indicating there is a multipart body encoding inbound. Which is confusing. Other C based HTTP clients can have any number of reactions up to and including disconnecting sessions. Curl, depending on the headers present either returns success but notes body weirdness or actually returns return code 18. Python-Requests kind of has it a little worse, and we see this with clients. With it, it tries to prepare a respones content body based upon the presence of the header indicating there is a body. But it blows up thinking there is more data to read on the socket when there is not more data to read. Regardless, all of this is an RFC7230 violation. Neither Content-Length nor Transfer-Encoding should be on an HTTP 204 response. However, Content-Length is the lesser evil, and we have a similar endpoing in Ironic which *does* explicitly get returned with a zero length content-length, and does not demonstrate such issues. As such, in the interest of the lesser evils until Eventlet's evil ways of header injection are remedied, we're explicitly going to force a Content-Length header to be sent indicating a zero length response. For more information, please see: https://github.com/eventlet/eventlet/issues/746 Change-Id: I014cc65c79222f4d4d7c2b6ff11a76e56659340c (cherry picked from commit 55e47c630beb99f880045113e4f86bc1817b50da) --- ironic_inspector/main.py | 14 ++++++++++++++ ironic_inspector/test/unit/test_main.py | 6 ++++++ ...t_wsgi_evil_override-3905c6eef0ad7fa3.yaml | 19 +++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 releasenotes/notes/handle_eventlet_wsgi_evil_override-3905c6eef0ad7fa3.yaml diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index fa137d00c..8ce84ab5f 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -152,7 +152,21 @@ def _generate_empty_response(code): # be included for user friendly response bodies. if code == 204: response = flask.make_response('', code) + # NOTE(TheJulia): Explicitly set a content length to zero. + # as this is the *lesser* of all evils until + # https://github.com/eventlet/eventlet/issues/746 + # is resolved. + response.headers['Content-Length'] = 0 response.mimetype = 'text/plain' + + def _return_headers(cls): + # Dynamically re-maps over get_wsgi_headers() method in + # which werkzueg strips the content-length and sets us up + # for eventlet forcing in a Transfer-Encoding: chunked + # header. + return response.headers + + response.get_wsgi_headers = _return_headers else: # Send an empty dictionary to set a mimetype, and ultimately # with this being a rest API we can, at some point, choose to diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index e40b79226..c24204ef6 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -611,6 +611,12 @@ class TestApiRules(BaseAPITest): delete_mock.assert_called_once_with(self.uuid) self.assertEqual('text/plain; charset=utf-8', res.headers['content-type']) + # NOTE(TheJulia): This should be able to be removed once + # https://github.com/eventlet/eventlet/issues/746 + # is resolved and available. This ensures we don't + # send out a content length header out which is the + # less evil of the RFC7230 broken-ness. + self.assertEqual('0', res.headers['Content-Length']) class TestApiMisc(BaseAPITest): diff --git a/releasenotes/notes/handle_eventlet_wsgi_evil_override-3905c6eef0ad7fa3.yaml b/releasenotes/notes/handle_eventlet_wsgi_evil_override-3905c6eef0ad7fa3.yaml new file mode 100644 index 000000000..a356a16bf --- /dev/null +++ b/releasenotes/notes/handle_eventlet_wsgi_evil_override-3905c6eef0ad7fa3.yaml @@ -0,0 +1,19 @@ +--- +issues: + - | + The response headers for empty body HTTP 204 replies, at present, violate + RFC7230. This was not intentional, but underlying libraries also + make inappropriate changes to the headers, which can cause clients to + experience odd failures. This is anticipated to be corrected once an + underlying issue in + `eventlet `_ is resolved. +fixes: + - | + Fixes HTTP responses so the Eventlet library, which is used to support + the operation of the WSGI application, does not incorrectly inject + a ``Transfer-Encoding`` header into the HTTP response, even on HTTP 204 + replies, which is a violation of RFC7230. This header ultimately can + cause varying client reactions which are not expected and can raise + exceptions. For now, this has been remedied via an explicit return of + a ``Content-Length`` header, which is also an RFC7230 violation, but + it appears to be the lesser of known evils at this time.