Fix KeyError on cookies with no expires_utc (e.g. Firefox session cookies) - #294
Merged
Merged
Conversation
Owner
|
Awesome, thank you! FF support is new, so thanks for taking a look and submitting a fix. |
Contributor
Author
|
Happy to help! Ran into it when parsing a FF profile from a Linux triage collection. Glad the fix was useful! |
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.
Problem
HindsightEncoder.default()crashes withKeyError: 'expires_utc'whenserializing a cookie item whose
expires_utcisNonewhich happensfor session cookies (no expiration set). This reliably reproduces when
processing a Firefox profile with any session cookies present.
Traceback:
Root cause
base_encoder()drops any field whose value isNone(analysis.py,line ~42). Firefox's session-cookie handling (browsers/firefox.py,
~line 387) sets
expires_utc=Nonewhen a cookie has no expiry. By thetime
default()reaches the cookie branch and doesitem['expires_utc'],that key has already been silently dropped, causing the KeyError.
I also found the identical pattern a few lines earlier
(
item['visit_duration'], line 75) same risk, not yet triggered butsame shape, so I fixed it defensively too.
Fix
Two one-line changes, using
.get()instead of direct indexing so amissing (dropped) key just evaluates the comparison as False instead of
raising:
Testing
Verified against a real Firefox profile with session cookies (previously
crashed 100% of the time on
-f jsonloutput) now completes cleanlyand writes all cookie records to the JSONL output, including the
session cookies that previously caused the crash.