From 99e3c0d653e23d1af3e4236b88a747317e1c8a8a Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 23 Mar 2017 11:52:01 +0100 Subject: [PATCH 1/5] Set bodyproducer with empty content for POST --- scrapy/core/downloader/handlers/http11.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 37e836809..bff4a30c9 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -280,19 +280,7 @@ class ScrapyAgent(object): if request.body: bodyproducer = _RequestBodyProducer(request.body) else: - bodyproducer = None - # Setting Content-Length: 0 even for POST requests is not a - # MUST per HTTP RFCs, but it's common behavior, and some - # servers require this, otherwise returning HTTP 411 Length required - # - # RFC 7230#section-3.3.2: - # "a Content-Length header field is normally sent in a POST - # request even when the value is 0 (indicating an empty payload body)." - # - # Twisted Agent will not add "Content-Length: 0" by itself - if method == b'POST': - headers.addRawHeader(b'Content-Length', b'0') - + bodyproducer = _RequestBodyProducer(b'') if method == b'POST' else None start_time = time() d = agent.request( method, to_bytes(url, encoding='ascii'), headers, bodyproducer) From 97fc68fa1699b1c782c6e6d888e21d995bdf071d Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 24 Apr 2017 22:08:39 +0200 Subject: [PATCH 2/5] Refactor conditions on body producer --- scrapy/core/downloader/handlers/http11.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index bff4a30c9..46493f87f 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -279,8 +279,10 @@ class ScrapyAgent(object): headers.removeHeader(b'Proxy-Authorization') if request.body: bodyproducer = _RequestBodyProducer(request.body) + elif method == b'POST': + bodyproducer = _RequestBodyProducer(b'') else: - bodyproducer = _RequestBodyProducer(b'') if method == b'POST' else None + bodyproducer = None start_time = time() d = agent.request( method, to_bytes(url, encoding='ascii'), headers, bodyproducer) From b1a0a6e25810353f46d314cea1fd34bd37109b0c Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Tue, 25 Apr 2017 17:01:54 +0200 Subject: [PATCH 3/5] Make mockserver runnable outside of tox Add POST support for Echo resource --- tests/mockserver.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/mockserver.py b/tests/mockserver.py index 26ab51183..b95a6c3c4 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -15,7 +15,6 @@ from twisted.internet.task import deferLater from scrapy.utils.python import to_bytes, to_unicode -from tests import tests_datadir def getarg(request, name, default=None, type=None): @@ -122,6 +121,7 @@ class Echo(LeafResource): 'body': to_unicode(request.content.read()), } return to_bytes(json.dumps(output)) + render_POST = render_GET class RedirectTo(LeafResource): @@ -174,7 +174,11 @@ class Root(Resource): self.putChild(b"echo", Echo()) self.putChild(b"payload", PayloadResource()) self.putChild(b"xpayload", EncodingResourceWrapper(PayloadResource(), [GzipEncoderFactory()])) - self.putChild(b"files", File(os.path.join(tests_datadir, 'test_site/files/'))) + try: + from tests import tests_datadir + self.putChild(b"files", File(os.path.join(tests_datadir, 'test_site/files/'))) + except: + pass self.putChild(b"redirect-to", RedirectTo()) def getChild(self, name, request): From a63d9f502f50fbd948154fc65c8a94ffd4722d11 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Tue, 25 Apr 2017 17:03:03 +0200 Subject: [PATCH 4/5] Restore comments on why POST needs `Content-Length: 0` --- scrapy/core/downloader/handlers/http11.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 46493f87f..55bd31303 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -280,6 +280,18 @@ class ScrapyAgent(object): if request.body: bodyproducer = _RequestBodyProducer(request.body) elif method == b'POST': + # Setting Content-Length: 0 even for POST requests is not a + # MUST per HTTP RFCs, but it's common behavior, and some + # servers require this, otherwise returning HTTP 411 Length required + # + # RFC 7230#section-3.3.2: + # "a Content-Length header field is normally sent in a POST + # request even when the value is 0 (indicating an empty payload body)." + # + # Twisted < 17 will not add "Content-Length: 0" by itself; + # Twisted >= 17 fixes this; + # Using a producer with an empty-string sends `0` as Content-Length + # for all versions of Twisted. bodyproducer = _RequestBodyProducer(b'') else: bodyproducer = None From c3d0f9b6c10b68e436054ce9421d2ddadfc47087 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Tue, 25 Apr 2017 17:03:41 +0200 Subject: [PATCH 5/5] Add test for non-duplicated `Content-Length: 0` for bodyless POST --- tests/test_downloader_handlers.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 3efcf6e9c..0f28037ba 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -29,7 +29,7 @@ from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler from scrapy.core.downloader.handlers.s3 import S3DownloadHandler from scrapy.spiders import Spider -from scrapy.http import Request +from scrapy.http import Headers, Request from scrapy.http.response.text import TextResponse from scrapy.responsetypes import responsetypes from scrapy.settings import Settings @@ -37,7 +37,7 @@ from scrapy.utils.test import get_crawler, skip_if_no_boto from scrapy.utils.python import to_bytes from scrapy.exceptions import NotConfigured -from tests.mockserver import MockServer, ssl_context_factory +from tests.mockserver import MockServer, ssl_context_factory, Echo from tests.spiders import SingleRequestSpider class DummyDH(object): @@ -202,6 +202,7 @@ class HttpTestCase(unittest.TestCase): r.putChild(b"broken-chunked", BrokenChunkedResource()) r.putChild(b"contentlength", ContentLengthHeaderResource()) r.putChild(b"nocontenttype", EmptyContentTypeHeaderResource()) + r.putChild(b"echo", Echo()) self.site = server.Site(r, timeout=None) self.wrapper = WrappingFactory(self.site) self.host = 'localhost' @@ -310,6 +311,17 @@ class HttpTestCase(unittest.TestCase): request = Request(self.getURL('contentlength'), method='POST', headers={'Host': 'example.com'}) return self.download_request(request, Spider('foo')).addCallback(_test) + def test_content_length_zero_bodyless_post_only_one(self): + def _test(response): + import json + headers = Headers(json.loads(response.text)['headers']) + contentlengths = headers.getlist('Content-Length') + self.assertEquals(len(contentlengths), 1) + self.assertEquals(contentlengths, [b"0"]) + + request = Request(self.getURL('echo'), method='POST') + return self.download_request(request, Spider('foo')).addCallback(_test) + def test_payload(self): body = b'1'*100 # PayloadResource requires body length to be 100 request = Request(self.getURL('payload'), method='POST', body=body)