From 147a066dbecaf0a51bc2b554d8163201daf441ca Mon Sep 17 00:00:00 2001 From: Ilya Sukhanov Date: Tue, 6 Jul 2021 15:00:06 -0400 Subject: [PATCH] Add internal support for file-like object responses to improve adapter plugin support (#1094) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 Co-authored-by: Jakub Roztocil --- CHANGELOG.rst | 1 + httpie/client.py | 3 +-- httpie/models.py | 25 +++++++++++++----- httpie/utils.py | 22 +++++++++++++--- tests/test_cookie.py | 47 ++++++++++++++++++++++++++++++++++ tests/test_sessions.py | 35 +++++++++---------------- tests/test_transport_plugin.py | 45 ++++++++++++++++++++++++++++++++ 7 files changed, 143 insertions(+), 35 deletions(-) create mode 100644 tests/test_cookie.py create mode 100644 tests/test_transport_plugin.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2c59d9ca..b92847f8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,7 @@ This project adheres to `Semantic Versioning `_. 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) diff --git a/httpie/client.py b/httpie/client.py index f959b4df..af486722 100644 --- a/httpie/client.py +++ b/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 diff --git a/httpie/models.py b/httpie/models.py index c30f06e1..a90b6a13 100644 --- a/httpie/models.py +++ b/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 diff --git a/httpie/utils.py b/httpie/utils.py index c7d1beab..d828d488 100644 --- a/httpie/utils.py +++ b/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]) diff --git a/tests/test_cookie.py b/tests/test_cookie.py new file mode 100644 index 00000000..c2a97465 --- /dev/null +++ b/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 diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 8bd4d479..be249292 100644 --- a/tests/test_sessions.py +++ b/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. # - [ - ('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): diff --git a/tests/test_transport_plugin.py b/tests/test_transport_plugin.py new file mode 100644 index 00000000..b71592df --- /dev/null +++ b/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'Hello') + 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)