diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index 9b8ec6c77..7fb935f10 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -129,11 +129,11 @@ class H2ClientProtocol(Protocol): """ # Pop all streams which were pending and were not yet started for stream_id in list(self.streams): - self.streams[stream_id].close(StreamCloseReason.CONNECTION_LOST) + self.streams[stream_id].close(StreamCloseReason.CONNECTION_LOST, reason) self.conn.close_connection() - LOGGER.info("Connection lost with reason " + str(reason)) + LOGGER.warning("Connection lost with reason " + str(reason)) def _handle_events(self, events): """Private method which acts as a bridge between the events diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index 023f4f4eb..a26a33918 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -19,8 +19,15 @@ from scrapy.responsetypes import responsetypes class _ResponseTypedDict(TypedDict): + # Data received frame by frame from the server is appended + # and passed to the response Deferred when completely received. body: BytesIO + + # The amount of data received that counts against the flow control + # window flow_controlled_size: int + + # Headers received after sending the request headers: Headers @@ -40,6 +47,9 @@ class StreamCloseReason(IntFlag): # Expected response body size is more than allowed limit MAXSIZE_EXCEEDED = auto() + # When the response deferred is cancelled + CANCELLED = auto() + class Stream: """Represents a single HTTP/2 Stream. @@ -110,20 +120,16 @@ class Stream: # this response is then converted to appropriate Response class # passed to the response deferred callback self._response: _ResponseTypedDict = { - # Data received frame by frame from the server is appended - # and passed to the response Deferred when completely received. 'body': BytesIO(), - - # The amount of data received that counts against the flow control - # window 'flow_controlled_size': 0, - - # Headers received after sending the request 'headers': Headers({}) } - # TODO: Add canceller for the Deferred below - self._deferred_response = Deferred() + def _cancel(_): + # Close this stream as gracefully as possible :) + self.reset_stream(StreamCloseReason.CANCELLED) + + self._deferred_response = Deferred(_cancel) def __str__(self): return "Stream(id={})".format(repr(self.stream_id)) @@ -177,13 +183,6 @@ class Stream: and has initiated request already by sending HEADER frame. If not then stream will raise ProtocolError (raise by h2 state machine). """ - # TODO: - # 1. Add test for sending very large data - # 2. Add test for small data - # 3. Both (1) and (2) should be tested for - # 3.1 Large number of request - # 3.2 Small number of requests - if self.stream_closed_local: raise StreamClosedError(self.stream_id) @@ -221,7 +220,6 @@ class Stream: # End the stream if no more data needs to be send if self.remaining_content_length == 0: - self.stream_closed_local = True self._conn.end_stream(self.stream_id) # Write data to transport -- Empty the outstanding data @@ -288,7 +286,6 @@ class Stream: def reset_stream(self, reason=StreamCloseReason.RESET): """Close this stream by sending a RST_FRAME to the remote peer""" - # TODO: Q. REFUSED_STREAM or CANCEL ? if self.stream_closed_local: raise StreamClosedError(self.stream_id) @@ -305,14 +302,16 @@ class Stream: return expected_size != received_body_size - def close(self, reason: StreamCloseReason): + def close(self, reason: StreamCloseReason, failure=None): """Based on the reason sent we will handle each case. """ if self.stream_closed_server: raise StreamClosedError(self.stream_id) + self._cb_close(self.stream_id) self.stream_closed_server = True + # Do nothing if the response deferred was cancelled flags = None if b'Content-Length' not in self._response['headers']: # Missing Content-Length - PotentialDataLoss @@ -320,7 +319,6 @@ class Stream: elif self._is_data_lost(): if self._fail_on_dataloss: self._deferred_response.errback(ResponseFailed([Failure()])) - self._cb_close(self.stream_id) return else: flags = ['dataloss'] @@ -328,10 +326,19 @@ class Stream: if reason is StreamCloseReason.ENDED: self._fire_response_deferred(flags) - elif reason in (StreamCloseReason.RESET | StreamCloseReason.CONNECTION_LOST): - # Stream was abruptly ended here - self._deferred_response.errback(ResponseFailed([Failure()])) + # Stream was abruptly ended here + elif reason is StreamCloseReason.CANCELLED: + # Client has cancelled the request. Remove all the data + # received and fire the response deferred with no flags set + self._response['body'].truncate(0) + self._response['headers'].clear() + self._fire_response_deferred() + elif reason in (StreamCloseReason.RESET | StreamCloseReason.CONNECTION_LOST): + if failure is None: + self._deferred_response.errback(ResponseFailed([Failure()])) + else: + self._deferred_response.errback(failure) elif reason is StreamCloseReason.MAXSIZE_EXCEEDED: expected_size = int(self._response['headers'].get(b'Content-Length', -1)) error_msg = ("Cancelling download of {url}: expected response " @@ -345,22 +352,21 @@ class Stream: LOGGER.error(error_msg, error_args) self._deferred_response.errback(CancelledError(error_msg.format(**error_args))) - self._cb_close(self.stream_id) - def _fire_response_deferred(self, flags=None): """Builds response from the self._response dict and fires the response deferred callback with the generated response instance""" - # TODO: - # 1. Update Client Side Status Codes here + # TODO: Update Client Side Status Codes here + body = self._response['body'].getvalue() response_cls = responsetypes.from_args( headers=self._response['headers'], url=self._request.url, - body=self._response['body'] + body=body ) - # If there is no :status in headers then + # If there is no :status in headers + # (happens when client called response_deferred.cancel()) # HTTP Status Code: 499 - Client Closed Request status = self._response['headers'].get(':status', '499') @@ -368,7 +374,7 @@ class Stream: url=self._request.url, status=status, headers=self._response['headers'], - body=self._response['body'].getvalue(), + body=body, request=self._request, flags=flags, certificate=self._conn_metadata['certificate'], diff --git a/setup.py b/setup.py index d1470df5e..575c74e7f 100644 --- a/setup.py +++ b/setup.py @@ -67,7 +67,6 @@ setup( ], python_requires='>=3.5.2', install_requires=[ - 'Twisted>=17.9.0', 'Twisted[http2]>=17.9.0' 'cryptography>=2.0', 'cssselect>=0.9.1', diff --git a/tests/test_http2_client_protocol.py b/tests/test_http2_client_protocol.py index a67575d3c..0f3730c9e 100644 --- a/tests/test_http2_client_protocol.py +++ b/tests/test_http2_client_protocol.py @@ -1,50 +1,315 @@ -from urllib.parse import urlparse +# TODO: Add test cases for +# 1. No Content Length response header +# 2. Cancel Response Deferred +import json +import os +import random +import shutil +import string from twisted.internet import reactor -from twisted.internet.endpoints import connectProtocol, SSL4ClientEndpoint -from twisted.internet.ssl import CertificateOptions -from twisted.trial import unittest +from twisted.internet.defer import inlineCallbacks, DeferredList +from twisted.internet.endpoints import SSL4ClientEndpoint, SSL4ServerEndpoint, TCP4ServerEndpoint +from twisted.internet.protocol import Factory +from twisted.internet.ssl import optionsForClientTLS, PrivateCertificate +from twisted.trial.unittest import TestCase +from twisted.web.http import Request as TxRequest +from twisted.web.resource import Resource +from twisted.web.server import Site +from twisted.web.static import File from scrapy.core.http2.protocol import H2ClientProtocol -from scrapy.http import Request, Response -from tests.mockserver import MockServer +from scrapy.http import Request, Response, JsonRequest +from tests.mockserver import ssl_context_factory -class Http2ClientProtocolTestCase(unittest.TestCase): +def generate_random_string(size): + return ''.join(random.choices( + string.ascii_uppercase + string.digits, + k=size + )) + + +def make_html_body(val): + response = ''' +
{}
+'''.format(val) + return bytes(response, 'utf-8') + + +class Data: + SMALL_SIZE = 1024 * 10 # 10 KB + LARGE_SIZE = (1024 ** 2) * 10 # 10 MB + + STR_SMALL = generate_random_string(SMALL_SIZE) + STR_LARGE = generate_random_string(LARGE_SIZE) + + EXTRA_SMALL = generate_random_string(1024 * 15) + EXTRA_LARGE = generate_random_string((1024 ** 2) * 15) + + HTML_SMALL = make_html_body(STR_SMALL) + HTML_LARGE = make_html_body(STR_LARGE) + + JSON_SMALL = {'data': STR_SMALL} + JSON_LARGE = {'data': STR_LARGE} + + +class LeafResource(Resource): + isLeaf = True + + +class GetDataHtmlSmall(LeafResource): + def render_GET(self, request: TxRequest): + request.setHeader('Content-Type', 'text/html; charset=UTF-8') + return Data.HTML_SMALL + + +class GetDataHtmlLarge(LeafResource): + def render_GET(self, request: TxRequest): + request.setHeader('Content-Type', 'text/html; charset=UTF-8') + return Data.HTML_LARGE + + +class PostDataJsonMixin: + @staticmethod + def make_response(request: TxRequest, extra_data: str): + response = { + 'request-headers': {}, + 'request-body': json.loads(request.content.read()), + 'extra-data': extra_data + } + for k, v in request.requestHeaders.getAllRawHeaders(): + response['request-headers'][k.decode('utf-8')] = v[0].decode('utf-8') + + response_bytes = bytes(json.dumps(response), 'utf-8') + request.setHeader('Content-Type', 'application/json') + return response_bytes + + +class PostDataJsonSmall(LeafResource, PostDataJsonMixin): + def render_POST(self, request: TxRequest): + return self.make_response(request, Data.EXTRA_SMALL) + + +class PostDataJsonLarge(LeafResource, PostDataJsonMixin): + def render_POST(self, request: TxRequest): + return self.make_response(request, Data.EXTRA_LARGE) + + +def get_client_certificate(key_file, certificate_file): + with open(key_file, 'r') as key, open(certificate_file, 'r') as certificate: + pem = ''.join(key.readlines()) + ''.join(certificate.readlines()) + + return PrivateCertificate.loadPEM(pem) + + +class Https2ClientProtocolTestCase(TestCase): scheme = 'https' + key_file = os.path.join(os.path.dirname(__file__), 'keys', 'localhost.key') + certificate_file = os.path.join(os.path.dirname(__file__), 'keys', 'localhost.crt') + def _init_resource(self): + self.temp_directory = self.mktemp() + os.mkdir(self.temp_directory) + r = File(self.temp_directory) + r.putChild(b'get-data-html-small', GetDataHtmlSmall()) + r.putChild(b'get-data-html-large', GetDataHtmlLarge()) + + r.putChild(b'post-data-json-small', PostDataJsonSmall()) + r.putChild(b'post-data-json-large', PostDataJsonLarge()) + return r + + @inlineCallbacks def setUp(self): + # Initialize resource tree + root = self._init_resource() + self.site = Site(root, timeout=None) + # Start server for testing - self.mockserver = MockServer() - self.mockserver.__enter__() - + self.hostname = u'localhost' if self.scheme == 'https': - self.url = urlparse(self.mockserver.https_address) + context_factory = ssl_context_factory(self.key_file, self.certificate_file) + server_endpoint = SSL4ServerEndpoint(reactor, 0, context_factory, interface=self.hostname) else: - self.url = urlparse(self.mockserver.http_address) + server_endpoint = TCP4ServerEndpoint(reactor, 0, interface=self.hostname) + self.server = yield server_endpoint.listen(self.site) + self.port_number = self.server.getHost().port - self.protocol = H2ClientProtocol() - - # Connect to the server using the custom HTTP2ClientProtocol - options = CertificateOptions(acceptableProtocols=[b'h2']) - endpoint = SSL4ClientEndpoint(reactor, self.url.hostname, self.url.port, options) - connectProtocol(endpoint, self.protocol) - - def getURL(self, path): - return "{}://{}:{}/{}".format(self.url.scheme, self.url.hostname, self.url.port, path) + # Connect H2 client with server + client_certificate = get_client_certificate(self.key_file, self.certificate_file) + client_options = optionsForClientTLS( + hostname=self.hostname, + trustRoot=client_certificate, + acceptableProtocols=[b'h2'] + ) + h2_client_factory = Factory.forProtocol(H2ClientProtocol) + client_endpoint = SSL4ClientEndpoint(reactor, self.hostname, self.port_number, client_options) + self.client = yield client_endpoint.connect(h2_client_factory) + @inlineCallbacks def tearDown(self): - self.mockserver.__exit__(None, None, None) + yield self.client.transport.loseConnection() + yield self.client.transport.abortConnection() + yield self.server.stopListening() + shutil.rmtree(self.temp_directory) - def test_download(self): - request = Request(self.getURL('')) + def get_url(self, path): + """ + :param path: Should have / at the starting compulsorily if not empty + :return: Complete url + """ + assert len(path) > 0 and (path[0] == '/' or path[0] == '&') + return "{}://{}:{}{}".format(self.scheme, self.hostname, self.port_number, path) - def assert_response(response: Response): - self.assertEqual(response.body, b'Scrapy mock HTTP server\n') - self.assertEqual(response.status, 200) + @staticmethod + def _check_repeat(get_deferred, count): + d_list = [] + for _ in range(count): + d = get_deferred() + d_list.append(d) + + return DeferredList(d_list, fireOnOneErrback=True) + + def _check_GET( + self, + request: Request, + expected_body, + expected_status + ): + def check_response(response: Response): + self.assertEqual(response.status, expected_status) + self.assertEqual(response.body, expected_body) self.assertEqual(response.request, request) self.assertEqual(response.url, request.url) - d = self.protocol.request(request) + content_length = int(response.headers.get('Content-Length')) + self.assertEqual(len(response.body), content_length) + + d = self.client.request(request) + d.addCallback(check_response) + d.addErrback(self.fail) + return d + + def test_GET_small_body(self): + request = Request(self.get_url('/get-data-html-small')) + return self._check_GET(request, Data.HTML_SMALL, 200) + + def test_GET_large_body(self): + request = Request(self.get_url('/get-data-html-large')) + return self._check_GET(request, Data.HTML_LARGE, 200) + + def _check_GET_x20(self, *args, **kwargs): + def get_deferred(): + return self._check_GET(*args, **kwargs) + + return self._check_repeat(get_deferred, 20) + + def test_GET_small_body_x20(self): + return self._check_GET_x20( + Request(self.get_url('/get-data-html-small')), + Data.HTML_SMALL, + 200 + ) + + def test_GET_large_body_x20(self): + return self._check_GET_x20( + Request(self.get_url('/get-data-html-large')), + Data.HTML_LARGE, + 200 + ) + + def _check_POST_json( + self, + request: Request, + expected_request_body, + expected_extra_data, + expected_status: int + ): + d = self.client.request(request) + + def assert_response(response: Response): + self.assertEqual(response.status, expected_status) + self.assertEqual(response.request, request) + self.assertEqual(response.url, request.url) + + content_length = int(response.headers.get('Content-Length')) + self.assertEqual(len(response.body), content_length) + + # Parse the body + body = json.loads(response.body.decode('utf-8')) + self.assertIn('request-body', body) + self.assertIn('extra-data', body) + self.assertIn('request-headers', body) + + request_body = body['request-body'] + self.assertEqual(request_body, expected_request_body) + + extra_data = body['extra-data'] + self.assertEqual(extra_data, expected_extra_data) + + # Check if headers were sent successfully + request_headers = body['request-headers'] + for k, v in request.headers.items(): + k_str = k.decode('utf-8') + self.assertIn(k_str, request_headers) + self.assertEqual(request_headers[k_str], v[0].decode('utf-8')) + d.addCallback(assert_response) return d + + def test_POST_small_json(self): + request = JsonRequest(url=self.get_url('/post-data-json-small'), method='POST', data=Data.JSON_SMALL) + return self._check_POST_json( + request, + Data.JSON_SMALL, + Data.EXTRA_SMALL, + 200 + ) + + def test_POST_large_json(self): + request = JsonRequest(url=self.get_url('/post-data-json-large'), method='POST', data=Data.JSON_LARGE) + return self._check_POST_json( + request, + Data.JSON_LARGE, + Data.EXTRA_LARGE, + 200 + ) + + def _check_POST_json_x20(self, *args, **kwargs): + def get_deferred(): + return self._check_POST_json(*args, **kwargs) + + return self._check_repeat(get_deferred, 20) + + def test_POST_small_json_x20(self): + request = JsonRequest(url=self.get_url('/post-data-json-small'), method='POST', data=Data.JSON_SMALL) + return self._check_POST_json_x20( + request, + Data.JSON_SMALL, + Data.EXTRA_SMALL, + 200 + ) + + def test_POST_large_json_x20(self): + request = JsonRequest(url=self.get_url('/post-data-json-large'), method='POST', data=Data.JSON_LARGE) + return self._check_POST_json_x20( + request, + Data.JSON_LARGE, + Data.EXTRA_LARGE, + 200 + ) + + def test_cancel_request(self): + request = Request(url=self.get_url('/get-data-html-large')) + + def assert_response(response: Response): + self.assertEqual(response.status, 499) + self.assertEqual(response.request, request) + self.assertEqual(response.url, request.url) + + d = self.client.request(request) + d.addCallback(assert_response) + d.cancel() + + return d