[PATCH] plugin.api.http_session: remove parse_* methods(streamlink#4803)

The `parse_{cookies,headers,query_params}` methods were added when the
subclass of `requests.Session` was implemented in order to support
setting cookies, headers and query parameters via `k1=v1;k2=v2` strings
(in addition to key-value dicts) via the session API and via the CLI:
- 936e66d
- c6e54fd

Since these methods implement logic purely for the `Streamlink` session
interface and are not meant to be called by any plugin or stream
implementations which use the session's `HTTPSession` instance, they
should be removed. Cookies, headers and query string parameters should
be set directly on their respective `HTTPSession` attributes:
- `cookies`: instance of `requests.cookies.RequestsCookieJar`
- `headers`: instance of `requests.structures.CaseInsensitiveDict`
- `params`: instance of `dict`

Also, at least in regards to HTTP headers, the `key=value` syntax
does not reflect the syntax of raw HTTP requests/responses or interfaces
of other tools like cURL, etc., so having these methods on the
`HTTPSession` class makes it unnecessarily confusing. The method names
themselves are also confusing, as they suggest that the input gets
parsed and that some result gets returned, which is wrong.

This commit therefore moves the `k1=v1;k2=v2` string logic from the
`http_session` module to the `session` module where it belongs and it
also simplifies the option setter.

src/streamlink/plugin/api/http_session.py

@@ -89,15 +89,6 @@ def __len__(self):
     urllib3.util.url.PERCENT_RE = Urllib3UtilUrlPercentReOverride
 
 
-def _parse_keyvalue_list(val):
-    for keyvalue in val.split(";"):
-        try:
-            key, value = keyvalue.split("=", 1)
-            yield key.strip(), value.strip()
-        except ValueError:
-            continue
-
-
 # requests.Request.__init__ keywords, except for "hooks"
 _VALID_REQUEST_ARGS = "method", "url", "headers", "files", "data", "params", "auth", "cookies", "json"
 
@@ -148,30 +139,6 @@ def xml(cls, res, *args, **kwargs):
         """Parses XML from a response."""
         return parse_xml(res.text, *args, **kwargs)
 
-    def parse_cookies(self, cookies, **kwargs):
-        """Parses a semi-colon delimited list of cookies.
-
-        Example: foo=bar;baz=qux
-        """
-        for name, value in _parse_keyvalue_list(cookies):
-            self.cookies.set(name, value, **kwargs)
-
-    def parse_headers(self, headers):
-        """Parses a semi-colon delimited list of headers.
-
-        Example: foo=bar;baz=qux
-        """
-        for name, value in _parse_keyvalue_list(headers):
-            self.headers[name] = value
-
-    def parse_query_params(self, cookies, **kwargs):
-        """Parses a semi-colon delimited list of query parameters.
-
-        Example: foo=bar;baz=qux
-        """
-        for name, value in _parse_keyvalue_list(cookies):
-            self.params[name] = value
-
     def resolve_url(self, url):
         """Resolves any redirects and returns the final URL."""
         return self.get(url, stream=True).url

src/streamlink/session.py

@@ -5,10 +5,7 @@
 import traceback
 from collections import OrderedDict
 from socket import AF_INET, AF_INET6
-try:
-    from typing import Tuple, Type
-except ImportError:
-    pass
+from typing import Iterator, Tuple, Type
 
 import requests
 import requests.packages.urllib3.util.connection as urllib3_connection
@@ -28,6 +25,9 @@
 logging.setLoggerClass(StreamlinkLogger)
 log = logging.getLogger(__name__)
 
+# options which support `key1=value1;key2=value2;...` strings as value
+_OPTIONS_HTTP_KEYEQUALSVALUE = {"http-cookies": "cookies", "http-headers": "headers", "http-query-params": "params"}
+
 
 def print_small_exception(start_after):
     type, value, traceback_ = sys.exc_info()
@@ -49,6 +49,16 @@ def print_small_exception(start_after):
     sys.stderr.write("\n")
 
 
+def _parse_keyvalue_string(value):
+    # type: (str) -> Iterator[Tuple[str, str]]
+    for keyval in value.split(";"):
+        try:
+            key, val = keyval.split("=", 1)
+            yield key.strip(), val.strip()
+        except ValueError:
+            continue
+
+
 class PythonDeprecatedWarning(UserWarning):
     pass
 
@@ -267,21 +277,11 @@ def set_option(self, key, value):
             if key == "https-proxy":
                 log.info("The https-proxy option has been deprecated in favour of a single http-proxy option")
 
-        elif key == "http-cookies":
-            if isinstance(value, dict):
-                self.http.cookies.update(value)
-            else:
-                self.http.parse_cookies(value)
-        elif key == "http-headers":
-            if isinstance(value, dict):
-                self.http.headers.update(value)
-            else:
-                self.http.parse_headers(value)
-        elif key == "http-query-params":
-            if isinstance(value, dict):
-                self.http.params.update(value)
-            else:
-                self.http.parse_query_params(value)
+        elif key in _OPTIONS_HTTP_KEYEQUALSVALUE:
+            getattr(self.http, _OPTIONS_HTTP_KEYEQUALSVALUE[key]).update(
+                value if isinstance(value, dict) else dict(_parse_keyvalue_string(value))
+            )
+
         elif key == "http-trust-env":
             self.http.trust_env = value
         elif key == "http-ssl-verify":

tests/test_session.py

@@ -3,6 +3,7 @@
 import unittest
 from socket import AF_INET, AF_INET6
 
+import pytest
 import requests_mock
 from requests.packages.urllib3.util.connection import allowed_gai_family
 
@@ -16,6 +17,12 @@
 from tests.mock import Mock, call, patch
 
 
+@pytest.fixture
+def session():
+    with patch("streamlink.session.Streamlink.load_builtin_plugins"):
+        yield Streamlink()
+
+
 class EmptyPlugin(Plugin):
     def _get_streams(self):
         pass  # pragma: no cover
@@ -389,46 +396,83 @@ def test_ipv4_ipv6(self, mock_urllib3_connection):
         self.assertEqual(session.get_option("ipv6"), False)
         self.assertEqual(mock_urllib3_connection.allowed_gai_family, allowed_gai_family)
 
-    def test_https_proxy_default(self):
-        session = Streamlink()
+
+class TestSessionOptionHttpProxy:
+    @pytest.fixture
+    def no_deprecation(self, caplog):
+        # type: (pytest.LogCaptureFixture)
+        yield
+        assert not caplog.get_records("call")
+
+    @pytest.fixture
+    def logs_deprecation(self, caplog):
+        # type: (pytest.LogCaptureFixture)
+        yield
+        # TODO: streamlink#4785 not yet implemented
+        # assert [(record.levelname, record.message) for record in caplog.get_records("call")] == [
+        #    ("warning", "The https-proxy option has been deprecated in favor of a single http-proxy option"),
+        # ]
+
+    def test_https_proxy_default(self, session, no_deprecation):
         session.set_option("http-proxy", "http://testproxy.com")
 
-        self.assertEqual("http://testproxy.com", session.http.proxies['http'])
-        self.assertEqual("http://testproxy.com", session.http.proxies['https'])
+        assert session.http.proxies["http"] == "http://testproxy.com"
+        assert session.http.proxies["https"] == "http://testproxy.com"
 
-    def test_https_proxy_set_first(self):
-        session = Streamlink()
+    def test_https_proxy_set_first(self, session, logs_deprecation):
         session.set_option("https-proxy", "https://testhttpsproxy.com")
         session.set_option("http-proxy", "http://testproxy.com")
 
-        self.assertEqual("http://testproxy.com", session.http.proxies['http'])
-        self.assertEqual("http://testproxy.com", session.http.proxies['https'])
+        assert session.http.proxies["http"] == "http://testproxy.com"
+        assert session.http.proxies["https"] == "http://testproxy.com"
 
-    def test_https_proxy_default_override(self):
-        session = Streamlink()
+    def test_https_proxy_default_override(self, session, logs_deprecation):
         session.set_option("http-proxy", "http://testproxy.com")
         session.set_option("https-proxy", "https://testhttpsproxy.com")
 
-        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['http'])
-        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['https'])
+        assert session.http.proxies["http"] == "https://testhttpsproxy.com"
+        assert session.http.proxies["https"] == "https://testhttpsproxy.com"
 
