Merge pull request #431 from twisted/treq-401-followups

Fix `treq.request()` argument handling

Author: twm

README.rst

@@ -9,17 +9,12 @@ treq: High-level Twisted HTTP Client API
     :alt: calver: YY.MM.MICRO
     :target: https://calver.org/
 
-.. |coverage| image:: https://coveralls.io/repos/github/twisted/treq/badge.svg
-    :alt: Coverage
-    :target: https://coveralls.io/github/twisted/treq
-
 .. |documentation| image:: https://readthedocs.org/projects/treq/badge/
     :alt: Documentation
     :target: https://treq.readthedocs.org
 
 |pypi|
 |calver|
-|coverage|
 |documentation|
 
 ``treq`` is an HTTP library inspired by

changelog.d/325.removal.rst

@@ -0,0 +1 @@
+treq no longer depends on `requests`. Consequently, the ``cookies()`` method no longer returns a `requests.cookies.RequestsCookieJar <https://requests.readthedocs.io/en/latest/api/#requests.cookies.RequestsCookieJar>`_. Instead, it returns `treq.cookies.IndexableCookieJar`, which implements ``__getitem__`` as a compatibility shim. We have *not* attempted to maintain full dict-interface compatibility with ``RequestsCookieJar``, as many of its interface extensions are difficult to use securely because they obscure the relationship between cookies and domains. treq interfaces still accept a ``request.cookies.RequestsCookieJar`` as the *cookies* parameter, like any `http.cookiejar.CookieJar` subclass.

docs/api.rst

@@ -94,12 +94,16 @@ Cookies
 
 .. module:: treq.cookies
 
-.. autoclass:: IndexableCookieJar
-
 .. autofunction:: scoped_cookie
 
 .. autofunction:: search
 
+.. autoclass:: IndexableCookieJar
+
+   .. automethod:: __getitem__
+
+.. autoexception:: CookieCollision
+
 Test Helpers
 ------------
 

docs/examples/using_cookies.py

@@ -9,11 +9,10 @@ async def using_cookies(reactor):
 
     jar = resp.cookies()
     [cookie] = treq.cookies.search(jar, domain="httpbin.org", name="hello")
-    print(f"The server set our hello cookie to: {cookie.value}")
+    print(f"The server set our hello cookie to: {cookie.value!r}")
 
-    await treq.get("https://httpbin.org/cookies", cookies=jar).addCallback(
-        print_response
-    )
+    response = await treq.get("https://httpbin.org/cookies", cookies=jar)
+    await print_response(response)
 
 
 react(using_cookies)

docs/index.rst

@@ -73,8 +73,6 @@ Here is a list of `requests`_ features and their status in treq.
 +----------------------------------+----------+----------+
 | Digest Authentication            | yes      | no       |
 +----------------------------------+----------+----------+
-| Elegant Key/Value Cookies        | yes      | yes      |
-+----------------------------------+----------+----------+
 | Automatic Decompression          | yes      | yes      |
 +----------------------------------+----------+----------+
 | Unicode Response Bodies          | yes      | yes      |
@@ -87,8 +85,6 @@ Here is a list of `requests`_ features and their status in treq.
 +----------------------------------+----------+----------+
 | .netrc support                   | yes      | no       |
 +----------------------------------+----------+----------+
-| Python 3.x                       | yes      | yes      |
-+----------------------------------+----------+----------+
 
 Table of Contents
 -----------------

src/treq/_types.py

@@ -33,7 +33,7 @@ class _ITreqReactor(IReactorTCP, IReactorTime, IReactorPluggableNameResolver):
 
 _S = Union[bytes, str]
 
-_URLType = Union[
+_SomeURL = Union[
     str,
     bytes,
     EncodedURL,

src/treq/api.py

@@ -1,9 +1,8 @@
 # -*- test-case-name: treq.test.test_api -*-
 from __future__ import annotations
 
-from typing import TypeVar, Union
+from typing import TypeVar
 
-from hyperlink import DecodedURL, EncodedURL
 from twisted.internet.defer import Deferred
 from twisted.internet.interfaces import IReactorTCP
 from twisted.web.client import Agent, HTTPConnectionPool
@@ -17,6 +16,7 @@
     _JSONType,
     _Nothing,
     _ParamsType,
+    _SomeURL,
 )
 from treq.client import HTTPClient
 from treq.response import _Response
@@ -25,11 +25,9 @@
 
 R = TypeVar("R")
 
-SomeURL = Union[DecodedURL, EncodedURL, str, bytes]
-
 
 def head(
-    url: SomeURL,
+    url: _SomeURL,
     *,
     agent: IAgent | None = None,
     pool: HTTPConnectionPool | None = None,
@@ -71,7 +69,7 @@ def head(
 
 
 def get(
-    url: SomeURL,
+    url: _SomeURL,
     headers: _HeadersType | None = None,
     *,
     agent: IAgent | None = None,
@@ -113,7 +111,7 @@ def get(
 
 
 def post(
-    url: SomeURL,
+    url: _SomeURL,
     data: _DataType | None = None,
     *,
     agent: IAgent | None = None,
@@ -155,7 +153,7 @@ def post(
 
 
 def put(
-    url: SomeURL,
+    url: _SomeURL,
     data: _DataType | None = None,
     *,
     agent: IAgent | None = None,
@@ -197,7 +195,7 @@ def put(
 
 
 def patch(
-    url: SomeURL,
+    url: _SomeURL,
     data: _DataType | None = None,
     *,
     agent: IAgent | None = None,
@@ -239,7 +237,7 @@ def patch(
 
 
 def delete(
-    url: SomeURL,
+    url: _SomeURL,
     *,
     agent: IAgent | None = None,
     pool: HTTPConnectionPool | None = None,
@@ -280,7 +278,26 @@ def delete(
     )
 
 
-def request(method, url, **kwargs):
+def request(
+    method: str,
+    url: _SomeURL,
+    data: _DataType | None = None,
+    *,
+    agent: IAgent | None = None,
+    pool: HTTPConnectionPool | None = None,
+    persistent: bool | None = None,
+    params: _ParamsType | None = None,
+    headers: _HeadersType | None = None,
+    files: _FilesType | None = None,
+    json: _JSONType | _Nothing = _NOTHING,
+    auth: tuple[str | bytes, str | bytes] | None = None,
+    cookies: _CookiesType | None = None,
+    allow_redirects: bool = True,
+    browser_like_redirects: bool = False,
+    unbuffered: bool = False,
+    reactor: _ITreqReactor | None = None,
+    timeout: float | None = None,
+) -> Deferred[_Response]:
     """
     Make an HTTP request.
 
@@ -292,27 +309,25 @@ def request(method, url, **kwargs):
         :class:`hyperlink.EncodedURL`
 
     :param headers: Optional HTTP Headers to send with this request.
-    :type headers: :class:`~twisted.web.http_headers.Headers` or None
 
     :param params: Optional parameters to be append to the URL query string.
         Any query string parameters in the *url* will be preserved.
-    :type params: dict w/ str or list/tuple of str values, list of 2-tuples, or
-        None.
 
     :param data:
         Arbitrary request body data.
 
         If *files* is also passed this must be a :class:`dict`,
         a :class:`tuple` or :class:`list` of field tuples as accepted by
-        :class:`MultiPartProducer`. The request is assigned a Content-Type of
-        ``multipart/form-data``.
+        :class:`~treq.multipart.MultiPartProducer`. The request is assigned
+        a Content-Type of ``multipart/form-data``.
 
         If a :class:`dict`, :class:`list`, or :class:`tuple` it is URL-encoded
         and the request assigned a Content-Type of
         ``application/x-www-form-urlencoded``.
 
         Otherwise, any non-``None`` value is passed to the client's
-        *data_to_body_producer* callable (by default, :class:`IBodyProducer`),
+        *data_to_body_producer* callable (by default,
+        :class:`~twisted.web.iweb.IBodyProducer`),
         which accepts :class:`bytes` and binary files like returned by
         ``open(..., "rb")``.
     :type data: `bytes`, `typing.BinaryIO`, `IBodyProducer`, or `None`
@@ -332,8 +347,8 @@ def request(method, url, **kwargs):
 
         Each ``binary_file`` is a file-like object open in binary mode (like
         returned by ``open("filename", "rb")``). The filename is taken from
-        the file's ``name`` attribute if not specified. The Content-Type is
-        guessed based on the filename using :func:`mimetypes.guess_type()` if
+        the file's ``name`` attribute, if not specified. The Content-Type is
+        guessed based on the filename using :func:`mimetypes.guess_type()`, if
         not specified, falling back to ``application/octet-stream``.
 
         While uploading Treq will measure the length of seekable files to
@@ -352,19 +367,20 @@ def request(method, url, **kwargs):
     :type auth: tuple of ``('username', 'password')``
 
     :param cookies: Cookies to send with this request.  The HTTP kind, not the
-        tasty kind.
-    :type cookies: ``dict`` or ``cookielib.CookieJar``
+        tasty kind. If you pass a :class:`dict`, the cookies therein will be
+        scoped to the origin of *url* (see :func:`~treq.cookies.scoped_cookie()`).
+    :type cookies: :class:`dict` or :class:`http.cookiejar.CookieJar`
 
     :param int timeout: Request timeout seconds. If a response is not
         received within this timeframe, a connection is aborted with
-        ``CancelledError``.
+        :exc:`~twisted.internet.defer.CancelledError`.
 
     :param bool allow_redirects: Follow HTTP redirects.  Default: ``True``
 
     :param bool browser_like_redirects: Follow redirects like a web browser:
         When a 301 or 302 redirect is received in response to a POST request
         convert the method to GET.
-        See :rfc:`7231 <7231#section-6.4.3>` and
+        See :rfc:`RFC 9110 <9110#section-15.4.3>` and
         :class:`~twisted.web.client.BrowserLikeRedirectAgent`). Default: ``False``
 
     :param bool unbuffered: Pass ``True`` to to disable response buffering.  By
@@ -387,7 +403,23 @@ def request(method, url, **kwargs):
         The *url* param now accepts :class:`hyperlink.DecodedURL` and
         :class:`hyperlink.EncodedURL` objects.
     """
-    return _client(**kwargs).request(method, url, _stacklevel=3, **kwargs)
+    return _client(agent, pool, persistent, reactor).request(
+        method,
+        url,
+        _stacklevel=3,
+        params=params,
+        headers=headers,
+        data=data,
+        files=files,
+        json=json,
+        auth=auth,
+        cookies=cookies,
+        allow_redirects=allow_redirects,
+        browser_like_redirects=browser_like_redirects,
+        unbuffered=unbuffered,
+        reactor=reactor,
+        timeout=timeout,
+    )
 
 
 #
@@ -432,6 +464,9 @@ def default_pool(reactor, pool, persistent):
     if get_global_pool() is None:
         set_global_pool(HTTPConnectionPool(reactor, persistent=True))
 
+    # NOTE: This doesn't necessarily return a pool that matches
+    # the *reactor* parameter, which can produce confusing behavior
+    # in tests that use a fake reactor.
     return get_global_pool()
 
 

src/treq/client.py

@@ -3,9 +3,8 @@
 
 import io
 import mimetypes
-import uuid
 import sys
-
+import uuid
 from collections import abc
 from http.cookiejar import CookieJar
 from json import dumps as json_dumps
@@ -19,9 +18,9 @@
 )
 
 if sys.version_info < (3, 10):
-    from typing_extensions import ParamSpec, Concatenate
+    from typing_extensions import Concatenate, ParamSpec
 else:
-    from typing import ParamSpec, Concatenate
+    from typing import Concatenate, ParamSpec
 
 from urllib.parse import quote_plus
 from urllib.parse import urlencode as _urlencode
@@ -55,7 +54,7 @@
     _JSONType,
     _Nothing,
     _ParamsType,
-    _URLType,
+    _SomeURL,
 )
 from treq.auth import add_auth
 from treq.cookies import scoped_cookie
@@ -146,9 +145,11 @@ def deliverBody(self, protocol):
 P2 = ParamSpec("P2")
 
 
-def _like(c: Callable[Concatenate[HTTPClient, str, _URLType, P], R]) -> Callable[
-    [Callable[Concatenate[HTTPClient, _URLType, P], R]],
-    Callable[Concatenate[HTTPClient, _URLType, P], R],
+def _like(
+    c: Callable[Concatenate[HTTPClient, str, _SomeURL, P], R],
+) -> Callable[
+    [Callable[Concatenate[HTTPClient, _SomeURL, P], R]],
+    Callable[Concatenate[HTTPClient, _SomeURL, P], R],
 ]:
     return lambda x: x
 
@@ -169,7 +170,7 @@ def __init__(
     def request(
         self,
         method: str,
-        url: _URLType,
+        url: _SomeURL,
         *,
         params: Optional[_ParamsType] = None,
         headers: Optional[_HeadersType] = None,
@@ -259,7 +260,7 @@ def gotResult(result):
         return d.addCallback(_Response, self._cookiejar)
 
     @_like(request)
-    def get(self, url: _URLType, **kwargs: Any) -> "Deferred[_Response]":
+    def get(self, url: _SomeURL, **kwargs: Any) -> "Deferred[_Response]":
         """
         See :func:`treq.get()`.
         """
@@ -268,7 +269,7 @@ def get(self, url: _URLType, **kwargs: Any) -> "Deferred[_Response]":
 
     @_like(request)
     def put(
-        self, url: _URLType, data: Optional[_DataType] = None, **kwargs: Any
+        self, url: _SomeURL, data: Optional[_DataType] = None, **kwargs: Any
     ) -> "Deferred[_Response]":
         """
         See :func:`treq.put()`.
@@ -278,7 +279,7 @@ def put(
 
     @_like(request)
     def patch(
-        self, url: _URLType, data: Optional[_DataType] = None, **kwargs: Any
+        self, url: _SomeURL, data: Optional[_DataType] = None, **kwargs: Any
     ) -> "Deferred[_Response]":
         """
         See :func:`treq.patch()`.
@@ -288,7 +289,7 @@ def patch(
 
     @_like(request)
     def post(
-        self, url: _URLType, data: Optional[_DataType] = None, **kwargs: Any
+        self, url: _SomeURL, data: Optional[_DataType] = None, **kwargs: Any
     ) -> "Deferred[_Response]":
         """
         See :func:`treq.post()`.
@@ -297,15 +298,15 @@ def post(
         return self.request("POST", url, data=data, **kwargs)
 
     @_like(request)
-    def head(self, url: _URLType, **kwargs: Any) -> "Deferred[_Response]":
+    def head(self, url: _SomeURL, **kwargs: Any) -> "Deferred[_Response]":
         """
         See :func:`treq.head()`.
         """
         kwargs.setdefault("_stacklevel", 3)
         return self.request("HEAD", url, **kwargs)
 
     @_like(request)
-    def delete(self, url: _URLType, **kwargs: Any) -> "Deferred[_Response]":
+    def delete(self, url: _SomeURL, **kwargs: Any) -> "Deferred[_Response]":
         """
         See :func:`treq.delete()`.
         """