Repository: scrapy/scrapy
Pull request #5062 (2021-03-22 14:01:20)

[HTTP/1.1] Skip Content-Length header if its value is UNKNOWN_LENGTH
I wasn't fast enough to comment this on #5057 before it got merged.

The following happens with the current `master` (ec5a7918ec2d8888ba356f9e3295dcd1ee935884):
```python
$ scrapy shell http://quotes.toscrape.com
(...)
2021-03-22 10:42:40 [scrapy.core.engine] INFO: Spider opened
resp2021-03-22 10:42:41 [scrapy.core.engine] DEBUG: Crawled (200) <GET http://quotes.toscrape.com> (referer: None)
onse.2021-03-22 10:42:41 [asyncio] DEBUG: Using selector: EpollSelector
[s] Available Scrapy objects:
[s]   scrapy     scrapy module (contains scrapy.Request, scrapy.Selector, etc)
[s]   crawler    <scrapy.crawler.Crawler object at 0x7f24d38fc790>
[s]   item       {}
[s]   request    <GET http://quotes.toscrape.com>
[s]   response   <200 http://quotes.toscrape.com>
[s]   settings   <scrapy.settings.Settings object at 0x7f24d38fcee0>
[s]   spider     <DefaultSpider 'default' at 0x7f24d2b792e0>
[s] Useful shortcuts:
[s]   fetch(url[, redirect=True]) Fetch URL and update local objects (by default, redirects are followed)
[s]   fetch(req)                  Fetch a scrapy.Request and update local objects 
[s]   shelp()           Shell help (print this help)
[s]   view(response)    View response in a browser
2021-03-22 10:42:41 [asyncio] DEBUG: Using selector: EpollSelector
In [1]: response.headers["Content-Length"]
Out[1]: b'twisted.web.iweb.UNKNOWN_LENGTH'
```

codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/5062?src=pr&el=h1) Report
> Merging [#5062](https://codecov.io/gh/scrapy/scrapy/pull/5062?src=pr&el=desc) (f8d27ba) into [master](https://codecov.io/gh/scrapy/scrapy/commit/ec5a7918ec2d8888ba356f9e3295dcd1ee935884?el=desc) (ec5a791) will **decrease** coverage by `0.45%`.
> The diff coverage is `100.00%`.


```diff
@@            Coverage Diff             @@
##           master    #5062      +/-   ##
==========================================
- Coverage   88.14%   87.69%   -0.46%     
==========================================
  Files         162      162              
  Lines       10297    10172     -125     
  Branches     1501     1498       -3     
==========================================
- Hits         9076     8920     -156     
- Misses        949      979      +30     
- Partials      272      273       +1     
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/5062?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/core/downloader/handlers/http11.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci9oYW5kbGVycy9odHRwMTEucHk=) | `92.92% <100.00%> (-0.76%)` | :arrow_down: |
| [scrapy/utils/asyncgen.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL2FzeW5jZ2VuLnB5) | `20.00% <0.00%> (-80.00%)` | :arrow_down: |
| [scrapy/robotstxt.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L3JvYm90c3R4dC5weQ==) | `73.33% <0.00%> (-24.20%)` | :arrow_down: |
| [scrapy/utils/spider.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL3NwaWRlci5weQ==) | `69.69% <0.00%> (-12.13%)` | :arrow_down: |
| [scrapy/utils/test.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL3Rlc3QucHk=) | `54.68% <0.00%> (-6.25%)` | :arrow_down: |
| [scrapy/utils/ssl.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL3NzbC5weQ==) | `65.85% <0.00%> (-4.88%)` | :arrow_down: |
| [scrapy/extensions/memdebug.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvbWVtZGVidWcucHk=) | `45.00% <0.00%> (-2.62%)` | :arrow_down: |
| [scrapy/extensions/debug.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvZGVidWcucHk=) | `44.73% <0.00%> (-1.42%)` | :arrow_down: |
| [scrapy/extensions/throttle.py](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvdGhyb3R0bGUucHk=) | `44.89% <0.00%> (-1.11%)` | :arrow_down: |
| ... and [64 more](https://codecov.io/gh/scrapy/scrapy/pull/5062/diff?src=pr&el=tree-more) | |


Patch:
diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst
index a0448c5ab..c0283df01 100644
--- a/docs/topics/request-response.rst
+++ b/docs/topics/request-response.rst
@@ -363,26 +363,26 @@ are some special keys recognized by Scrapy and its built-in extensions.
 
 Those are:
 
-* :reqmeta:`dont_redirect`
-* :reqmeta:`dont_retry`
-* :reqmeta:`handle_httpstatus_list`
-* :reqmeta:`handle_httpstatus_all`
-* :reqmeta:`dont_merge_cookies`
+* :reqmeta:`bindaddress`
 * :reqmeta:`cookiejar`
 * :reqmeta:`dont_cache`
-* :reqmeta:`redirect_reasons`
-* :reqmeta:`redirect_urls`
-* :reqmeta:`bindaddress`
+* :reqmeta:`dont_merge_cookies`
 * :reqmeta:`dont_obey_robotstxt`
-* :reqmeta:`download_timeout`
-* :reqmeta:`download_maxsize`
-* :reqmeta:`download_latency`
+* :reqmeta:`dont_redirect`
+* :reqmeta:`dont_retry`
 * :reqmeta:`download_fail_on_dataloss`
-* :reqmeta:`proxy`
-* ``ftp_user`` (See :setting:`FTP_USER` for more info)
+* :reqmeta:`download_latency`
+* :reqmeta:`download_maxsize`
+* :reqmeta:`download_timeout`
 * ``ftp_password`` (See :setting:`FTP_PASSWORD` for more info)
-* :reqmeta:`referrer_policy`
+* ``ftp_user`` (See :setting:`FTP_USER` for more info)
+* :reqmeta:`handle_httpstatus_all`
+* :reqmeta:`handle_httpstatus_list`
 * :reqmeta:`max_retry_times`
+* :reqmeta:`proxy`
+* :reqmeta:`redirect_reasons`
+* :reqmeta:`redirect_urls`
+* :reqmeta:`referrer_policy`
 
 .. reqmeta:: bindaddress
 
diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py
index 1f82751fd..25cb3ec62 100644
--- a/scrapy/core/downloader/handlers/http11.py
+++ b/scrapy/core/downloader/handlers/http11.py
@@ -361,10 +361,9 @@ class ScrapyAgent:
     @staticmethod
     def _headers_from_twisted_response(response):
         headers = Headers()
-        if response.length is not None:
+        if response.length != UNKNOWN_LENGTH:
             headers[b'Content-Length'] = str(response.length).encode()
-        for key, value in response.headers.getAllRawHeaders():
-            headers[key] = value
+        headers.update(response.headers.getAllRawHeaders())
         return headers
 
     def _cb_bodyready(self, txresponse, request):
diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py
index 081cd33f1..5c986eedc 100644
--- a/scrapy/utils/misc.py
+++ b/scrapy/utils/misc.py
@@ -9,7 +9,6 @@ from collections import deque
 from contextlib import contextmanager
 from importlib import import_module
 from pkgutil import iter_modules
-from textwrap import dedent
 
 from w3lib.html import replace_entities
 
@@ -227,7 +226,8 @@ def is_generator_with_return_value(callable):
         return value is None or isinstance(value, ast.NameConstant) and value.value is None
 
     if inspect.isgeneratorfunction(callable):
-        tree = ast.parse(dedent(inspect.getsource(callable)))
+        code = re.sub(r"^[\t ]+", "", inspect.getsource(callable))
+        tree = ast.parse(code)
         for node in walk_callable(tree):
             if isinstance(node, ast.Return) and not returns_none(node):
                 _generator_callbacks_cache[callable] = True
@@ -242,12 +242,23 @@ def warn_on_generator_with_return_value(spider, callable):
     Logs a warning if a callable is a generator function and includes
     a 'return' statement with a value different than None
     """
-    if is_generator_with_return_value(callable):
+    try:
+        if is_generator_with_return_value(callable):
+            warnings.warn(
+                f'The "{spider.__class__.__name__}.{callable.__name__}" method is '
+                'a generator and includes a "return" statement with a value '
+                'different than None. This could lead to unexpected behaviour. Please see '
+                'https://docs.python.org/3/reference/simple_stmts.html#the-return-statement '
+                'for details about the semantics of the "return" statement within generators',
+                stacklevel=2,
+            )
+    except IndentationError:
+        callable_name = spider.__class__.__name__ + "." + callable.__name__
         warnings.warn(
-            f'The "{spider.__class__.__name__}.{callable.__name__}" method is '
-            'a generator and includes a "return" statement with a value '
-            'different than None. This could lead to unexpected behaviour. Please see '
-            'https://docs.python.org/3/reference/simple_stmts.html#the-return-statement '
-            'for details about the semantics of the "return" statement within generators',
+            f'Unable to determine whether or not "{callable_name}" is a generator with a return value. '
+            'This will not prevent your code from working, but it prevents Scrapy from detecting '
+            f'potential issues in your implementation of "{callable_name}". Please, report this in the '
+            'Scrapy issue tracker (https://github.com/scrapy/scrapy/issues), '
+            f'including the code of "{callable_name}"',
             stacklevel=2,
         )


Test patch:
diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py
index 2be38620c..1c85ca353 100644
--- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py
+++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py
@@ -1,35 +1,116 @@
 import unittest
+import warnings
+from unittest import mock
 
-from scrapy.utils.misc import is_generator_with_return_value
+from scrapy.utils.misc import is_generator_with_return_value, warn_on_generator_with_return_value
+
+
+def _indentation_error(*args, **kwargs):
+    raise IndentationError()
+
+
+def top_level_return_something():
+    """
+docstring
+    """
+    url = """
+https://example.org
+"""
+    yield url
+    return 1
+
+
+def top_level_return_none():
+    """
+docstring
+    """
+    url = """
+https://example.org
+"""
+    yield url
+    return
+
+
+def generator_that_returns_stuff():
+    yield 1
+    yield 2
+    return 3
 
 
 class UtilsMiscPy3TestCase(unittest.TestCase):
 
-    def test_generators_with_return_statements(self):
-        def f():
+    def test_generators_return_something(self):
+        def f1():
             yield 1
             return 2
 
-        def g():
+        def g1():
+            yield 1
+            return "asdf"
+
+        def h1():
             yield 1
-            return 'asdf'
 
-        def h():
+            def helper():
+                return 0
+
+            yield helper()
+            return 2
+
+        def i1():
+            """
+docstring
+            """
+            url = """
+https://example.org
+        """
+            yield url
+            return 1
+
+        assert is_generator_with_return_value(top_level_return_something)
+        assert is_generator_with_return_value(f1)
+        assert is_generator_with_return_value(g1)
+        assert is_generator_with_return_value(h1)
+        assert is_generator_with_return_value(i1)
+
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, top_level_return_something)
+            self.assertEqual(len(w), 1)
+            self.assertIn('The "NoneType.top_level_return_something" method is a generator', str(w[0].message))
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, f1)
+            self.assertEqual(len(w), 1)
+            self.assertIn('The "NoneType.f1" method is a generator', str(w[0].message))
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, g1)
+            self.assertEqual(len(w), 1)
+            self.assertIn('The "NoneType.g1" method is a generator', str(w[0].message))
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, h1)
+            self.assertEqual(len(w), 1)
+            self.assertIn('The "NoneType.h1" method is a generator', str(w[0].message))
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, i1)
+            self.assertEqual(len(w), 1)
+            self.assertIn('The "NoneType.i1" method is a generator', str(w[0].message))
+
+    def test_generators_return_none(self):
+        def f2():
             yield 1
             return None
 
-        def i():
+        def g2():
             yield 1
             return
 
-        def j():
+        def h2():
             yield 1
 
-        def k():
+        def i2():
             yield 1
-            yield from g()
+            yield from generator_that_returns_stuff()
 
-        def m():
+        def j2():
             yield 1
 
             def helper():
@@ -37,20 +118,56 @@ class UtilsMiscPy3TestCase(unittest.TestCase):
 
             yield helper()
 
-        def n():
-            yield 1
-
-            def helper():
-                return 0
+        def k2():
+            """
+docstring
+            """
+            url = """
+https://example.org
+        """
+            yield url
+            return
 
-            yield helper()
-            return 2
+        def l2():
+            return
 
-        assert is_generator_with_return_value(f)
-        assert is_generator_with_return_value(g)
-        assert not is_generator_with_return_value(h)
-        assert not is_generator_with_return_value(i)
-        assert not is_generator_with_return_value(j)
-        assert not is_generator_with_return_value(k)  # not recursive
-        assert not is_generator_with_return_value(m)
-        assert is_generator_with_return_value(n)
+        assert not is_generator_with_return_value(top_level_return_none)
+        assert not is_generator_with_return_value(f2)
+        assert not is_generator_with_return_value(g2)
+        assert not is_generator_with_return_value(h2)
+        assert not is_generator_with_return_value(i2)
+        assert not is_generator_with_return_value(j2)  # not recursive
+        assert not is_generator_with_return_value(k2)  # not recursive
+        assert not is_generator_with_return_value(l2)
+
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, top_level_return_none)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, f2)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, g2)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, h2)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, i2)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, j2)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, k2)
+            self.assertEqual(len(w), 0)
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, l2)
+            self.assertEqual(len(w), 0)
+
+    @mock.patch("scrapy.utils.misc.is_generator_with_return_value", new=_indentation_error)
+    def test_indentation_error(self):
+        with warnings.catch_warnings(record=True) as w:
+            warn_on_generator_with_return_value(None, top_level_return_none)
+            self.assertEqual(len(w), 1)
+            self.assertIn('Unable to determine', str(w[0].message))