fix(dl): repair damaged cookie files without rewriting them

Space separators, a BOM, a missing header and #HttpOnly_ rows now load; repairs happen in memory and an unparseable file raises instead of loading as empty.

docs/getting-started/quickstart.md

@@ -132,6 +132,11 @@ Services that require a login read either **cookies** or **credentials**.
   `cookies/{SERVICE}/default.txt`. So unshackle finds a file at `cookies/EXAMPLE.txt`
   automatically for the `EXAMPLE` service.
 
+    unshackle repairs the usual export damage in memory and does not change your
+    file: a missing header line, a byte-order mark, spaces in place of tabs, and
+    rows a text editor broke. unshackle refuses a JSON export. Export the cookies
+    again in Netscape format.
+
 - **Credentials**: store a username and password per service in your config:
 
     ```yaml title="unshackle.yaml"

tests/commands/test_cookie_loading.py

@@ -0,0 +1,145 @@
+"""Cookie files that use spaces instead of tabs must still load."""
+
+from http.cookiejar import CookieJar
+from pathlib import Path
+
+import pytest
+
+from unshackle.commands.dl import dl
+
+HEADER = "# Netscape HTTP Cookie File\n"
+
+
+def _names(jar) -> dict:
+    return {c.name: c.value for c in jar}
+
+
+def test_space_separated_cookie_file(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + ".example.com TRUE / TRUE 1234567890 sid abc123\n", "utf8")
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+
+
+def test_tab_separated_value_keeps_spaces(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(
+        HEADER + "\t".join([".example.com", "TRUE", "/", "TRUE", "1234567890", "sid", "a b c"]) + "\n", "utf8"
+    )
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "a b c"}
+
+
+def test_space_separated_empty_value(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + ".example.com TRUE / TRUE 1234567890 sid \n", "utf8")
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": ""}
+
+
+def test_space_separated_httponly(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + "#HttpOnly_.example.com TRUE / TRUE 1234567890 sid abc123\n", "utf8")
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+
+
+def test_mixed_separators_in_one_file(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(
+        HEADER
+        + "\t".join([".a.com", "TRUE", "/", "TRUE", "1234567890", "tabbed", "1"])
+        + "\n.b.com TRUE / TRUE 1234567890 spaced 2\n",
+        "utf8",
+    )
+
+    assert _names(dl.load_cookie_file(file)) == {"tabbed": "1", "spaced": "2"}
+
+
+def test_malformed_row_is_reported(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + ".a.com\tTRUE\t/\tTRUE\t1\tsid\tv\nnot a cookie\n", "utf8")
+
+    with pytest.raises(ValueError, match="line 3"):
+        dl.load_cookie_file(file)
+
+
+def test_missing_header_is_added(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(".example.com\tTRUE\t/\tTRUE\t1234567890\tsid\tabc123\n", "utf8")
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+
+
+def test_utf8_bom_is_ignored(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_bytes(b"\xef\xbb\xbf" + (HEADER + ".example.com\tTRUE\t/\tTRUE\t1234567890\tsid\tabc123\n").encode())
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+
+
+def test_json_export_gives_clear_error(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text('[{"domain": ".example.com", "name": "sid", "value": "abc123"}]\n', "utf8")
+
+    with pytest.raises(ValueError, match="JSON export"):
+        dl.load_cookie_file(file)
+
+
+def test_leading_blank_line_is_not_json(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text("\n" + HEADER + ".example.com TRUE / TRUE 1234567890 sid abc123\n", "utf8")
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+
+
+def test_dot_flag_mismatch_does_not_abort_file(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + "example.com\tTRUE\t/\tTRUE\t1\tsid\tv\n", "utf8")
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "v"}
+
+
+def test_the_users_file_is_never_rewritten(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    original = ".example.com TRUE / TRUE 1234567890 sid abc123\n"
+    file.write_text(original, "utf8")
+
+    dl.load_cookie_file(file)
+
+    assert file.read_text("utf8") == original
+
+
+def test_save_cookies_onto_an_unrepaired_file(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(".example.com TRUE / TRUE 1234567890 sid abc123\n", "utf8")
+
+    dl.save_cookies(file, CookieJar())
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+
+
+def test_tabbed_row_is_never_resplit_on_spaces(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + ".a.com\tTRUE\t/\tFALSE\thello world\n", "utf8")
+
+    with pytest.raises(ValueError, match="not a Netscape cookie row"):
+        dl.load_cookie_file(file)
+
+
+def test_file_without_cookies_is_reported(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + "# nothing else here\n", "utf8")
+
+    with pytest.raises(ValueError, match="holds no cookies"):
+        dl.load_cookie_file(file)
+
+
+def test_round_trip_keeps_httponly_and_blank_expiry(tmp_path: Path) -> None:
+    file = tmp_path / "cookies.txt"
+    file.write_text(HEADER + "#HttpOnly_.example.com TRUE / TRUE 1234567890 sid abc123\n", "utf8")
+
+    dl.save_cookies(file, CookieJar())
+
+    assert _names(dl.load_cookie_file(file)) == {"sid": "abc123"}
+    assert "#HttpOnly_" in file.read_text("utf8")

unshackle/commands/dl.py

@@ -19,6 +19,7 @@ from copy import deepcopy
 from datetime import date, timedelta
 from functools import partial
 from http.cookiejar import CookieJar, MozillaCookieJar
+from io import StringIO
 from itertools import product
 from pathlib import Path
 from threading import Lock
@@ -4674,15 +4675,28 @@ class dl:
     def load_cookie_file(cookie_file: Path) -> MozillaCookieJar:
         """Load a Netscape cookie file with every expiry blanked, so no layer drops an expired cookie."""
         cookie_jar = MozillaCookieJar(cookie_file)
-        cookie_data = html.unescape(cookie_file.read_text("utf8")).splitlines(keepends=False)
+        cookie_data = html.unescape(cookie_file.read_text("utf-8-sig")).splitlines(keepends=False)
+        head = next((line.lstrip() for line in cookie_data if line.strip()), "")
+        if head[:1] in ("[", "{"):
+            raise ValueError(f"{cookie_file} is a JSON export; export it again in Netscape format")
         for i, line in enumerate(cookie_data):
-            if line and not line.startswith("#"):
-                line_data = line.lstrip().split("\t")
-                # Even under ignore_expires=True, requests may still skip an expired cookie
-                line_data[4] = ""
-                cookie_data[i] = "\t".join(line_data)
-        cookie_file.write_text("\n".join(cookie_data), "utf8")
-        cookie_jar.load(ignore_discard=True, ignore_expires=True)
+            body = line.lstrip()
+            prefix = "#HttpOnly_" if body.startswith("#HttpOnly_") else ""
+            body = body[len(prefix) :]
+            if not body or body.startswith("#"):
+                continue
+            line_data = body.split("\t") if "\t" in body else body.split(None, 6)
+            if len(line_data) == 6:
+                line_data.append("")
+            if len(line_data) != 7:
+                raise ValueError(f"{cookie_file} line {i + 1} is not a Netscape cookie row: {line!r}")
+            line_data[4] = ""
+            line_data[1] = str(line_data[0].startswith(".")).upper()
+            cookie_data[i] = prefix + "\t".join(line_data)
+        cookie_data.insert(0, "# Netscape HTTP Cookie File")
+        cookie_jar._really_load(StringIO("\n".join(cookie_data)), str(cookie_file), True, True)  # type: ignore[attr-defined]
+        if not len(cookie_jar):
+            raise ValueError(f"{cookie_file} holds no cookies; export it again in Netscape format")
         return cookie_jar
 
     @staticmethod
@@ -4690,8 +4704,7 @@ class dl:
         if hasattr(cookies, "jar"):
             cookies = cookies.jar
 
-        cookie_jar = MozillaCookieJar(path)
-        cookie_jar.load(ignore_discard=True, ignore_expires=True)
+        cookie_jar = dl.load_cookie_file(path) if path.exists() else MozillaCookieJar(path)
         for cookie in cookies:
             if not cookie.name or not cookie.domain:
                 continue