-    def test_https_proxy_set_only(self):
-        session = Streamlink()
+    def test_https_proxy_set_only(self, session, logs_deprecation):
         session.set_option("https-proxy", "https://testhttpsproxy.com")
 
-        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['http'])
-        self.assertEqual("https://testhttpsproxy.com", session.http.proxies['https'])
+        assert session.http.proxies["http"] == "https://testhttpsproxy.com"
+        assert session.http.proxies["https"] == "https://testhttpsproxy.com"
 
-    def test_http_proxy_socks(self):
-        session = self.subject(load_plugins=False)
+    def test_http_proxy_socks(self, session, no_deprecation):
         session.set_option("http-proxy", "socks5://localhost:1234")
 
-        self.assertEqual("socks5://localhost:1234", session.http.proxies["http"])
-        self.assertEqual("socks5://localhost:1234", session.http.proxies["https"])
+        assert session.http.proxies["http"] == "socks5://localhost:1234"
+        assert session.http.proxies["https"] == "socks5://localhost:1234"
 
-    def test_https_proxy_socks(self):
-        session = self.subject(load_plugins=False)
+    def test_https_proxy_socks(self, session, logs_deprecation):
         session.set_option("https-proxy", "socks5://localhost:1234")
 
-        self.assertEqual("socks5://localhost:1234", session.http.proxies["http"])
-        self.assertEqual("socks5://localhost:1234", session.http.proxies["https"])
+        assert session.http.proxies["http"] == "socks5://localhost:1234"
+        assert session.http.proxies["https"] == "socks5://localhost:1234"
+
+
+@pytest.mark.parametrize("option", [
+    pytest.param(("http-cookies", "cookies"), id="http-cookies"),
+    pytest.param(("http-headers", "headers"), id="http-headers"),
+    pytest.param(("http-query-params", "params"), id="http-query-params"),
+], indirect=True)
+class TestOptionsKeyEqualsValue:
+    @pytest.fixture
+    def option(self, request, session):
+        option, attr = request.param
+        httpsessionattr = getattr(session.http, attr)
+        assert session.get_option(option) is httpsessionattr
+        assert "foo" not in httpsessionattr
+        assert "bar" not in httpsessionattr
+        yield option
+        assert httpsessionattr.get("foo") == "foo=bar"
+        assert httpsessionattr.get("bar") == "123"
+
+    def test_dict(self, session, option):
+        # type: (Streamlink, str)
+        session.set_option(option, {"foo": "foo=bar", "bar": "123"})
+
+    def test_string(self, session, option):
+        # type: (Streamlink, str)
+        session.set_option(option, "foo=foo=bar;bar=123;baz")