Skip to content

iter_jsonl_record_lines materializes the whole upload with str.split("\n") before the GOGGLES_MAX_DUMP_RECORDS guard, so a many-record upload OOM-amplifies (~30x) ahead of the record cap #293

Description

@erskingardner

Summary

iter_jsonl_record_lines is typed -> Iterator[str] and documented as a streaming record splitter, but it splits with raw_text.split("\n"), which eagerly builds the entire list of line strings in memory before yielding the first record. All of parse_jsonl's protective guards — the per-line byte cap, and crucially the record-count cap GOGGLES_MAX_DUMP_RECORDS (50,000) — run inside the per-record loop, i.e. only after the full split has already materialized. A byte-bounded upload made of many tiny records therefore blows up peak heap far past what the record cap is meant to bound.

Code

forensics/ingest.py:136-146:

def iter_jsonl_record_lines(raw_text: str) -> Iterator[str]:
    """Yield JSONL records split only on the LF record delimiter.
    ...
    """
    for raw_line in raw_text.split("\n"):   # eager: builds the whole list first
        if raw_line.endswith("\r"):
            raw_line = raw_line[:-1]
        yield raw_line

forensics/ingest.py:407-428 — the guards that cannot fire until after the split:

def parse_jsonl(raw_text: str) -> list[ParsedLine]:
    parsed_lines = []
    for line_number, raw_line in enumerate(iter_jsonl_record_lines(raw_text), start=1):
        ...
        if len(parsed_lines) >= settings.GOGGLES_MAX_DUMP_RECORDS:
            raise AuditLogComplexityError(...)   # only stops the loop, not the split

Concrete failure scenario

An authenticated upload-token holder POSTs a dump at the GOGGLES_MAX_DUMP_BYTES (50 MiB) ceiling consisting of ~25M one-character records ("1\n1\n1\n…"). raw_text.split("\n") allocates ~25M distinct 1-char str objects (~49–50 bytes each in CPython) plus the list's ~25M-pointer array — on the order of 1.3–1.5 GB of peak heap from a ~50 MB input (~30x amplification)before the 50,000-record cap gets a chance to reject anything. The record cap does stop parsing at 50k, but the memory blow-up already happened in the splitter. On the compose deployment (no per-container memory limits — see #182) this OOM-kills the ingest worker and fails the upload; a small number of concurrent such uploads can exhaust host memory.

Why not a duplicate

No open issue covers iter_jsonl_record_lines's eager split("\n") or the GOGGLES_MAX_DUMP_RECORDS guard being bypassable by pre-split materialization.

Suggested fix

Iterate lazily so the per-line byte cap and record-count cap can short-circuit before the whole corpus is materialized — e.g. a manual str.find("\n") scan yielding one slice at a time, or wrapping in io.StringIO and iterating. (Note str.splitlines() is both eager and wrong here, since it also breaks on U+2028/U+2029 — the reason the current code avoids it.)

Severity: MEDIUM — token-holder-triggerable memory-amplification DoS that partially defeats the record-count guard.

Metadata

Metadata

Assignees

No one assigned

    Labels

    MEDIUMSeverity: important bug or performance issue with bounded impactperformancesecurity

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions