From 71c750ef9c3e53ef416d1de6e85458f16ca48d74 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Tue, 26 May 2015 22:06:20 +0200 Subject: [refactor] move http twisted code from soledad Implements an HTTP client the twisted way, with a focus on pinning the SSL certs. * Related: #6506 --- src/leap/common/http.py | 162 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 src/leap/common/http.py (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py new file mode 100644 index 0000000..39f01ba --- /dev/null +++ b/src/leap/common/http.py @@ -0,0 +1,162 @@ +# -*- coding: utf-8 -*- +# http.py +# Copyright (C) 2015 LEAP +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +""" +Twisted HTTP/HTTPS client. +""" + +import os + +from zope.interface import implements + +from OpenSSL.crypto import load_certificate +from OpenSSL.crypto import FILETYPE_PEM + +from twisted.internet import reactor +from twisted.internet.ssl import ClientContextFactory +from twisted.internet.ssl import CertificateOptions +from twisted.internet.defer import succeed + +from twisted.web.client import Agent +from twisted.web.client import HTTPConnectionPool +from twisted.web.client import readBody +from twisted.web.client import BrowserLikePolicyForHTTPS +from twisted.web.http_headers import Headers +from twisted.web.iweb import IBodyProducer + + +class HTTPClient(object): + """ + HTTP client done the twisted way, with a main focus on pinning the SSL + certificate. + """ + + def __init__(self, cert_file=None): + """ + Init the HTTP client + + :param cert_file: The path to the certificate file, if None given the + system's CAs will be used. + :type cert_file: str + """ + self._pool = HTTPConnectionPool(reactor, persistent=True) + self._pool.maxPersistentPerHost = 10 + + if cert_file: + cert = self._load_cert(cert_file) + self._agent = Agent( + reactor, + HTTPClient.ClientContextFactory(cert), + pool=self._pool) + else: + # trust the system's CAs + self._agent = Agent( + reactor, + BrowserLikePolicyForHTTPS(), + pool=self._pool) + + def _load_cert(self, cert_file): + """ + Load a X509 certificate from a file. + + :param cert_file: The path to the certificate file. + :type cert_file: str + + :return: The X509 certificate. + :rtype: OpenSSL.crypto.X509 + """ + if os.path.exists(cert_file): + with open(cert_file) as f: + data = f.read() + return load_certificate(FILETYPE_PEM, data) + + def request(self, url, method='GET', body=None, headers={}): + """ + Perform an HTTP request. + + :param url: The URL for the request. + :type url: str + :param method: The HTTP method of the request. + :type method: str + :param body: The body of the request, if any. + :type body: str + :param headers: The headers of the request. + :type headers: dict + + :return: A deferred that fires with the body of the request. + :rtype: twisted.internet.defer.Deferred + """ + if body: + body = HTTPClient.StringBodyProducer(body) + d = self._agent.request( + method, url, headers=Headers(headers), bodyProducer=body) + d.addCallback(readBody) + return d + + class ClientContextFactory(ClientContextFactory): + """ + A context factory that will verify the server's certificate against a + given CA certificate. + """ + + def __init__(self, cacert): + """ + Initialize the context factory. + + :param cacert: The CA certificate. + :type cacert: OpenSSL.crypto.X509 + """ + self._cacert = cacert + + def getContext(self, hostname, port): + opts = CertificateOptions(verify=True, caCerts=[self._cacert]) + return opts.getContext() + + class StringBodyProducer(object): + """ + A producer that writes the body of a request to a consumer. + """ + + implements(IBodyProducer) + + def __init__(self, body): + """ + Initialize the string produer. + + :param body: The body of the request. + :type body: str + """ + self.body = body + self.length = len(body) + + def startProducing(self, consumer): + """ + Write the body to the consumer. + + :param consumer: Any IConsumer provider. + :type consumer: twisted.internet.interfaces.IConsumer + + :return: A successful deferred. + :rtype: twisted.internet.defer.Deferred + """ + consumer.write(self.body) + return succeed(None) + + def pauseProducing(self): + pass + + def stopProducing(self): + pass -- cgit v1.2.3 From c071c69e1b5a0d897674a1f7adc6ff32f19400ff Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Wed, 27 May 2015 12:49:44 -0300 Subject: [bug] Use BrowserLikePolicyForHTTPS for checking While testing the way that its implemented now, I found out that no check is being made on certificate attributes against the host. I found this simple way of creating a BrowserLikePolicyForHTTPS using a self signed cert and it worked on my test. I used test_https from Soledad for checking this (which we are fixing on another branch). Also, we don't want to depend on twisted for other things than leap.common.http. --- src/leap/common/http.py | 70 +++++++++++++------------------------------------ 1 file changed, 18 insertions(+), 52 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 39f01ba..1dc5642 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -18,22 +18,28 @@ Twisted HTTP/HTTPS client. """ -import os +try: + import twisted +except ImportError: + print "*******" + print "Twisted is needed to use leap.common.http module" + print "" + print "Install the extra requirement of the package:" + print "$ pip install leap.common[Twisted]" + import sys + sys.exit(1) -from zope.interface import implements -from OpenSSL.crypto import load_certificate -from OpenSSL.crypto import FILETYPE_PEM +from leap.common.certs import get_compatible_ssl_context_factory + +from zope.interface import implements from twisted.internet import reactor -from twisted.internet.ssl import ClientContextFactory -from twisted.internet.ssl import CertificateOptions from twisted.internet.defer import succeed from twisted.web.client import Agent from twisted.web.client import HTTPConnectionPool from twisted.web.client import readBody -from twisted.web.client import BrowserLikePolicyForHTTPS from twisted.web.http_headers import Headers from twisted.web.iweb import IBodyProducer @@ -55,33 +61,12 @@ class HTTPClient(object): self._pool = HTTPConnectionPool(reactor, persistent=True) self._pool.maxPersistentPerHost = 10 - if cert_file: - cert = self._load_cert(cert_file) - self._agent = Agent( - reactor, - HTTPClient.ClientContextFactory(cert), - pool=self._pool) - else: - # trust the system's CAs - self._agent = Agent( - reactor, - BrowserLikePolicyForHTTPS(), - pool=self._pool) - - def _load_cert(self, cert_file): - """ - Load a X509 certificate from a file. - - :param cert_file: The path to the certificate file. - :type cert_file: str + policy = get_compatible_ssl_context_factory(cert_file) - :return: The X509 certificate. - :rtype: OpenSSL.crypto.X509 - """ - if os.path.exists(cert_file): - with open(cert_file) as f: - data = f.read() - return load_certificate(FILETYPE_PEM, data) + self._agent = Agent( + reactor, + policy, + pool=self._pool) def request(self, url, method='GET', body=None, headers={}): """ @@ -106,25 +91,6 @@ class HTTPClient(object): d.addCallback(readBody) return d - class ClientContextFactory(ClientContextFactory): - """ - A context factory that will verify the server's certificate against a - given CA certificate. - """ - - def __init__(self, cacert): - """ - Initialize the context factory. - - :param cacert: The CA certificate. - :type cacert: OpenSSL.crypto.X509 - """ - self._cacert = cacert - - def getContext(self, hostname, port): - opts = CertificateOptions(verify=True, caCerts=[self._cacert]) - return opts.getContext() - class StringBodyProducer(object): """ A producer that writes the body of a request to a consumer. -- cgit v1.2.3 From bf18c2bc6e3f533187281a3b31febd37ef22f8c0 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Wed, 27 May 2015 17:19:13 -0300 Subject: [feat] Make it optional to have a dedicated pool As @meskio pointed out, some cases could need a dedicated pool with different parameters. This is a suggested implementation where the pool is reused by default, creating a dedicated one just if needed/asked. This way we ensure that resources are under control and special cases are still handled. --- src/leap/common/http.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 1dc5642..1e384e5 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -44,13 +44,21 @@ from twisted.web.http_headers import Headers from twisted.web.iweb import IBodyProducer +def createPool(maxPersistentPerHost=10, persistent=True): + pool = HTTPConnectionPool(reactor, persistent) + pool.maxPersistentPerHost = maxPersistentPerHost + return pool + +_pool = createPool() + + class HTTPClient(object): """ HTTP client done the twisted way, with a main focus on pinning the SSL certificate. """ - def __init__(self, cert_file=None): + def __init__(self, cert_file=None, pool=_pool): """ Init the HTTP client @@ -58,15 +66,13 @@ class HTTPClient(object): system's CAs will be used. :type cert_file: str """ - self._pool = HTTPConnectionPool(reactor, persistent=True) - self._pool.maxPersistentPerHost = 10 policy = get_compatible_ssl_context_factory(cert_file) self._agent = Agent( reactor, policy, - pool=self._pool) + pool=pool) def request(self, url, method='GET', body=None, headers={}): """ -- cgit v1.2.3 From b67648aa666345b0800b48f6c203538b21c9a201 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Mon, 8 Jun 2015 15:18:00 -0300 Subject: [bug] Makes request method respect a hard limit Altough we specify maxPersistentPerHost, Twisted won't stop opening connections after that. This limit is used just to keep the size of persistent connections pool under control. Additional connections will be made as non persistent. So, if we ask 10000 requests, it will open 10000 connections immediately and leave 10 open after all finished. For checking this behavior, see getConnection from Twisted source: http://twistedmatrix.com/trac/browser/tags/releases/twisted-15.2.1/twisted/web/client.py#L1203 I tested this by using http_target from soledad without a local database to download all encrypted docs from one account with 1700 of them. The program just hangs and crashes with 1000+ connections and "Too many files open" warnings. With this fix, it was able to download normally, respecting the maxPersistentPerHost as a limiter. :) --- src/leap/common/http.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 1e384e5..d4a214c 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -35,6 +35,7 @@ from leap.common.certs import get_compatible_ssl_context_factory from zope.interface import implements from twisted.internet import reactor +from twisted.internet import defer from twisted.internet.defer import succeed from twisted.web.client import Agent @@ -56,6 +57,13 @@ class HTTPClient(object): """ HTTP client done the twisted way, with a main focus on pinning the SSL certificate. + + By default, it uses a shared connection pool. If you want a dedicated + one, create and pass on __init__ pool parameter. + Please note that this client will limit the maximum amount of connections + by using a DeferredSemaphore. + This limit is equal to the maxPersistentPerHost used on pool and is needed + in order to avoid resource abuse on huge requests batches. """ def __init__(self, cert_file=None, pool=_pool): @@ -65,6 +73,9 @@ class HTTPClient(object): :param cert_file: The path to the certificate file, if None given the system's CAs will be used. :type cert_file: str + :param pool: An optional dedicated connection pool to override the + default shared one. + :type pool: HTTPConnectionPool """ policy = get_compatible_ssl_context_factory(cert_file) @@ -73,6 +84,7 @@ class HTTPClient(object): reactor, policy, pool=pool) + self._semaphore = defer.DeferredSemaphore(pool.maxPersistentPerHost) def request(self, url, method='GET', body=None, headers={}): """ @@ -92,8 +104,9 @@ class HTTPClient(object): """ if body: body = HTTPClient.StringBodyProducer(body) - d = self._agent.request( - method, url, headers=Headers(headers), bodyProducer=body) + d = self._semaphore.run(self._agent.request, + method, url, headers=Headers(headers), + bodyProducer=body) d.addCallback(readBody) return d -- cgit v1.2.3 From 46870f1e26ef159bf6fe4744aba3b00a5e81cc3b Mon Sep 17 00:00:00 2001 From: drebs Date: Wed, 8 Jul 2015 19:14:00 -0300 Subject: [feat] add close method for http agent The ability to close cached connections is needed in order to have a clean reactor when the program ends. --- src/leap/common/http.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index d4a214c..8d22f2c 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -80,6 +80,7 @@ class HTTPClient(object): policy = get_compatible_ssl_context_factory(cert_file) + self._pool = pool self._agent = Agent( reactor, policy, @@ -110,6 +111,12 @@ class HTTPClient(object): d.addCallback(readBody) return d + def close(self): + """ + Close any cached connections. + """ + self._pool.closeCachedConnections() + class StringBodyProducer(object): """ A producer that writes the body of a request to a consumer. -- cgit v1.2.3 From 34f6b07e08540fd9b2ec473d1e4e5a15be4feacc Mon Sep 17 00:00:00 2001 From: drebs Date: Wed, 8 Jul 2015 19:15:56 -0300 Subject: [bug] add http request timeout The connectTimeout parameter of twisted.web.client.Agent only acts on the connection setup, and the Agent will wait forever for incoming data after the connection has been established. This commit adds a timeout for the connection, and will cancel the deferred if the result has not been received after a certain number of seconds. --- src/leap/common/http.py | 260 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 214 insertions(+), 46 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 8d22f2c..c93e65b 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -36,21 +36,24 @@ from zope.interface import implements from twisted.internet import reactor from twisted.internet import defer -from twisted.internet.defer import succeed +from twisted.python import failure from twisted.web.client import Agent from twisted.web.client import HTTPConnectionPool +from twisted.web.client import _HTTP11ClientFactory as HTTP11ClientFactory from twisted.web.client import readBody from twisted.web.http_headers import Headers from twisted.web.iweb import IBodyProducer +from twisted.web._newclient import HTTP11ClientProtocol -def createPool(maxPersistentPerHost=10, persistent=True): - pool = HTTPConnectionPool(reactor, persistent) - pool.maxPersistentPerHost = maxPersistentPerHost - return pool +__all__ = ["HTTPClient"] -_pool = createPool() + +# A default HTTP timeout is used for 2 distinct purposes: +# 1. as HTTP connection timeout, prior to connection estabilshment. +# 2. as data reception timeout, after the connection has been established. +DEFAULT_HTTP_TIMEOUT = 30 # seconds class HTTPClient(object): @@ -66,28 +69,36 @@ class HTTPClient(object): in order to avoid resource abuse on huge requests batches. """ - def __init__(self, cert_file=None, pool=_pool): + def __init__(self, cert_file=None, timeout=DEFAULT_HTTP_TIMEOUT): """ Init the HTTP client :param cert_file: The path to the certificate file, if None given the system's CAs will be used. :type cert_file: str - :param pool: An optional dedicated connection pool to override the - default shared one. - :type pool: HTTPConnectionPool + :param timeout: The amount of time that this Agent will wait for the + peer to accept a connection and for each request to be + finished. If a pool is passed, then this argument is + ignored. + :type timeout: float """ - policy = get_compatible_ssl_context_factory(cert_file) - - self._pool = pool + self._timeout = timeout + self._pool = self._createPool() self._agent = Agent( reactor, - policy, - pool=pool) - self._semaphore = defer.DeferredSemaphore(pool.maxPersistentPerHost) + get_compatible_ssl_context_factory(cert_file), + pool=self._pool, + connectTimeout=self._timeout) + self._semaphore = defer.DeferredSemaphore( + self._pool.maxPersistentPerHost) - def request(self, url, method='GET', body=None, headers={}): + def _createPool(self, maxPersistentPerHost=10, persistent=True): + pool = _HTTPConnectionPool(reactor, persistent, self._timeout) + pool.maxPersistentPerHost = maxPersistentPerHost + return pool + + def _request(self, url, method, body, headers): """ Perform an HTTP request. @@ -104,51 +115,208 @@ class HTTPClient(object): :rtype: twisted.internet.defer.Deferred """ if body: - body = HTTPClient.StringBodyProducer(body) - d = self._semaphore.run(self._agent.request, - method, url, headers=Headers(headers), - bodyProducer=body) + body = _StringBodyProducer(body) + d = self._agent.request( + method, url, headers=Headers(headers), bodyProducer=body) d.addCallback(readBody) return d + def request(self, url, method='GET', body=None, headers={}): + """ + Perform an HTTP request, but limit the maximum amount of concurrent + connections. + + :param url: The URL for the request. + :type url: str + :param method: The HTTP method of the request. + :type method: str + :param body: The body of the request, if any. + :type body: str + :param headers: The headers of the request. + :type headers: dict + + :return: A deferred that fires with the body of the request. + :rtype: twisted.internet.defer.Deferred + """ + return self._semaphore.run(self._request, url, method, body, headers) + def close(self): """ Close any cached connections. """ self._pool.closeCachedConnections() - class StringBodyProducer(object): +# +# An IBodyProducer to write the body of an HTTP request as a string. +# + +class _StringBodyProducer(object): + """ + A producer that writes the body of a request to a consumer. + """ + + implements(IBodyProducer) + + def __init__(self, body): + """ + Initialize the string produer. + + :param body: The body of the request. + :type body: str + """ + self.body = body + self.length = len(body) + + def startProducing(self, consumer): + """ + Write the body to the consumer. + + :param consumer: Any IConsumer provider. + :type consumer: twisted.internet.interfaces.IConsumer + + :return: A successful deferred. + :rtype: twisted.internet.defer.Deferred + """ + consumer.write(self.body) + return defer.succeed(None) + + def pauseProducing(self): + pass + + def stopProducing(self): + pass + + +# +# Patched twisted.web classes +# + +class _HTTP11ClientProtocol(HTTP11ClientProtocol): + """ + A timeout-able HTTP 1.1 client protocol, that is instantiated by the + _HTTP11ClientFactory below. + """ + + def __init__(self, quiescentCallback, timeout): + """ + Initialize the protocol. + + :param quiescentCallback: + :type quiescentCallback: callable + :param timeout: A timeout, in seconds, for requests made by this + protocol. + :type timeout: float + """ + HTTP11ClientProtocol.__init__(self, quiescentCallback) + self._timeout = timeout + self._timeoutCall = None + + def request(self, request): + """ + Issue request over self.transport and return a Deferred which + will fire with a Response instance or an error. + + :param request: The object defining the parameters of the request to + issue. + :type request: twisted.web._newclient.Request + + :return: A deferred which fires after the request has finished. + :rtype: Deferred + """ + d = HTTP11ClientProtocol.request(self, request) + if self._timeout: + self._last_buffer_len = 0 + timeoutCall = reactor.callLater( + self._timeout, self._doTimeout, request) + self._timeoutCall = timeoutCall + return d + + def _doTimeout(self, request): """ - A producer that writes the body of a request to a consumer. + Give up the request because of a timeout. + + :param request: The object defining the parameters of the request to + issue. + :type request: twisted.web._newclient.Request + """ + self._giveUp( + failure.Failure( + defer.TimeoutError( + "Getting %s took longer than %s seconds." + % (request.absoluteURI, self._timeout)))) + + def _cancelTimeout(self): """ + Cancel the request timeout, when it's finished. + """ + if self._timeoutCall.active(): + self._timeoutCall.cancel() + self._timeoutCall = None + + def _finishResponse_WAITING(self, rest): + """ + Cancel the timeout when finished receiving the response. + """ + self._cancelTimeout() + HTTP11ClientProtocol._finishResponse_WAITING(self, rest) - implements(IBodyProducer) + def _finishResponse_TRANSMITTING(self, rest): + """ + Cancel the timeout when finished receiving the response. + """ + self._cancelTimeout() + HTTP11ClientProtocol._finishResponse_TRANSMITTING(self, rest) - def __init__(self, body): - """ - Initialize the string produer. + def dataReceived(self, bytes): + """ + Receive some data and extend the timeout period of this request. + + :param bytes: A string of indeterminate length. + :type bytes: str + """ + HTTP11ClientProtocol.dataReceived(self, bytes) + if self._timeoutCall and self._timeoutCall.active(): + self._timeoutCall.reset(self._timeout) - :param body: The body of the request. - :type body: str - """ - self.body = body - self.length = len(body) - def startProducing(self, consumer): - """ - Write the body to the consumer. +class _HTTP11ClientFactory(HTTP11ClientFactory): + """ + A timeout-able HTTP 1.1 client protocol factory. + """ - :param consumer: Any IConsumer provider. - :type consumer: twisted.internet.interfaces.IConsumer + def __init__(self, quiescentCallback, timeout): + """ + :param quiescentCallback: The quiescent callback to be passed to + protocol instances, used to return them to + the connection pool. + :type quiescentCallback: callable(Protocol) + :param timeout: The timeout, in seconds, for requests made by + protocols created by this factory. + :type timeout: float + """ + HTTP11ClientFactory.__init__(self, quiescentCallback) + self._timeout = timeout + + def buildProtocol(self, _): + """ + Build the HTTP 1.1 client protocol. + """ + return _HTTP11ClientProtocol(self._quiescentCallback, self._timeout) + + +class _HTTPConnectionPool(HTTPConnectionPool): + """ + A timeout-able HTTP connection pool. + """ - :return: A successful deferred. - :rtype: twisted.internet.defer.Deferred - """ - consumer.write(self.body) - return succeed(None) + _factory = _HTTP11ClientFactory - def pauseProducing(self): - pass + def __init__(self, reactor, persistent, timeout): + HTTPConnectionPool.__init__(self, reactor, persistent=persistent) + self._timeout = timeout - def stopProducing(self): - pass + def _newConnection(self, key, endpoint): + def quiescentCallback(protocol): + self._putConnection(key, protocol) + factory = self._factory(quiescentCallback, timeout=self._timeout) + return endpoint.connect(factory) -- cgit v1.2.3 From 25d3b7c80f85cd94159b574274108061a94f1bc9 Mon Sep 17 00:00:00 2001 From: Bruno Wagner Date: Wed, 22 Jul 2015 15:38:56 -0300 Subject: [style] Fixed pep8 warnings --- src/leap/common/http.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index c93e65b..56938b4 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -150,6 +150,7 @@ class HTTPClient(object): # An IBodyProducer to write the body of an HTTP request as a string. # + class _StringBodyProducer(object): """ A producer that writes the body of a request to a consumer. @@ -254,18 +255,18 @@ class _HTTP11ClientProtocol(HTTP11ClientProtocol): self._timeoutCall = None def _finishResponse_WAITING(self, rest): - """ - Cancel the timeout when finished receiving the response. - """ - self._cancelTimeout() - HTTP11ClientProtocol._finishResponse_WAITING(self, rest) + """ + Cancel the timeout when finished receiving the response. + """ + self._cancelTimeout() + HTTP11ClientProtocol._finishResponse_WAITING(self, rest) def _finishResponse_TRANSMITTING(self, rest): - """ - Cancel the timeout when finished receiving the response. - """ - self._cancelTimeout() - HTTP11ClientProtocol._finishResponse_TRANSMITTING(self, rest) + """ + Cancel the timeout when finished receiving the response. + """ + self._cancelTimeout() + HTTP11ClientProtocol._finishResponse_TRANSMITTING(self, rest) def dataReceived(self, bytes): """ -- cgit v1.2.3 From 5502318835626527d9818c360c7fd2b1a4b01145 Mon Sep 17 00:00:00 2001 From: Bruno Wagner Date: Wed, 22 Jul 2015 23:55:39 -0300 Subject: [tests] implemented http feature according to test Two test cases were broken and were implemented here: The first was that HTTPClient should share the connection between clients if a pool was not passed explicitly. If you initialize an HTTPClient without a pool, it will reuse a pool created on the class. The second was that you should be able to pass to the HTTPCLient a pool on initialization. Added that possibility and fixed the tests accordingly --- src/leap/common/http.py | 98 ++++++++++++++++++++++++++----------------------- 1 file changed, 53 insertions(+), 45 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 56938b4..f67507d 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -56,6 +56,50 @@ __all__ = ["HTTPClient"] DEFAULT_HTTP_TIMEOUT = 30 # seconds +class _HTTP11ClientFactory(HTTP11ClientFactory): + """ + A timeout-able HTTP 1.1 client protocol factory. + """ + + def __init__(self, quiescentCallback, timeout): + """ + :param quiescentCallback: The quiescent callback to be passed to + protocol instances, used to return them to + the connection pool. + :type quiescentCallback: callable(Protocol) + :param timeout: The timeout, in seconds, for requests made by + protocols created by this factory. + :type timeout: float + """ + HTTP11ClientFactory.__init__(self, quiescentCallback) + self._timeout = timeout + + def buildProtocol(self, _): + """ + Build the HTTP 1.1 client protocol. + """ + return _HTTP11ClientProtocol(self._quiescentCallback, self._timeout) + + +class _HTTPConnectionPool(HTTPConnectionPool): + """ + A timeout-able HTTP connection pool. + """ + + _factory = _HTTP11ClientFactory + + def __init__(self, reactor, persistent, timeout, maxPersistentPerHost=10): + HTTPConnectionPool.__init__(self, reactor, persistent=persistent) + self.maxPersistentPerHost = maxPersistentPerHost + self._timeout = timeout + + def _newConnection(self, key, endpoint): + def quiescentCallback(protocol): + self._putConnection(key, protocol) + factory = self._factory(quiescentCallback, timeout=self._timeout) + return endpoint.connect(factory) + + class HTTPClient(object): """ HTTP client done the twisted way, with a main focus on pinning the SSL @@ -69,7 +113,14 @@ class HTTPClient(object): in order to avoid resource abuse on huge requests batches. """ - def __init__(self, cert_file=None, timeout=DEFAULT_HTTP_TIMEOUT): + _pool = _HTTPConnectionPool( + reactor, + persistent=True, + timeout = DEFAULT_HTTP_TIMEOUT, + maxPersistentPerHost=10 + ) + + def __init__(self, cert_file=None, timeout=DEFAULT_HTTP_TIMEOUT, pool=None): """ Init the HTTP client @@ -84,7 +135,7 @@ class HTTPClient(object): """ self._timeout = timeout - self._pool = self._createPool() + self._pool = pool if pool is not None else self._pool self._agent = Agent( reactor, get_compatible_ssl_context_factory(cert_file), @@ -278,46 +329,3 @@ class _HTTP11ClientProtocol(HTTP11ClientProtocol): HTTP11ClientProtocol.dataReceived(self, bytes) if self._timeoutCall and self._timeoutCall.active(): self._timeoutCall.reset(self._timeout) - - -class _HTTP11ClientFactory(HTTP11ClientFactory): - """ - A timeout-able HTTP 1.1 client protocol factory. - """ - - def __init__(self, quiescentCallback, timeout): - """ - :param quiescentCallback: The quiescent callback to be passed to - protocol instances, used to return them to - the connection pool. - :type quiescentCallback: callable(Protocol) - :param timeout: The timeout, in seconds, for requests made by - protocols created by this factory. - :type timeout: float - """ - HTTP11ClientFactory.__init__(self, quiescentCallback) - self._timeout = timeout - - def buildProtocol(self, _): - """ - Build the HTTP 1.1 client protocol. - """ - return _HTTP11ClientProtocol(self._quiescentCallback, self._timeout) - - -class _HTTPConnectionPool(HTTPConnectionPool): - """ - A timeout-able HTTP connection pool. - """ - - _factory = _HTTP11ClientFactory - - def __init__(self, reactor, persistent, timeout): - HTTPConnectionPool.__init__(self, reactor, persistent=persistent) - self._timeout = timeout - - def _newConnection(self, key, endpoint): - def quiescentCallback(protocol): - self._putConnection(key, protocol) - factory = self._factory(quiescentCallback, timeout=self._timeout) - return endpoint.connect(factory) -- cgit v1.2.3 From e8d54bd3dc4b5f0327a30c0a6848dd832beb7da0 Mon Sep 17 00:00:00 2001 From: Bruno Wagner Date: Thu, 23 Jul 2015 00:09:03 -0300 Subject: [style] fixed pep8 warnings on http and test events --- src/leap/common/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index f67507d..8e8d3d9 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -116,7 +116,7 @@ class HTTPClient(object): _pool = _HTTPConnectionPool( reactor, persistent=True, - timeout = DEFAULT_HTTP_TIMEOUT, + timeout=DEFAULT_HTTP_TIMEOUT, maxPersistentPerHost=10 ) -- cgit v1.2.3 From a119dd4fa2fc4a14577fd2d6e32dff950d934193 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Tue, 28 Jul 2015 11:41:32 -0400 Subject: [style] more pep8 cleanup --- src/leap/common/http.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 8e8d3d9..1e7ded7 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -120,7 +120,8 @@ class HTTPClient(object): maxPersistentPerHost=10 ) - def __init__(self, cert_file=None, timeout=DEFAULT_HTTP_TIMEOUT, pool=None): + def __init__(self, cert_file=None, + timeout=DEFAULT_HTTP_TIMEOUT, pool=None): """ Init the HTTP client -- cgit v1.2.3 From 0a8f455965fff778d993912f1dc11a77db264a93 Mon Sep 17 00:00:00 2001 From: Bruno Wagner Date: Tue, 4 Aug 2015 18:44:21 -0300 Subject: [bug] HTTP timeout was not being cleared on abort In case the http client loses connection, it has to clear it's timeout or the reactor will be left in a dirty state Fixing this solves a problem with some of the tests in Soledad that were trying to run on a dirty reactor --- src/leap/common/http.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 1e7ded7..cda8ee8 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -302,23 +302,16 @@ class _HTTP11ClientProtocol(HTTP11ClientProtocol): """ Cancel the request timeout, when it's finished. """ - if self._timeoutCall.active(): + if self._timeoutCall and self._timeoutCall.active(): self._timeoutCall.cancel() self._timeoutCall = None - def _finishResponse_WAITING(self, rest): - """ - Cancel the timeout when finished receiving the response. - """ - self._cancelTimeout() - HTTP11ClientProtocol._finishResponse_WAITING(self, rest) - - def _finishResponse_TRANSMITTING(self, rest): + def _finishResponse(self, rest): """ Cancel the timeout when finished receiving the response. """ self._cancelTimeout() - HTTP11ClientProtocol._finishResponse_TRANSMITTING(self, rest) + HTTP11ClientProtocol._finishResponse(self, rest) def dataReceived(self, bytes): """ @@ -330,3 +323,7 @@ class _HTTP11ClientProtocol(HTTP11ClientProtocol): HTTP11ClientProtocol.dataReceived(self, bytes) if self._timeoutCall and self._timeoutCall.active(): self._timeoutCall.reset(self._timeout) + + def connectionLost(self, reason): + self._cancelTimeout() + return HTTP11ClientProtocol.connectionLost(self, reason) -- cgit v1.2.3 From 8965be42773e3e6c7e154f2c1a27f4e34031ae91 Mon Sep 17 00:00:00 2001 From: drebs Date: Tue, 11 Aug 2015 11:44:42 -0300 Subject: [feature] allow passing callback to http client --- src/leap/common/http.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index cda8ee8..6fc10b4 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -31,6 +31,7 @@ except ImportError: from leap.common.certs import get_compatible_ssl_context_factory +from leap.common.check import leap_assert from zope.interface import implements @@ -150,7 +151,7 @@ class HTTPClient(object): pool.maxPersistentPerHost = maxPersistentPerHost return pool - def _request(self, url, method, body, headers): + def _request(self, url, method, body, headers, callback): """ Perform an HTTP request. @@ -162,6 +163,9 @@ class HTTPClient(object): :type body: str :param headers: The headers of the request. :type headers: dict + :param callback: A callback to be added to the request's deferred + callback chain. + :type callback: callable :return: A deferred that fires with the body of the request. :rtype: twisted.internet.defer.Deferred @@ -170,14 +174,21 @@ class HTTPClient(object): body = _StringBodyProducer(body) d = self._agent.request( method, url, headers=Headers(headers), bodyProducer=body) - d.addCallback(readBody) + d.addCallback(callback) return d - def request(self, url, method='GET', body=None, headers={}): + def request(self, url, method='GET', body=None, headers={}, + callback=readBody): """ Perform an HTTP request, but limit the maximum amount of concurrent connections. + May be passed a callback to be added to the request's deferred + callback chain. The callback is expected to receive the response of + the request and may do whatever it wants with the response. By + default, if no callback is passed, we will use a simple body reader + which returns a deferred that is fired with the body of the response. + :param url: The URL for the request. :type url: str :param method: The HTTP method of the request. @@ -186,11 +197,18 @@ class HTTPClient(object): :type body: str :param headers: The headers of the request. :type headers: dict + :param callback: A callback to be added to the request's deferred + callback chain. + :type callback: callable :return: A deferred that fires with the body of the request. :rtype: twisted.internet.defer.Deferred """ - return self._semaphore.run(self._request, url, method, body, headers) + leap_assert( + callable(callback), + message="The callback parameter should be a callable!") + return self._semaphore.run(self._request, url, method, body, headers, + callback) def close(self): """ -- cgit v1.2.3 From 97096fa2c22e96ec238c38ad3befdd052b5028cf Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 17 Aug 2015 19:15:05 -0400 Subject: [style] pep8 fix --- src/leap/common/http.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/leap/common/http.py') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 6fc10b4..0dee3a2 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -20,6 +20,7 @@ Twisted HTTP/HTTPS client. try: import twisted + assert twisted except ImportError: print "*******" print "Twisted is needed to use leap.common.http module" @@ -178,7 +179,7 @@ class HTTPClient(object): return d def request(self, url, method='GET', body=None, headers={}, - callback=readBody): + callback=readBody): """ Perform an HTTP request, but limit the maximum amount of concurrent connections. @@ -208,7 +209,7 @@ class HTTPClient(object): callable(callback), message="The callback parameter should be a callable!") return self._semaphore.run(self._request, url, method, body, headers, - callback) + callback) def close(self): """ -- cgit v1.2.3