From 98114245c686ef7cd62c9932071fd972ad3efedb Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 2 May 2013 12:46:59 -0400 Subject: [PATCH 1/4] Refactor merge_kwargs for clarity and to fix a few bugs --- requests/sessions.py | 74 +++++++++++++++++++------------------------- requests/utils.py | 4 +-- test_requests.py | 14 +++++++++ 3 files changed, 47 insertions(+), 45 deletions(-) diff --git a/requests/sessions.py b/requests/sessions.py index a8819247..f4aeeee6 100644 --- a/requests/sessions.py +++ b/requests/sessions.py @@ -9,14 +9,16 @@ requests (cookies, auth, proxies). """ import os +from collections import Mapping from datetime import datetime from .compat import cookielib, OrderedDict, urljoin, urlparse from .cookies import cookiejar_from_dict, extract_cookies_to_jar, RequestsCookieJar from .models import Request, PreparedRequest from .hooks import default_hooks, dispatch_hook -from .utils import from_key_val_list, default_headers +from .utils import to_key_val_list, default_headers from .exceptions import TooManyRedirects, InvalidSchema +from .structures import CaseInsensitiveDict from .adapters import HTTPAdapter @@ -32,49 +34,35 @@ REDIRECT_STATI = ( DEFAULT_REDIRECT_LIMIT = 30 -def merge_kwargs(local_kwarg, default_kwarg): - """Merges kwarg dictionaries. - - If a local key in the dictionary is set to None, it will be removed. +def merge_setting(request_setting, session_setting, dict_class=OrderedDict): + """ + Determines appropriate setting for a given request, taking into account the + explicit setting on that request, and the setting in the session. If a + setting is a dictionary, they will be merged together using `dict_class` """ - if default_kwarg is None: - return local_kwarg + if session_setting is None: + return request_setting - if isinstance(local_kwarg, str): - return local_kwarg + if request_setting is None: + return session_setting - if local_kwarg is None: - return default_kwarg + # Bypass if not a dictionary (e.g. verify) + if not ( + isinstance(session_setting, Mapping) and + isinstance(request_setting, Mapping) + ): + return request_setting - # Bypass if not a dictionary (e.g. timeout) - if not hasattr(default_kwarg, 'items'): - return local_kwarg - - default_kwarg = from_key_val_list(default_kwarg) - local_kwarg = from_key_val_list(local_kwarg) - - # Update new values in a case-insensitive way - def get_original_key(original_keys, new_key): - """ - Finds the key from original_keys that case-insensitive matches new_key. - """ - for original_key in original_keys: - if key.lower() == original_key.lower(): - return original_key - return new_key - - kwargs = default_kwarg.copy() - original_keys = kwargs.keys() - for key, value in local_kwarg.items(): - kwargs[get_original_key(original_keys, key)] = value + merged_setting = dict_class(to_key_val_list(session_setting)) + merged_setting.update(to_key_val_list(request_setting)) # Remove keys that are set to None. - for (k, v) in local_kwarg.items(): + for (k, v) in request_setting.items(): if v is None: - del kwargs[k] + del merged_setting[k] - return kwargs + return merged_setting class SessionRedirectMixin(object): @@ -311,14 +299,14 @@ class Session(SessionRedirectMixin): verify = os.environ.get('CURL_CA_BUNDLE') # Merge all the kwargs. - params = merge_kwargs(params, self.params) - headers = merge_kwargs(headers, self.headers) - auth = merge_kwargs(auth, self.auth) - proxies = merge_kwargs(proxies, self.proxies) - hooks = merge_kwargs(hooks, self.hooks) - stream = merge_kwargs(stream, self.stream) - verify = merge_kwargs(verify, self.verify) - cert = merge_kwargs(cert, self.cert) + params = merge_setting(params, self.params) + headers = merge_setting(headers, self.headers, dict_class=CaseInsensitiveDict) + auth = merge_setting(auth, self.auth) + proxies = merge_setting(proxies, self.proxies) + hooks = merge_setting(hooks, self.hooks) + stream = merge_setting(stream, self.stream) + verify = merge_setting(verify, self.verify) + cert = merge_setting(cert, self.cert) # Create the Request. req = Request() diff --git a/requests/utils.py b/requests/utils.py index d690559d..b21bf8fb 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -11,11 +11,11 @@ that are also useful for external consumption. import cgi import codecs +import collections import os import platform import re import sys -import zlib from netrc import netrc, NetrcParseError from . import __version__ @@ -135,7 +135,7 @@ def to_key_val_list(value): if isinstance(value, (str, bytes, bool, int)): raise ValueError('cannot encode objects that are not 2-tuples') - if isinstance(value, dict): + if isinstance(value, collections.Mapping): value = value.items() return list(value) diff --git a/test_requests.py b/test_requests.py index 60e44984..aabce29b 100644 --- a/test_requests.py +++ b/test_requests.py @@ -521,6 +521,20 @@ class RequestsTestCase(unittest.TestCase): self.assertTrue('http://' in s2.adapters) self.assertTrue('https://' in s2.adapters) + def test_header_remove_is_case_insensitive(self): + # From issue #1321 + s = requests.Session() + s.headers['foo'] = 'bar' + r = s.get(httpbin('get'), headers={'FOO': None}) + assert 'foo' not in r.request.headers + + def test_params_are_merged_case_sensitive(self): + s = requests.Session() + s.params['foo'] = 'bar' + r = s.get(httpbin('get'), params={'FOO': 'bar'}) + assert r.json()['args'] == {'foo': 'bar', 'FOO': 'bar'} + + def test_long_authinfo_in_url(self): url = 'http://{0}:{1}@{2}:9000/path?query#frag'.format( 'E8A3BE87-9E3F-4620-8858-95478E385B5B', From 003c795afed5f3b12c3ea8af12affbeb1218d7da Mon Sep 17 00:00:00 2001 From: Ib Lundgren Date: Tue, 21 May 2013 09:46:28 +0100 Subject: [PATCH 2/4] Only pass unicode fieldnames to urllib3. --- requests/models.py | 2 +- test_requests.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/requests/models.py b/requests/models.py index 55716307..db898bca 100644 --- a/requests/models.py +++ b/requests/models.py @@ -105,7 +105,7 @@ class RequestEncodingMixin(object): for v in val: if v is not None: new_fields.append( - (field.encode('utf-8') if isinstance(field, str) else field, + (field.decode('utf-8') if isinstance(field, bytes) else field, v.encode('utf-8') if isinstance(v, str) else v)) for (k, v) in files: diff --git a/test_requests.py b/test_requests.py index aabce29b..2b5f231c 100644 --- a/test_requests.py +++ b/test_requests.py @@ -342,6 +342,16 @@ class RequestsTestCase(unittest.TestCase): files={'file': ('test_requests.py', open(__file__, 'rb'))}) self.assertEqual(r.status_code, 200) + def test_unicode_multipart_post_fieldnames(self): + r = requests.Request(method='POST', + url=httpbin('post'), + data={'stuff'.encode('utf-8'): 'elixr'}, + files={'file': ('test_requests.py', + open(__file__, 'rb'))}) + prep = r.prepare() + self.assertTrue(b'name="stuff"' in prep.body) + self.assertFalse(b'name="b\'stuff\'"' in prep.body) + def test_custom_content_type(self): r = requests.post(httpbin('post'), data={'stuff': json.dumps({'a': 123})}, From 715a57dec888c850d16b767d7cdab9705fda7292 Mon Sep 17 00:00:00 2001 From: papaeye Date: Wed, 22 May 2013 02:20:51 +0900 Subject: [PATCH 3/4] Fix typo, %t -> %r --- requests/models.py | 2 +- test_requests.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/requests/models.py b/requests/models.py index db898bca..6cf2aaa1 100644 --- a/requests/models.py +++ b/requests/models.py @@ -291,7 +291,7 @@ class PreparedRequest(RequestEncodingMixin, RequestHooksMixin): raise MissingSchema("Invalid URL %r: No schema supplied" % url) if not host: - raise InvalidURL("Invalid URL %t: No host supplied" % url) + raise InvalidURL("Invalid URL %r: No host supplied" % url) # Only want to apply IDNA to the hostname try: diff --git a/test_requests.py b/test_requests.py index 2b5f231c..09619fff 100644 --- a/test_requests.py +++ b/test_requests.py @@ -14,6 +14,7 @@ from requests.auth import HTTPDigestAuth from requests.adapters import HTTPAdapter from requests.compat import str, cookielib from requests.cookies import cookiejar_from_dict +from requests.exceptions import InvalidURL, MissingSchema from requests.structures import CaseInsensitiveDict try: @@ -53,7 +54,8 @@ class RequestsTestCase(unittest.TestCase): requests.post def test_invalid_url(self): - self.assertRaises(ValueError, requests.get, 'hiwpefhipowhefopw') + self.assertRaises(MissingSchema, requests.get, 'hiwpefhipowhefopw') + self.assertRaises(InvalidURL, requests.get, 'http://') def test_basic_building(self): req = requests.Request() From 2a34335dc3efacf545171bf81075c556aae49f5d Mon Sep 17 00:00:00 2001 From: papaeye Date: Wed, 22 May 2013 04:30:19 +0900 Subject: [PATCH 4/4] Fix #1374 --- test_requests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) mode change 100644 => 100755 test_requests.py diff --git a/test_requests.py b/test_requests.py old mode 100644 new mode 100755 index 09619fff..4a4831e5 --- a/test_requests.py +++ b/test_requests.py @@ -345,11 +345,12 @@ class RequestsTestCase(unittest.TestCase): self.assertEqual(r.status_code, 200) def test_unicode_multipart_post_fieldnames(self): + filename = os.path.splitext(__file__)[0] + '.py' r = requests.Request(method='POST', url=httpbin('post'), data={'stuff'.encode('utf-8'): 'elixr'}, files={'file': ('test_requests.py', - open(__file__, 'rb'))}) + open(filename, 'rb'))}) prep = r.prepare() self.assertTrue(b'name="stuff"' in prep.body) self.assertFalse(b'name="b\'stuff\'"' in prep.body)