Apply suggestions from the review

SECURITY.md

@@ -0,0 +1,10 @@
+# Security Policy
+
+## Reporting a Vulnerability
+
+To report a vulnerability, please send an email to `security@httpie.io` describing the:
+
+- The description of the vulnerability itself
+- A short reproducer to verify it (you can submit a small HTTP server, a shell script, a docker image etc.)
+- The severity level classification (`LOW`/`MEDIUM`/`HIGH`/`CRITICAL`)
+- If associated with any, the [CWE](https://cwe.mitre.org/) ID.

docs/README.md

@@ -2422,23 +2422,6 @@ And since there’s neither data nor `EOF`, it will get stuck. So unless you’r
 
 Also, it might be good to set a connection `--timeout` limit to prevent your program from hanging if the server never responds.
 
-### Security
-
-#### Exposure of Cookies To The 3rd Party Hosts On Redirects
-
-*Vulnerability Type*: [CWE-200](https://cwe.mitre.org/data/definitions/200.html)
-*Severity Level*: LOW
-*Affected Versions*: `<3.1.0`
-
-The handling of [cookies](#cookies) was not compatible with the [RFC 6265](https://datatracker.ietf.org/doc/html/rfc6265)
-on the point of handling the `Domain` attribute when they were saved into [session](#sessions) files. All cookies were shared
-across all hosts during the runtime, including redirects to the 3rd party hosts.
-
-This vulnerability has been fixed in [3.1.0](https://github.com/httpie/httpie/releases/tag/3.1.0) and the
-[`httpie cli sessions upgrade`](#upgrading-sessions)/[`httpie cli sessions upgrade-all`]((#upgrading-sessions) commands
-have been put in place in order to allow a smooth transition to the new session layout from the existing [session](#sessions)
-files.
-
 ## Plugin manager
 
 HTTPie offers extensibility through a [plugin API](https://github.com/httpie/httpie/blob/master/httpie/plugins/base.py),

httpie/legacy/cookie_format.py

@@ -0,0 +1,103 @@
+import argparse
+from typing import Any, Type, List, Dict, TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from httpie.sessions import Session
+
+INSECURE_COOKIE_JAR_WARNING = '''\
+Outdated layout detected for the current session. Please consider updating it,
+in order to not get affected by potential security problems.
+
+For fixing the current session:
+
+    With binding all cookies to the current host (secure):
+        $ httpie cli sessions upgrade --bind-cookies {hostname} {session_id}
+
+    Without binding cookies (leaving them as is) (insecure):
+        $ httpie cli sessions upgrade {hostname} {session_id}
+'''
+
+
+INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS = '''\
+
+For fixing all named sessions:
+
+    With binding all cookies to the current host (secure):
+        $ httpie cli sessions upgrade-all --bind-cookies
+
+    Without binding cookies (leaving them as is) (insecure):
+        $ httpie cli sessions upgrade-all
+'''
+
+INSECURE_COOKIE_SECURITY_LINK = '\nSee https://pie.co/docs/security for more information.'
+
+
+def pre_process(session: 'Session', cookies: Any) -> List[Dict[str, Any]]:
+    """Load the given cookies to the cookie jar while maintaining
+    support for the old cookie layout."""
+
+    is_old_style = isinstance(cookies, dict)
+    if is_old_style:
+        normalized_cookies = [
+            {
+                'name': key,
+                **value
+            }
+            for key, value in cookies.items()
+        ]
+    else:
+        normalized_cookies = cookies
+
+    should_issue_warning = is_old_style and any(
+        cookie.get('domain', '') == ''
+        for cookie in normalized_cookies
+    )
+
+    if should_issue_warning and not session.refactor_mode:
+        warning = INSECURE_COOKIE_JAR_WARNING.format(hostname=session.bound_host, session_id=session.session_id)
+        if not session.is_anonymous:
+            warning += INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS
+        warning += INSECURE_COOKIE_SECURITY_LINK
+
+        session.env.log_error(
+            warning,
+            level='warning'
+        )
+
+    return normalized_cookies
+
+
+def post_process(
+    normalized_cookies: List[Dict[str, Any]],
+    *,
+    original_type: Type[Any]
+) -> Any:
+    """Convert the cookies to their original format for
+    maximum compatibility."""
+
+    if issubclass(original_type, dict):
+        return {
+            cookie.pop('name'): cookie
+            for cookie in normalized_cookies
+        }
+    else:
+        return normalized_cookies
+
+
+def fix_layout(session: 'Session', hostname: str, args: argparse.Namespace) -> None:
+    if not isinstance(session['cookies'], dict):
+        return None
+
+    session['cookies'] = [
+        {
+            'name': key,
+            **value
+        }
+        for key, value in session['cookies'].items()
+    ]
+    for cookie in session.cookies:
+        if cookie.domain == '':
+            if args.bind_cookies:
+                cookie.domain = hostname
+            else:
+                cookie._rest['is_explicit_none'] = True

httpie/manager/cli.py

@@ -4,7 +4,7 @@ from httpie import __version__
 
 CLI_SESSION_UPGRADE_FLAGS = [
     {
-        'variadic': ['--bind-cookies'],
+        'flags': ['--bind-cookies'],
         'action': 'store_true',
         'default': False,
         'help': 'Bind domainless cookies to the host that session belongs.'
@@ -102,8 +102,8 @@ def generate_subparsers(root, parent_parser, definitions):
 
         for argument in properties:
             argument = argument.copy()
-            variadic = argument.pop('variadic', [])
+            flags = argument.pop('flags', [])
-            command_parser.add_argument(*variadic, **argument)
+            command_parser.add_argument(*flags, **argument)
 
 
 parser = HTTPieManagerArgumentParser(

httpie/manager/tasks.py

@@ -1,9 +1,10 @@
 import argparse
 from typing import TypeVar, Callable, Tuple
 
-from httpie.sessions import SESSIONS_DIR_NAME, Session, get_httpie_session
+from httpie.sessions import SESSIONS_DIR_NAME, get_httpie_session
 from httpie.status import ExitStatus
 from httpie.context import Environment
+from httpie.legacy import cookie_format as legacy_cookies
 from httpie.manager.cli import missing_subcommand, parser
 
 T = TypeVar('T')
@@ -51,27 +52,8 @@ def is_version_greater(version_1: str, version_2: str) -> bool:
     return split_version(version_1) > split_version(version_2)
 
 
-def fix_cookie_layout(session: Session, hostname: str, args: argparse.Namespace) -> None:
-    if not isinstance(session['cookies'], dict):
-        return None
-
-    session['cookies'] = [
-        {
-            'name': key,
-            **value
-        }
-        for key, value in session['cookies'].items()
-    ]
-    for cookie in session.cookies:
-        if cookie.domain == '':
-            if args.bind_cookies:
-                cookie.domain = hostname
-            else:
-                cookie._rest['is_explicit_none'] = True
-
-
 FIXERS_TO_VERSIONS = {
-    '3.1.0': fix_cookie_layout
+    '3.1.0': legacy_cookies.fix_layout
 }
 
 

httpie/sessions.py

@@ -8,7 +8,7 @@ import re
 from http.cookies import SimpleCookie
 from http.cookiejar import Cookie
 from pathlib import Path
-from typing import Any, Dict, Optional, Union
+from typing import Any, Dict, List, Optional, Union
 
 from requests.auth import AuthBase
 from requests.cookies import RequestsCookieJar, remove_cookie_by_name
@@ -18,6 +18,7 @@ from .cli.dicts import HTTPHeadersDict
 from .config import BaseConfigDict, DEFAULT_CONFIG_DIR
 from .utils import url_as_host
 from .plugins.registry import plugin_manager
+from .legacy import cookie_format as legacy_cookies
 
 
 SESSIONS_DIR_NAME = 'sessions'
@@ -32,37 +33,25 @@ SESSION_IGNORED_HEADER_PREFIXES = ['Content-', 'If-']
 KEPT_COOKIE_OPTIONS = ['name', 'expires', 'path', 'value', 'domain', 'secure']
 DEFAULT_COOKIE_PATH = '/'
 
-INSECURE_COOKIE_JAR_WARNING = '''\
-Outdated layout detected for the current session. Please consider updating it,
-in order to not get affected by potential security problems.
-
-For fixing the current session:
-
-    With binding all cookies to the current host (secure):
-        $ httpie cli sessions upgrade --bind-cookies {hostname} {session_id}
-
-    Without binding cookies (leaving them as is) (insecure):
-        $ httpie cli sessions upgrade {hostname} {session_id}
-'''
-
-INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS = '''\
-
-For fixing all named sessions:
-
-    With binding all cookies to the current host (secure):
-        $ httpie cli sessions upgrade-all --bind-cookies
-
-    Without binding cookies (leaving them as is) (insecure):
-        $ httpie cli sessions upgrade-all
-
-See https://pie.co/docs/security for more information.
-'''
-
 
 def is_anonymous_session(session_name: str) -> bool:
     return os.path.sep in session_name
 
 
+def session_hostname_to_dirname(hostname: str, session_name: str) -> str:
+    # host:port => host_port
+    hostname = hostname.replace(':', '_')
+    return os.path.join(
+        SESSIONS_DIR_NAME,
+        hostname,
+        f'{session_name}.json'
+    )
+
+
+def strip_port(hostname: str) -> str:
+    return hostname.split(':')[0]
+
+
 def materialize_cookie(cookie: Cookie) -> Dict[str, Any]:
     materialized_cookie = {
         option: getattr(cookie, option)
@@ -92,22 +81,18 @@ def get_httpie_session(
         # HACK/FIXME: httpie-unixsocket's URLs have no hostname.
         bound_hostname = 'localhost'
 
-    # host:port => host_port
-    hostname = bound_hostname.replace(':', '_')
     if is_anonymous_session(session_name):
         path = os.path.expanduser(session_name)
         session_id = path
     else:
-        path = (
+        path = config_dir / session_hostname_to_dirname(bound_hostname, session_name)
-            config_dir / SESSIONS_DIR_NAME / hostname / f'{session_name}.json'
-        )
         session_id = session_name
 
     session = Session(
         path,
         env=env,
         session_id=session_id,
-        bound_host=bound_hostname.split(':')[0],
+        bound_host=strip_port(bound_hostname),
         refactor_mode=refactor_mode
     )
     session.load()
@@ -142,60 +127,35 @@ class Session(BaseConfigDict):
 
     def pre_process_data(self, data: Dict[str, Any]) -> Dict[str, Any]:
         cookies = data.get('cookies')
-        if isinstance(cookies, dict):
+        if cookies:
-            normalized_cookies = [
+            normalized_cookies = legacy_cookies.pre_process(self, cookies)
-                {
-                    'name': key,
-                    **value
-                }
-                for key, value in cookies.items()
-            ]
-        elif isinstance(cookies, list):
-            normalized_cookies = cookies
         else:
             normalized_cookies = []
 
-        should_issue_warning = False
         for cookie in normalized_cookies:
             domain = cookie.get('domain', '')
-            if domain == '' and isinstance(cookies, dict):
+            if domain is None:
-                should_issue_warning = True
-            elif domain is None:
                 # domain = None means explicitly lack of cookie, though
-                # requests requires domain to be string so we'll cast it
+                # requests requires domain to be a string so we'll cast it
                 # manually.
                 cookie['domain'] = ''
                 cookie['rest'] = {'is_explicit_none': True}
 
             self.cookie_jar.set(**cookie)
 
-        if should_issue_warning and not self.refactor_mode:
-            warning = INSECURE_COOKIE_JAR_WARNING.format(hostname=self.bound_host, session_id=self.session_id)
-            if not is_anonymous_session(self.session_id):
-                warning += INSECURE_COOKIE_JAR_WARNING_FOR_NAMED_SESSIONS
-
-            self.env.log_error(
-                warning,
-                level='warning'
-            )
-
         return data
 
     def post_process_data(self, data: Dict[str, Any]) -> Dict[str, Any]:
         cookies = data.get('cookies')
-        # Save in the old-style fashion
 
         normalized_cookies = [
             materialize_cookie(cookie)
             for cookie in self.cookie_jar
         ]
-        if isinstance(cookies, dict):
+        data['cookies'] = legacy_cookies.post_process(
-            data['cookies'] = {
+            normalized_cookies,
-                cookie.pop('name'): cookie
+            original_type=type(cookies)
-                for cookie in normalized_cookies
+        )
-            }
-        else:
-            data['cookies'] = normalized_cookies
 
         return data
 
@@ -251,7 +211,7 @@ class Session(BaseConfigDict):
     def cookies(self, jar: RequestsCookieJar):
         self.cookie_jar = jar
 
-    def remove_cookies(self, cookies: Dict[str, str]):
+    def remove_cookies(self, cookies: List[Dict[str, str]]):
         for cookie in cookies:
             remove_cookie_by_name(
                 self.cookie_jar,
@@ -293,3 +253,7 @@ class Session(BaseConfigDict):
     def auth(self, auth: dict):
         assert {'type', 'raw_auth'} == auth.keys()
         self['auth'] = auth
+
+    @property
+    def is_anonymous(self):
+        return is_anonymous_session(self.session_id)