Add internal support for file-like object responses to improve adapter plugin support (#1094)

* Support `requests.response.raw` being a file-like object

Previously HTTPie relied on `requests.models.Response.raw` being
`urllib3.HTTPResponse`. The `requests` documentation specifies that
(requests.models.Response.raw)[https://docs.python-requests.org/en/master/api/#requests.Response.raw]
is a file-like object but allows for other types for internal use.

This change introduces graceful handling for scenarios when
`requests.models.Response.raw` is not `urllib3.HTTPResponse`. In such a scenario
HTTPie now falls back to extracting metadata from `requests.models.Response`
directly instead of direct access from protected protected members such as
`response.raw._original_response`. A side effect in this fallback procedure is
that we can no longer determine HTTP protocol version and report it as `1.1`.

This change is necessary to make it possible to implement `TransportPlugins`
without having to also needing to emulate internal behavior of `urlib3` and
`http.client`.

* Load cookies from `response.headers` instead of `response.raw._original_response.msg._headers`

`response.cookies` was not utilized as it not possible to construct original
payload from `http.cookiejar.Cookie`. Data is stored in lossy format. For example
`Cookie.secure` defaults to `False` so we cannot distinguish if `Cookie.secure` was
set to `False` or was not set at all. Same problem applies to other fields also.

* Simpler HTTP envelope data extraction

* Test cookie extraction and make cookie presentment backwards compatible

Co-authored-by: Mickaël Schoentgen <contact@tiger-222.fr>
Co-authored-by: Jakub Roztocil <jakub@roztocil.co>

CHANGELOG.rst

@@ -13,6 +13,7 @@ This project adheres to `Semantic Versioning <https://semver.org/>`_.
   an alternative to ``stdin``. (`#534`_)
 * Fixed ``--continue --download`` with a single byte to be downloaded left. (`#1032`_)
 * Fixed ``--verbose`` HTTP 307 redirects with streamed request body. (`#1088`_)
+* Add internal support for file-like object responses to improve adapter plugin support. (`#1094`_)
 
 
 `2.4.0`_ (2021-02-06)

httpie/client.py

@@ -104,9 +104,8 @@ def collect_messages(
                     **send_kwargs,
                 )
 
-            # noinspection PyProtectedMember
             expired_cookies += get_expired_cookies(
-                headers=response.raw._original_response.msg._headers
+                response.headers.get('Set-Cookie', '')
             )
 
             response_count += 1

httpie/models.py

@@ -1,6 +1,8 @@
 from typing import Iterable, Optional
 from urllib.parse import urlsplit
 
+from .utils import split_cookies
+
 
 class HTTPMessage:
     """Abstract class for HTTP messages."""
@@ -52,21 +54,30 @@ class HTTPResponse(HTTPMessage):
     # noinspection PyProtectedMember
     @property
     def headers(self):
-        original = self._orig.raw._original_response
-
+        try:
+            raw_version = self._orig.raw._original_response.version
+        except AttributeError:
+            # Assume HTTP/1.1
+            raw_version = 11
         version = {
             9: '0.9',
             10: '1.0',
             11: '1.1',
             20: '2',
-        }[original.version]
+        }[raw_version]
 
-        status_line = f'HTTP/{version} {original.status} {original.reason}'
+        original = self._orig
+        status_line = f'HTTP/{version} {original.status_code} {original.reason}'
         headers = [status_line]
-        # `original.msg` is a `http.client.HTTPMessage`
-        # `_headers` is a 2-tuple
         headers.extend(
-            f'{header[0]}: {header[1]}' for header in original.msg._headers)
+            ': '.join(header)
+            for header in original.headers.items()
+            if header[0] != 'Set-Cookie'
+        )
+        headers.extend(
+            f'Set-Cookie: {cookie}'
+            for cookie in split_cookies(original.headers.get('Set-Cookie'))
+        )
         return '\r\n'.join(headers)
 
     @property

httpie/utils.py

@@ -5,9 +5,12 @@ from collections import OrderedDict
 from http.cookiejar import parse_ns_headers
 from pprint import pformat
 from typing import List, Optional, Tuple
+import re
 
 import requests.auth
 
+RE_COOKIE_SPLIT = re.compile(r', (?=[^ ;]+=)')
+
 
 def load_json_preserve_order(s):
     return json.loads(s, object_pairs_hook=OrderedDict)
@@ -85,8 +88,21 @@ def get_content_type(filename):
         return content_type
 
 
+def split_cookies(cookies):
+    """
+    When ``requests`` stores cookies in ``response.headers['Set-Cookie']``
+    it concatenates all of them through ``, ``.
+
+    This function splits cookies apart being careful to not to
+    split on ``, `` which may be part of cookie value.
+    """
+    if not cookies:
+        return []
+    return RE_COOKIE_SPLIT.split(cookies)
+
+
 def get_expired_cookies(
-    headers: List[Tuple[str, str]],
+    cookies: str,
     now: float = None
 ) -> List[dict]:
 
@@ -96,9 +112,9 @@ def get_expired_cookies(
         return expires is not None and expires <= now
 
     attr_sets: List[Tuple[str, str]] = parse_ns_headers(
-        value for name, value in headers
-        if name.lower() == 'set-cookie'
+        split_cookies(cookies)
     )
+
     cookies = [
         # The first attr name is the cookie name.
         dict(attrs[1:], name=attrs[0][0])

tests/test_cookie.py

@@ -0,0 +1,47 @@
+from http.cookies import SimpleCookie
+from http.server import BaseHTTPRequestHandler, HTTPServer
+from threading import Thread
+
+from .utils import http
+
+
+class TestIntegration:
+
+    def setup_mock_server(self, handler):
+        """Configure mock server."""
+        # Passing 0 as the port will cause a random free port to be chosen.
+        self.mock_server = HTTPServer(('localhost', 0), handler)
+        _, self.mock_server_port = self.mock_server.server_address
+
+        # Start running mock server in a separate thread.
+        # Daemon threads automatically shut down when the main process exits.
+        self.mock_server_thread = Thread(target=self.mock_server.serve_forever)
+        self.mock_server_thread.setDaemon(True)
+        self.mock_server_thread.start()
+
+    def test_cookie_parser(self):
+        """Not directly testing HTTPie but `requests` to ensure their cookies handling
+        is still as expected by `get_expired_cookies()`.
+        """
+
+        class MockServerRequestHandler(BaseHTTPRequestHandler):
+            """"HTTP request handler."""
+
+            def do_GET(self):
+                """Handle GET requests."""
+                # Craft multiple cookies
+                cookie = SimpleCookie()
+                cookie['hello'] = 'world'
+                cookie['hello']['path'] = self.path
+                cookie['oatmeal_raisin'] = 'is the best'
+                cookie['oatmeal_raisin']['path'] = self.path
+
+                # Send HTTP headers
+                self.send_response(200)
+                self.send_header('Set-Cookie', cookie.output())
+                self.end_headers()
+
+        self.setup_mock_server(MockServerRequestHandler)
+        response = http(f'http://localhost:{self.mock_server_port}/')
+        assert 'Set-Cookie: hello=world; Path=/' in response
+        assert 'Set-Cookie: oatmeal_raisin="is the best"; Path=/' in response

tests/test_sessions.py

@@ -343,22 +343,17 @@ class TestExpiredCookies(CookieTestBase):
         assert 'cookie2' not in updated_session['cookies']
 
     def test_get_expired_cookies_using_max_age(self):
-        headers = [
-            ('Set-Cookie', 'one=two; Max-Age=0; path=/; domain=.tumblr.com; HttpOnly')
-        ]
+        cookies = 'one=two; Max-Age=0; path=/; domain=.tumblr.com; HttpOnly'
         expected_expired = [
             {'name': 'one', 'path': '/'}
         ]
-        assert get_expired_cookies(headers, now=None) == expected_expired
+        assert get_expired_cookies(cookies, now=None) == expected_expired
 
     @pytest.mark.parametrize(
-        argnames=['headers', 'now', 'expected_expired'],
+        argnames=['cookies', 'now', 'expected_expired'],
         argvalues=[
             (
-                [
-                    ('Set-Cookie', 'hello=world; Path=/; Expires=Thu, 01-Jan-1970 00:00:00 GMT; HttpOnly'),
-                    ('Connection', 'keep-alive')
-                ],
+                'hello=world; Path=/; Expires=Thu, 01-Jan-1970 00:00:00 GMT; HttpOnly',
                 None,
                 [
                     {
@@ -368,11 +363,10 @@ class TestExpiredCookies(CookieTestBase):
                 ]
             ),
             (
-                [
-                    ('Set-Cookie', 'hello=world; Path=/; Expires=Thu, 01-Jan-1970 00:00:00 GMT; HttpOnly'),
-                    ('Set-Cookie', 'pea=pod; Path=/ab; Expires=Thu, 01-Jan-1970 00:00:00 GMT; HttpOnly'),
-                    ('Connection', 'keep-alive')
-                ],
+                (
+                    'hello=world; Path=/; Expires=Thu, 01-Jan-1970 00:00:00 GMT; HttpOnly, '
+                    'pea=pod; Path=/ab; Expires=Thu, 01-Jan-1970 00:00:00 GMT; HttpOnly'
+                ),
                 None,
                 [
                     {'name': 'hello', 'path': '/'},
@@ -382,24 +376,19 @@ class TestExpiredCookies(CookieTestBase):
             (
                 # Checks we gracefully ignore expires date in invalid format.
                 # <https://github.com/httpie/httpie/issues/963>
-                [
-                    ('Set-Cookie', 'pfg=; Expires=Sat, 19-Sep-2020 06:58:14 GMT+0000; Max-Age=0; path=/; domain=.tumblr.com; secure; HttpOnly'),
-                ],
+                'pfg=; Expires=Sat, 19-Sep-2020 06:58:14 GMT+0000; Max-Age=0; path=/; domain=.tumblr.com; secure; HttpOnly',
                 None,
                 []
             ),
             (
-                [
-                    ('Set-Cookie', 'hello=world; Path=/; Expires=Fri, 12 Jun 2020 12:28:55 GMT; HttpOnly'),
-                    ('Connection', 'keep-alive')
-                ],
+                'hello=world; Path=/; Expires=Fri, 12 Jun 2020 12:28:55 GMT; HttpOnly',
                 datetime(2020, 6, 11).timestamp(),
                 []
             ),
         ]
     )
-    def test_get_expired_cookies_manages_multiple_cookie_headers(self, headers, now, expected_expired):
-        assert get_expired_cookies(headers, now=now) == expected_expired
+    def test_get_expired_cookies_manages_multiple_cookie_headers(self, cookies, now, expected_expired):
+        assert get_expired_cookies(cookies, now=now) == expected_expired
 
 
 class TestCookieStorage(CookieTestBase):

tests/test_transport_plugin.py

@@ -0,0 +1,45 @@
+from io import BytesIO
+
+from requests.adapters import BaseAdapter
+from requests.models import Response
+from requests.utils import get_encoding_from_headers
+
+from httpie.plugins import TransportPlugin
+from httpie.plugins.registry import plugin_manager
+
+from .utils import HTTP_OK, http
+
+SCHEME = 'http+fake'
+
+
+class FakeAdapter(BaseAdapter):
+    def send(self, request, **kwargs):
+        response = Response()
+        response.status_code = 200
+        response.reason = 'OK'
+        response.headers = {
+            'Content-Type': 'text/html; charset=UTF-8',
+        }
+        response.encoding = get_encoding_from_headers(response.headers)
+        response.raw = BytesIO(b'<!doctype html><html>Hello</html>')
+        return response
+
+
+class FakeTransportPlugin(TransportPlugin):
+    name = 'Fake Transport'
+
+    prefix = SCHEME
+
+    def get_adapter(self):
+        return FakeAdapter()
+
+
+def test_transport_from_requests_response(httpbin):
+    plugin_manager.register(FakeTransportPlugin)
+    try:
+        r = http(f'{SCHEME}://example.com')
+        assert HTTP_OK in r
+        assert 'Hello' in r
+        assert 'Content-Type: text/html; charset=UTF-8' in r
+    finally:
+        plugin_manager.unregister(FakeTransportPlugin)