From b76326bf94382c9b44062435fd985d68b2e60edb Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Sat, 17 Nov 2012 13:00:39 +0000 Subject: [PATCH 01/13] MockRequest needs a type property for Py3.3 --- requests/cookies.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requests/cookies.py b/requests/cookies.py index c3c2debb..245fdd9e 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -32,9 +32,10 @@ class MockRequest(object): def __init__(self, request): self._r = request self._new_headers = {} + self.type = urlparse(self._r.full_url).scheme def get_type(self): - return urlparse(self._r.full_url).scheme + return self.type def get_host(self): return urlparse(self._r.full_url).netloc From 8da100f652793f95f7dfdad588c925dfac9e71a1 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Sun, 18 Nov 2012 12:06:33 +0000 Subject: [PATCH 02/13] Respect the no_proxy environment variable. This change is in response to issue #879. --- requests/models.py | 2 +- requests/utils.py | 24 +++++++++++++++++++++--- tests/test_utils.py | 26 +++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/requests/models.py b/requests/models.py index 58e7f9a7..9c0666a6 100644 --- a/requests/models.py +++ b/requests/models.py @@ -119,7 +119,7 @@ class Request(object): # If no proxies are given, allow configuration by environment variables # HTTP_PROXY and HTTPS_PROXY. if not self.proxies and self.config.get('trust_env'): - self.proxies = get_environ_proxies() + self.proxies = get_environ_proxies(self.url) self.data = data self.params = params diff --git a/requests/utils.py b/requests/utils.py index b3d33f4f..fef2d83f 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -502,7 +502,7 @@ def requote_uri(uri): return quote(unquote_unreserved(uri), safe="!#$%&'()*+,/:;=?@[]~") -def get_environ_proxies(): +def get_environ_proxies(url): """Return a dict of environment proxies.""" proxy_keys = [ @@ -510,11 +510,29 @@ def get_environ_proxies(): 'http', 'https', 'ftp', - 'socks', - 'no' + 'socks' ] get_proxy = lambda k: os.environ.get(k) or os.environ.get(k.upper()) + + # First check whether no_proxy is defined. If it is, check that the URL + # we're getting isn't in the no_proxy list. + no_proxy = get_proxy('no_proxy') + + if no_proxy: + # We need to check whether we match here. We need to see if we match + # the end of the netloc, both with and without the port. + no_proxy = no_proxy.split(',') + netloc = urlparse(url).netloc + + for host in no_proxy: + if netloc.endswith(host) or netloc.split(':')[0].endswith(host): + # The URL does match something in no_proxy, so we don't want + # to apply the proxies on this URL. + return {} + + # If we get here, we either didn't have no_proxy set or we're not going + # anywhere that no_proxy applies to. proxies = [(key, get_proxy(key + '_proxy')) for key in proxy_keys] return dict([(key, val) for (key, val) in proxies if val]) diff --git a/tests/test_utils.py b/tests/test_utils.py index 5cd0684e..7febc570 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -9,6 +9,7 @@ import random # Path hack. sys.path.insert(0, os.path.abspath('..')) +from requests.utils import get_environ_proxies import requests.utils from requests.compat import is_py3, bytes @@ -20,7 +21,7 @@ else: byteschr = chr -class GuessJSONUTFTests(unittest.TestCase): +class UtilityTests(unittest.TestCase): """Tests for the JSON UTF encoding guessing code.""" codecs = ( @@ -73,5 +74,28 @@ class GuessJSONUTFTests(unittest.TestCase): continue raise + def test_get_environ_proxies_respects_no_proxy(self): + '''This test confirms that the no_proxy environment setting is + respected by get_environ_proxies().''' + + # Set up some example environment settings. + os.environ['http_proxy'] = 'http://www.example.com/' + os.environ['no_proxy'] = r'localhost,.0.0.1:8080' + + # Set up expected proxy return values. + proxy_yes = {'http': 'http://www.example.com/'} + proxy_no = {} + + # Check that we get the right things back. + self.assertEqual(proxy_yes, + get_environ_proxies('http://www.google.com/')) + self.assertEqual(proxy_no, + get_environ_proxies('http://localhost/test')) + self.assertEqual(proxy_no, + get_environ_proxies('http://127.0.0.1:8080/')) + self.assertEqual(proxy_yes, + get_environ_proxies('http://127.0.0.1:8081/')) + + if __name__ == '__main__': unittest.main() From c02520ed99d5aead74d70bcae70d3d1fa6424012 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Sun, 18 Nov 2012 12:31:55 +0000 Subject: [PATCH 03/13] Make sure we reset environment variables. Turns out nose runs all the tests in one process, so changing the os.environ dictionary makes everything go horribly wrong. --- tests/test_utils.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index 7febc570..015cac63 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -78,6 +78,17 @@ class UtilityTests(unittest.TestCase): '''This test confirms that the no_proxy environment setting is respected by get_environ_proxies().''' + # Store the current environment settings. + try: + old_http_proxy = os.environ['http_proxy'] + except KeyError: + old_http_proxy = None + + try: + old_no_proxy = os.environ['no_proxy'] + except KeyError: + old_no_proxy = None + # Set up some example environment settings. os.environ['http_proxy'] = 'http://www.example.com/' os.environ['no_proxy'] = r'localhost,.0.0.1:8080' @@ -96,6 +107,16 @@ class UtilityTests(unittest.TestCase): self.assertEqual(proxy_yes, get_environ_proxies('http://127.0.0.1:8081/')) + # Return the settings to what they were. + if old_http_proxy: + os.environ['http_proxy'] = old_http_proxy + else: + del os.environ['http_proxy'] + + if old_no_proxy: + os.environ['no_proxy'] = old_no_proxy + else: + del os.environ['no_proxy'] if __name__ == '__main__': unittest.main() From c5fddb95edd7694c0164ae2a5279ceeebe475511 Mon Sep 17 00:00:00 2001 From: John Peacock Date: Wed, 21 Nov 2012 13:34:03 -0500 Subject: [PATCH 04/13] Only return a path if the cacert.pem file exists. This will permit the deletion of just that one file in order to fall back to the [probably more accurate but less consistent] Distro provided CA certs. --- requests/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/requests/utils.py b/requests/utils.py index b3d33f4f..fd96d28b 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -29,7 +29,9 @@ CERTIFI_BUNDLE_PATH = None try: # see if requests's own CA certificate bundle is installed from . import certs - CERTIFI_BUNDLE_PATH = certs.where() + path = certs.where() + if os.path.exists(path): + CERTIFI_BUNDLE_PATH = certs.where() except ImportError: pass From 20cd46426fda1b048afa90955f445424efa4be46 Mon Sep 17 00:00:00 2001 From: John Peacock Date: Wed, 21 Nov 2012 13:41:16 -0500 Subject: [PATCH 05/13] Only return a path if the cacert.pem file exists. This will permit the deletion of just that one file in order to fall back to the [probably more accurate but less consistent] Distro provided CA certs. --- requests/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/requests/utils.py b/requests/utils.py index b3d33f4f..fd96d28b 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -29,7 +29,9 @@ CERTIFI_BUNDLE_PATH = None try: # see if requests's own CA certificate bundle is installed from . import certs - CERTIFI_BUNDLE_PATH = certs.where() + path = certs.where() + if os.path.exists(path): + CERTIFI_BUNDLE_PATH = certs.where() except ImportError: pass From 18be26fc2a7f8d41b691762d826f235ae04f24fd Mon Sep 17 00:00:00 2001 From: Marcin Wielgoszewski Date: Thu, 22 Nov 2012 11:10:22 -0500 Subject: [PATCH 06/13] Back to issue #630, .isalnum() was sufficient in addressing the issue. Adding a try/except block just masks any issues that are raised here, issues that the developer should definitely be made aware of. --- requests/utils.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/requests/utils.py b/requests/utils.py index b3d33f4f..df25126b 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -473,21 +473,18 @@ def unquote_unreserved(uri): """Un-escape any percent-escape sequences in a URI that are unreserved characters. This leaves all reserved, illegal and non-ASCII bytes encoded. """ - try: - parts = uri.split('%') - for i in range(1, len(parts)): - h = parts[i][0:2] - if len(h) == 2 and h.isalnum(): - c = chr(int(h, 16)) - if c in UNRESERVED_SET: - parts[i] = c + parts[i][2:] - else: - parts[i] = '%' + parts[i] + parts = uri.split('%') + for i in range(1, len(parts)): + h = parts[i][0:2] + if len(h) == 2 and h.isalnum(): + c = chr(int(h, 16)) + if c in UNRESERVED_SET: + parts[i] = c + parts[i][2:] else: parts[i] = '%' + parts[i] - return ''.join(parts) - except ValueError: - return uri + else: + parts[i] = '%' + parts[i] + return ''.join(parts) def requote_uri(uri): From 8269ee726656cc76e67f02e7f37b90c8d01f5520 Mon Sep 17 00:00:00 2001 From: Jonatan Heyman Date: Fri, 23 Nov 2012 16:48:51 +0100 Subject: [PATCH 07/13] Fixed so that safe_mode works for Sessions --- AUTHORS.rst | 1 + requests/api.py | 2 -- requests/safe_mode.py | 10 +++++----- requests/sessions.py | 8 +++++++- tests/test_requests.py | 13 +++++++++++++ 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 3c074d1b..e18d5751 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -116,3 +116,4 @@ Patches and Suggestions - André Graf (dergraf) - Stephen Zhuang (everbird) - Martijn Pieters +- Jonatan Heyman diff --git a/requests/api.py b/requests/api.py index ded79352..297f4cbf 100644 --- a/requests/api.py +++ b/requests/api.py @@ -12,10 +12,8 @@ This module implements the Requests API. """ from . import sessions -from .safe_mode import catch_exceptions_if_in_safe_mode -@catch_exceptions_if_in_safe_mode def request(method, url, **kwargs): """Constructs and sends a :class:`Request `. Returns :class:`Response ` object. diff --git a/requests/safe_mode.py b/requests/safe_mode.py index 0fb8d705..18808d74 100644 --- a/requests/safe_mode.py +++ b/requests/safe_mode.py @@ -18,17 +18,17 @@ import socket def catch_exceptions_if_in_safe_mode(function): - """New implementation of safe_mode. We catch all exceptions at the API level + """New implementation of safe_mode. We catch all exceptions at the Session level and then return a blank Response object with the error field filled. This decorator - wraps request() in api.py. + wraps Session._send_request() in sessions.py. """ - def wrapped(method, url, **kwargs): + def wrapped(*args, **kwargs): # if save_mode, we catch exceptions and fill error field if (kwargs.get('config') and kwargs.get('config').get('safe_mode')) or (kwargs.get('session') and kwargs.get('session').config.get('safe_mode')): try: - return function(method, url, **kwargs) + return function(*args, **kwargs) except (RequestException, ConnectionError, HTTPError, socket.timeout, socket.gaierror) as e: r = Response() @@ -36,5 +36,5 @@ def catch_exceptions_if_in_safe_mode(function): r.raw = HTTPResponse() # otherwise, tests fail r.status_code = 0 # with this status_code, content returns None return r - return function(method, url, **kwargs) + return function(*args, **kwargs) return wrapped diff --git a/requests/sessions.py b/requests/sessions.py index 0962d819..5d67b4d8 100644 --- a/requests/sessions.py +++ b/requests/sessions.py @@ -17,6 +17,7 @@ from .models import Request from .hooks import dispatch_hook from .utils import header_expand, from_key_val_list from .packages.urllib3.poolmanager import PoolManager +from .safe_mode import catch_exceptions_if_in_safe_mode def merge_kwargs(local_kwarg, default_kwarg): @@ -265,7 +266,12 @@ class Session(object): return r # Send the HTTP Request. - r.send(prefetch=prefetch) + return self._send_request(r, **args) + + @catch_exceptions_if_in_safe_mode + def _send_request(self, r, **kwargs): + # Send the request. + r.send(prefetch=kwargs.get("prefetch")) # Return the response. return r.response diff --git a/tests/test_requests.py b/tests/test_requests.py index 6615678f..cf326acf 100755 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -929,6 +929,19 @@ class RequestsTestSuite(TestSetup, TestBaseMixin, unittest.TestCase): ds2 = pickle.loads(pickle.dumps(requests.session(prefetch=False))) self.assertTrue(ds1.prefetch) self.assertFalse(ds2.prefetch) + + def test_session_connection_error_with_safe_mode(self): + config = {"safe_mode":True} + + s = requests.session() + r = s.get("http://localhost:1/nope", timeout=0.1, config=config) + self.assertFalse(r.ok) + self.assertTrue(r.content is None) + + s2 = requests.session(config=config) + r2 = s2.get("http://localhost:1/nope", timeout=0.1) + self.assertFalse(r2.ok) + self.assertTrue(r2.content is None) def test_connection_error(self): try: From d5f9a2a51c884099294f75ae9148ec0f64b0bab4 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Sat, 24 Nov 2012 12:15:30 +0000 Subject: [PATCH 08/13] Avoid using callable(). Callable() is not included in Python 3.1, so we shouldn't use it. --- requests/models.py | 5 +++-- tests/test_requests.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/requests/models.py b/requests/models.py index 9c0666a6..3448335d 100644 --- a/requests/models.py +++ b/requests/models.py @@ -9,6 +9,7 @@ This module contains the primary objects that power Requests. import os import socket +import collections from datetime import datetime from io import BytesIO @@ -467,10 +468,10 @@ class Request(object): def register_hook(self, event, hook): """Properly register a hook.""" - if callable(hook): + if isinstance(hook, collections.Callable): self.hooks[event].append(hook) elif hasattr(hook, '__iter__'): - self.hooks[event].extend(h for h in hook if callable(h)) + self.hooks[event].extend(h for h in hook if isinstance(h, collections.Callable)) def deregister_hook(self, event, hook): """Deregister a previously registered hook. diff --git a/tests/test_requests.py b/tests/test_requests.py index 6615678f..22df526e 100755 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -11,6 +11,7 @@ import json import unittest import pickle import tempfile +import collections import requests from requests.compat import str, StringIO @@ -805,7 +806,7 @@ class RequestsTestSuite(TestSetup, TestBaseMixin, unittest.TestCase): def assert_hooks_are_callable(hooks): for h in hooks['args']: - self.assertTrue(callable(h)) + self.assertTrue(isinstance(h, collections.Callable)) hooks = [add_foo_header, add_bar_header] r = requests.models.Request() From 31f74bd02eb89592c1173a301c402132e171153f Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Sat, 24 Nov 2012 21:17:29 +0000 Subject: [PATCH 09/13] Make OAuth handle less-common body data better. Related to Issue #910. Specifically, OAuth won't sign the request unless it gets a body type that is urlencoded or multipart. This is overly restrictive. The correct behaviour is to sign the message without including the body as part of the signature. --- requests/auth.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/requests/auth.py b/requests/auth.py index 65568f52..b662397e 100644 --- a/requests/auth.py +++ b/requests/auth.py @@ -92,14 +92,28 @@ class OAuth1(AuthBase): # Omit body data in the signing and since it will always # be empty (cant add paras to body if multipart) and we wish # to preserve body. - r.url, r.headers, _ = self.client.sign( - unicode(r.full_url), unicode(r.method), None, r.headers) - elif decoded_body is not None and contenttype in (CONTENT_TYPE_FORM_URLENCODED, ''): - # Normal signing - if not contenttype: - r.headers['Content-Type'] = CONTENT_TYPE_FORM_URLENCODED - r.url, r.headers, r.data = self.client.sign( - unicode(r.full_url), unicode(r.method), r.data, r.headers) + r.url, r.headers, _ = self.client.sign(unicode(r.full_url), + unicode(r.method), + None, + r.headers) + elif (decoded_body is not None and + contenttype == CONTENT_TYPE_FORM_URLENCODED): + # If the Content-Type header is urlencoded and there are no + # illegal characters in the body, assume that the content actually + # is urlencoded, and so should be part of the signature. + r.url, r.headers, r.data = self.client.sign(unicode(r.full_url), + unicode(r.method), + r.data, + r.headers) + elif r.data: + # The data we passed was either definitely not urlencoded + # (because extract_params returned nothing) or doesn't have a + # content header that assures us that it is. Assume then that the + # data shouldn't be part of the signature. + r.url, r.headers, _ = self.client.sign(unicode(r.full_url), + unicode(r.method), + None, + r.headers) else: _oauth_signed = False if _oauth_signed: From f003025a37f9d546570e52f7d355c6f5d1a81617 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 24 Nov 2012 21:47:47 -0500 Subject: [PATCH 10/13] Attach Content-Length to everything. Closes #223 --- requests/models.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/requests/models.py b/requests/models.py index 9c0666a6..84fae6e3 100644 --- a/requests/models.py +++ b/requests/models.py @@ -541,6 +541,10 @@ class Request(object): else: content_type = 'application/x-www-form-urlencoded' + self.headers['Content-Length'] = '0' + if body is not None: + self.headers['Content-Length'] = str(len(body)) + # Add content-type if it wasn't explicitly provided. if (content_type) and (not 'content-type' in self.headers): self.headers['Content-Type'] = content_type From 61f16d1ddcf4729945ce7b0b9bfbd94b75dc4b92 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 24 Nov 2012 22:01:16 -0500 Subject: [PATCH 11/13] Handle files as well. --- requests/models.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/requests/models.py b/requests/models.py index 84fae6e3..4025dae7 100644 --- a/requests/models.py +++ b/requests/models.py @@ -542,7 +542,11 @@ class Request(object): content_type = 'application/x-www-form-urlencoded' self.headers['Content-Length'] = '0' - if body is not None: + if isinstance(body, file): + body.seek(0, 2) + self.headers['Content-Length'] = str(body.tell()) + body.seek(0, 0) + elif body is not None: self.headers['Content-Length'] = str(len(body)) # Add content-type if it wasn't explicitly provided. From b93fbd30c32b122bf7aba7db6663ca5a292b17d3 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 24 Nov 2012 22:43:52 -0500 Subject: [PATCH 12/13] Fix python 3 tests. --- requests/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requests/models.py b/requests/models.py index 4025dae7..44b41d8e 100644 --- a/requests/models.py +++ b/requests/models.py @@ -542,7 +542,7 @@ class Request(object): content_type = 'application/x-www-form-urlencoded' self.headers['Content-Length'] = '0' - if isinstance(body, file): + if hasattr(body, 'seek') and hasattr(body, 'tell'): body.seek(0, 2) self.headers['Content-Length'] = str(body.tell()) body.seek(0, 0) From 24ed8c9457be0fd8fe3c5ac32d5c32e6996593ef Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Mon, 26 Nov 2012 13:23:38 -0500 Subject: [PATCH 13/13] Add a bit more to the developer's documentation I'm not sure what else could be added to satisfy #601. If this is sufficient, I suggest closing that. --- docs/dev/todo.rst | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/dev/todo.rst b/docs/dev/todo.rst index 7aa2cae8..e9930a8e 100644 --- a/docs/dev/todo.rst +++ b/docs/dev/todo.rst @@ -23,7 +23,19 @@ order to run requests' test suite:: $ make $ make test -The ``Makefile`` has various useful targets for testing. +The ``Makefile`` has various useful targets for testing. For example, if you +want to see how your pull request will behave with Travis-CI you would run +``make travis``. + +Versions of Python to Test On +----------------------------- + +Officially (as of 26-Nov-2012), requests supports python 2.6-3.3. In the +future, support for 3.1 and 3.2 may be dropped. In general you will need to +test on at least one python 2 and one python 3 version. You can also set up +Travis CI for your own fork before you submit a pull request so that you are +assured your fork works. To use Travis CI for your fork and other projects see +their `documentation `_. What Needs to be Done ---------------------