Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch treq so public APIs don't swallow any unexpected kwargs. #251

Closed
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions src/treq/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def head(url, **kwargs):

See :py:func:`treq.request`
"""
return _client(**kwargs).head(url, **kwargs)
return _client(kwargs).head(url, **kwargs)


def get(url, headers=None, **kwargs):
Expand All @@ -21,7 +21,7 @@ def get(url, headers=None, **kwargs):

See :py:func:`treq.request`
"""
return _client(**kwargs).get(url, headers=headers, **kwargs)
return _client(kwargs).get(url, headers=headers, **kwargs)


def post(url, data=None, **kwargs):
Expand All @@ -30,7 +30,7 @@ def post(url, data=None, **kwargs):

See :py:func:`treq.request`
"""
return _client(**kwargs).post(url, data=data, **kwargs)
return _client(kwargs).post(url, data=data, **kwargs)


def put(url, data=None, **kwargs):
Expand All @@ -39,7 +39,7 @@ def put(url, data=None, **kwargs):

See :py:func:`treq.request`
"""
return _client(**kwargs).put(url, data=data, **kwargs)
return _client(kwargs).put(url, data=data, **kwargs)


def patch(url, data=None, **kwargs):
Expand All @@ -48,7 +48,7 @@ def patch(url, data=None, **kwargs):

See :py:func:`treq.request`
"""
return _client(**kwargs).patch(url, data=data, **kwargs)
return _client(kwargs).patch(url, data=data, **kwargs)


def delete(url, **kwargs):
Expand All @@ -57,7 +57,7 @@ def delete(url, **kwargs):

See :py:func:`treq.request`
"""
return _client(**kwargs).delete(url, **kwargs)
return _client(kwargs).delete(url, **kwargs)


def request(method, url, **kwargs):
Expand All @@ -81,6 +81,10 @@ def request(method, url, **kwargs):
:param data: Optional request body.
:type data: str, file-like, IBodyProducer, or None

:param files: If present, issue a ``multipart/form-data`` request as it
suits better for cases with filse and/or large objects.
evilham marked this conversation as resolved.
Show resolved Hide resolved
:type files: list of (unicode, file-like) for (filename, file).

:param json: Optional JSON-serializable content to pass in body.
:type json: dict, list/tuple, int, string/unicode, bool, or None

Expand Down Expand Up @@ -116,19 +120,19 @@ def request(method, url, **kwargs):
:rtype: Deferred that fires with an IResponse provider.

"""
return _client(**kwargs).request(method, url, **kwargs)
return _client(kwargs).request(method, url, **kwargs)


#
# Private API
#

def _client(*args, **kwargs):
agent = kwargs.get('agent')
def _client(kwargs):
evilham marked this conversation as resolved.
Show resolved Hide resolved
agent = kwargs.pop('agent', None)
pool = kwargs.pop('pool', None)
persistent = kwargs.pop('persistent', None)
if agent is None:
reactor = default_reactor(kwargs.get('reactor'))
pool = default_pool(reactor,
kwargs.get('pool'),
kwargs.get('persistent'))
pool = default_pool(reactor, pool, persistent)
agent = Agent(reactor, pool=pool)
return HTTPClient(agent)
38 changes: 18 additions & 20 deletions src/treq/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ def deliverBody(self, protocol):
self._waiters.append(protocol)


class _NoJsonData():
evilham marked this conversation as resolved.
Show resolved Hide resolved
"""Dummy class to identify data that is not Json."""
evilham marked this conversation as resolved.
Show resolved Hide resolved
pass


class HTTPClient(object):
def __init__(self, agent, cookiejar=None,
data_to_body_producer=IBodyProducer):
Expand Down Expand Up @@ -141,15 +146,18 @@ def delete(self, url, **kwargs):
"""
return self.request('DELETE', url, **kwargs)

def request(self, method, url, **kwargs):
def request(self, method, url,
headers=None, params=None, data=None, files=None,
auth=None, cookies=None, allow_redirects=True,
evilham marked this conversation as resolved.
Show resolved Hide resolved
browser_like_redirects=False, unbuffered=False,
timeout=None, reactor=None, json=_NoJsonData()):
"""
See :func:`treq.request()`.
"""
method = method.encode('ascii').upper()

# Join parameters provided in the URL
# and the ones passed as argument.
params = kwargs.get('params')
if params:
url = _combine_query_params(url, params)

Expand All @@ -158,7 +166,6 @@ def request(self, method, url, **kwargs):

# Convert headers dictionary to
# twisted raw headers format.
headers = kwargs.get('headers')
if headers:
if isinstance(headers, dict):
h = Headers({})
Expand All @@ -175,11 +182,9 @@ def request(self, method, url, **kwargs):
# Here we choose a right producer
# based on the parameters passed in.
bodyProducer = None
data = kwargs.get('data')
files = kwargs.get('files')
# since json=None needs to be serialized as 'null', we need to
# explicitly check kwargs for this key
has_json = 'json' in kwargs
# perform a different kind of check.
has_json = not isinstance(json, _NoJsonData)

if files:
# If the files keyword is present we will issue a
Expand Down Expand Up @@ -209,38 +214,31 @@ def request(self, method, url, **kwargs):
# If data is sent as json, set Content-Type as 'application/json'
headers.setRawHeaders(
b'content-type', [b'application/json; charset=UTF-8'])
content = kwargs['json']
json = json_dumps(content, separators=(u',', u':')).encode('utf-8')
json = json_dumps(json, separators=(u',', u':')).encode('utf-8')
bodyProducer = self._data_to_body_producer(json)

cookies = kwargs.get('cookies', {})

if not isinstance(cookies, CookieJar):
cookies = cookiejar_from_dict(cookies)

cookies = dict() if cookies is None else cookies
cookies = merge_cookies(self._cookiejar, cookies)
wrapped_agent = CookieAgent(self._agent, cookies)

if kwargs.get('allow_redirects', True):
if kwargs.get('browser_like_redirects', False):
if allow_redirects:
if browser_like_redirects:
wrapped_agent = BrowserLikeRedirectAgent(wrapped_agent)
else:
wrapped_agent = RedirectAgent(wrapped_agent)

wrapped_agent = ContentDecoderAgent(wrapped_agent,
[(b'gzip', GzipDecoder)])

auth = kwargs.get('auth')
if auth:
wrapped_agent = add_auth(wrapped_agent, auth)

d = wrapped_agent.request(
method, url, headers=headers,
bodyProducer=bodyProducer)

timeout = kwargs.get('timeout')
if timeout:
delayedCall = default_reactor(kwargs.get('reactor')).callLater(
delayedCall = default_reactor(reactor).callLater(
timeout, d.cancel)

def gotResult(result):
Expand All @@ -250,7 +248,7 @@ def gotResult(result):

d.addBoth(gotResult)

if not kwargs.get('unbuffered', False):
if not unbuffered:
d.addCallback(_BufferedResponse)

return d.addCallback(_Response, cookies)
Expand Down