fix(net): canonicalize the cookie domain on every jar key - #649
Open
aech wants to merge 1 commit into
Open
Conversation
Network.setCookies kept a leading dot and the letter case, the Set-Cookie path stripped the dot. The same cookie arriving over both entrances became two jar entries and both went out in one request. RFC 6265 4.1.2.3 ignores the dot and hosts are case-insensitive, so one helper now builds every key: the CDP import, the Set-Cookie and JS paths, both delete paths, and the MCP domain filter. domain_matches keeps its allocation-free comparison, it is on the per-request path and already ignores dot and case. load_from_file imports through set_cookies_from_cdp, so a persisted store with dotted domains is repaired on load. Fixes h4ckf0r0day#648.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #648
What changed
Network.setCookiesretained the leading dot and the letter case, while theSet-Cookiepath stripped the dot. The same cookie, when set via both entrances, resulted in two jar entries, and both were sent out in a single request. RFC 6265 4.1.2.3 ignores the dot, and hosts are case-insensitive. A helper therefore now constructs every jar key: the CDP import, theSet-Cookiepath, and the JS path, both deletion paths, and the MCP domain filter. This also eliminates three copies of a search across three different spellings, which existed solely to mask the inconsistency.domain_matchesretains its comparison without allocation. It sits on the per-request path and ignores dot and case anyway.load_from_fileimports viaset_cookies_from_cdp, so a persisted store with dotted domains is repaired on load.Validation
The stealth run is included because
wreq_client.rsuses the same cookie jar and callsset_cookie. It therefore exercises the changed path.Five tests in
cookies.rscover the defect. Each was tested against an intentionally broken version:entry.domaincausestest_cdp_stored_domain_field_carries_no_dotto failtest_cdp_zero_expiry_deletes_across_domain_spellingsandtest_delete_cookie_canonicalizes_its_lookupto failTwo failures also occur on
mainand are not caused by this change:observer-intersection(expected 'io:50', got ''). The remaining 32 pass.wreq_client::tests::stealth_client_decodes_gzip_response. Its fixture listens on 127.0.0.1, and the SSRF guard blocks this unlessOBSCURA_ALLOW_PRIVATE_NETWORKis set. If it is set,client::ssrf_tests::validate_url_blocks_unspecified_and_allows_publicfails instead, because it requires that0.0.0.0be rejected. Both were measured againstmainat 94374e6.Rendering
Not applicable.
Performance
get_cookie_headeranddomain_matches, the per-request path, remain unaffected. The change adds one allocation per cookie on import and one per delete call. Median latency of the obstacle course: 3034.2 ms with the change, 3030.5 ms onmain, inside the noise floor.Checklist