Repository: falconry/falcon
Pull request #1481 (2019-03-15 22:25:54)

fix(API): Revert breaking change vs. 1.4 regarding forcing the Content-Length header

kgriffs: This patch reverts the 2.0 behavior to that of 1.4. I found this issue while testing a project that was relying on the Content-Length being overridden. It resulted in a lot of weird behavior by HAProxy / NGINX that is difficult to diagnose. 
codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=h1) Report
> Merging [#1481](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/401c699a37cc049df444ac151bcd2fd6996482ec?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1481/graphs/tree.svg?width=650&token=zOtkfNQ3h6&height=150&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1481   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          39      39           
  Lines        2613    2613           
  Branches      416     416           
======================================
  Hits         2613    2613
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/response.py](https://codecov.io/gh/falconry/falcon/pull/1481/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlLnB5) | `100% <ø> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=footer). Last update [401c699...1a326c4](https://codecov.io/gh/falconry/falcon/pull/1481?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

vytas7: What is even worse, reverse-proxies, front-end servers or HTTP clients might hang for a long time, or time-out waiting for "full" content if you set the `Content-Length` to a greater value than the actual content length being sent.

Patch:
diff --git a/falcon/api.py b/falcon/api.py
index 8bb89b3..287c4b3 100644
--- a/falcon/api.py
+++ b/falcon/api.py
@@ -318,7 +318,12 @@ class API(object):
 
             # PERF(kgriffs): Böse mußt sein. Operate directly on resp._headers
             #   to reduce overhead since this is a hot/critical code path.
-            if 'content-length' not in resp._headers and length is not None:
+            # NOTE(kgriffs): We always set content-length to match the
+            #   body bytes length, even if content-length is already set. The
+            #   reason being that web servers and LBs behave unpredictably
+            #   when the header doesn't match the body (sometimes choosing to
+            #   drop the HTTP connection prematurely, for example).
+            if length is not None:
                 resp._headers['content-length'] = str(length)
 
         headers = resp._wsgi_headers(media_type)
diff --git a/falcon/response.py b/falcon/response.py
index 5709821..eb25ce4 100644
--- a/falcon/response.py
+++ b/falcon/response.py
@@ -819,8 +819,12 @@ class Response(object):
         'Content-Length',
         """Set the Content-Length header.
 
-        Useful for responding to HEAD requests when you aren't actually
-        providing the response body.
+        This property can be used for responding to HEAD requests when you
+        aren't actually providing the response body, or when streaming the
+        response. If either the `body` property or the `data` property is set
+        on the response, the framework will force Content-Length to be the
+        length of the given body bytes. Therefore, it is only necessary to
+        manually set the content length when those properties are not used.
 
         Note:
             In cases where the response content is a stream (readable


Test patch:
diff --git a/tests/test_headers.py b/tests/test_headers.py
index a855f2b..97fc7d7 100644
--- a/tests/test_headers.py
+++ b/tests/test_headers.py
@@ -207,14 +207,23 @@ class RemoveHeaderResource(object):
 
 class ContentLengthHeaderResource(object):
 
-    def __init__(self, content_length, body=None):
+    def __init__(self, content_length, body=None, data=None):
         self._content_length = content_length
         self._body = body
+        self._data = data
 
     def on_get(self, req, resp):
         # NOTE(kgriffs): Use stream_len for now to cover the deprecated alias
         resp.stream_len = self._content_length
-        resp.body = self._body
+
+        if self._body:
+            resp.body = self._body
+
+        if self._data:
+            resp.data = self._data
+
+    def on_head(self, req, resp):
+        resp.content_length = self._content_length
 
 
 class ExpiresHeaderResource(object):
@@ -236,18 +245,31 @@ class TestHeaders(object):
         content_length = str(len(SAMPLE_BODY))
         assert result.headers['Content-Length'] == content_length
 
-    def test_declare_content_length(self, client):
+    def test_declared_content_length_on_head(self, client):
+        client.app.add_route('/', ContentLengthHeaderResource(42))
+        result = client.simulate_head()
+        assert result.headers['Content-Length'] == '42'
+
+    def test_declared_content_length_overridden_by_no_body(self, client):
         client.app.add_route('/', ContentLengthHeaderResource(42))
         result = client.simulate_get()
+        assert result.headers['Content-Length'] == '0'
 
-        assert result.headers['Content-Length'] == '42'
+    def test_declared_content_length_overriden_by_body_length(self, client):
+        resource = ContentLengthHeaderResource(42, body=SAMPLE_BODY)
+        client.app.add_route('/', resource)
+        result = client.simulate_get()
 
-    def test_declared_content_length_not_overriden_by_body_length(self, client):
-        resource = ContentLengthHeaderResource(42, body='Hello World')
+        assert result.headers['Content-Length'] == str(len(SAMPLE_BODY))
+
+    def test_declared_content_length_overriden_by_data_length(self, client):
+        data = SAMPLE_BODY.encode()
+
+        resource = ContentLengthHeaderResource(42, data=data)
         client.app.add_route('/', resource)
         result = client.simulate_get()
 
-        assert result.headers['Content-Length'] == '42'
+        assert result.headers['Content-Length'] == str(len(data))
 
     def test_expires_header(self, client):
         expires = datetime(2013, 1, 1, 10, 30, 30)