Skip to content

[WIP] Implement cookie_filename support in check_features_manually#2807

Draft
yyq1043-cloud wants to merge 1 commit into
soxoj:mainfrom
yyq1043-cloud:feat/use-cookie-filename
Draft

[WIP] Implement cookie_filename support in check_features_manually#2807
yyq1043-cloud wants to merge 1 commit into
soxoj:mainfrom
yyq1043-cloud:feat/use-cookie-filename

Conversation

@yyq1043-cloud

Copy link
Copy Markdown
Contributor

Summary

Implement previously dead TODO in submit.py β€” the cookie_filename parameter of check_features_manually was accepted but never used.

When --submit is invoked with --cookie-file <path>, site checks that go through check_features_manually (e.g. with urlProbe probes) now send the cookies from that file.

Changes

  • _parse_cookie_file(): new @staticmethod that calls import_aiohttp_cookies() (already in activation.py) to load a Netscape/Mozilla cookies.txt into an aiohttp CookieJar. Returns None if the file is missing.
  • _inject_cookie_headers(): new instance method that formats cookies applicable to a URL as a Cookie: name=value; name2=value2 header, merged with any caller-supplied headers.
  • check_features_manually(): when cookie_filename is set, create a temporary ClientSession with the loaded cookie jar (SSL-off configuration mirrors the default session). Apply _inject_cookie_headers() to both HTTP requests made by the function.

Testing

Verified by analysing the call path:

  1. run() β†’ interactive_site_check() β†’ check_features_manually(cookie_file=self.args.cookie_file, ...)
  2. check_features_manually() spans the existing-account check + non-existing-account check, both now include the cookie header when a file is provided.

- Add _parse_cookie_file() to load cookies from Netscape-format cookies.txt
- Add _inject_cookie_headers() to format cookies as the Cookie header
- When cookie_filename is provided, create a temporary ClientSession with
  the loaded cookie jar and inject cookies into request headers
- Fixes TODO: cookie_filename parameter is now actually used (250 lines)
@yyq1043-cloud

Copy link
Copy Markdown
Contributor Author

Hi! I'd like to request review for this PR: Implement cookie_filename support in check_features_manually
Requesting review from: @soxoj
Thank you! πŸ™‚

@soxoj

soxoj commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Thanks for picking this up! The idea β€” wiring the dead cookie_filename param β€” is right, but the implementation has several issues:

  1. Duplicate cookie injection. ClientSession(cookie_jar=...) already filters cookies by domain/path and attaches the Cookie: header automatically. The manual _inject_cookie_headers() will either duplicate or conflict with that. Pick one path β€” passing the jar to the session is the standard way.

  2. url parameter is ignored. _inject_cookie_headers(headers, jar, url) accepts url but iterates all cookies in the jar without filtering by domain/path. The docstring says "cookies applicable to url", but the code doesn't do that.

  3. Cookie header join is inconsistent.

    f"{existing}; {';'.join(cookie_strs)}"   # no space
    "; ".join(cookie_strs)                    # with space

    Two different separators in the same function.

  4. __import__("ssl") used twice instead of a normal import ssl at the top. Please clean up.

  5. Silent except Exception: return None in _parse_cookie_file. If the user passes --cookie-file /broken/path, they get no feedback. At least a logger.warning with the path and the exception.

  6. Raw cookie formatting. f"{key}={value}" doesn't escape β€” values containing ;, =, or whitespace will break the header. Consider http.cookies.SimpleCookie from stdlib.

  7. Implicit behavior change in session close. The previous code always did await session.close(), including self.session if the caller didn't pass one (likely a latent bug). The new code only closes _own_session. This is probably the correct behavior, but it's an unrelated fix bundled in β€” worth calling out explicitly, ideally in its own commit, with a test.

  8. No tests. This adds a non-trivial code path (cookie file β†’ session β†’ request). Please add at least one test covering _parse_cookie_file (missing file, bad file, valid file) and one verifying the cookie ends up on the request.

@soxoj soxoj marked this pull request as draft June 29, 2026 15:25
@soxoj soxoj changed the title Implement cookie_filename support in check_features_manually [WIP] Implement cookie_filename support in check_features_manually Jun 29, 2026
@yyq1043-cloud

Copy link
Copy Markdown
Contributor Author

Hi! I'd like to request review for this PR: [WIP] Implement cookie_filename support in check_features_manually
Requesting review from: @soxoj
Thank you! πŸ™‚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants