Skip to content

fix(summary): bound SummaryParser.buf on newline-less log floods (D3) - #227

Merged
saadqbal merged 1 commit into
developfrom
fix/summary-parser-buf-cap
Jul 10, 2026
Merged

fix(summary): bound SummaryParser.buf on newline-less log floods (D3)#227
saadqbal merged 1 commit into
developfrom
fix/summary-parser-buf-cap

Conversation

@saadqbal

@saadqbalsaadqbal commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Closes#226. Deferred finding D3 from the v0.8.0 review (#220). Low severity.

The bug

SummaryParser.Feed accumulates bytes into p.buf until it sees a \n. The display path caps an oversized tqdm line via displayLineMax (16 MB) in streamDisplayAndParse and drains the rest — but the drained bytes still flow through the TeeReader into Feed. So a pathological ingestor emitting many MB of \r-redraws with no \n for the life of a (up to 1h JobWatchTimeout) run grows p.buf without the display-side cap ever applying to the parser.

The fix

Bound buf at parserLineMax, defined as = displayLineMax — same submit package, referenced directly so the two paths can't drift. When the partial newline-less line passes the ceiling, drop it and enter a drop-until-newline state (droppingLine) so the oversized line's tail is discarded too rather than parsed as a spurious fresh line. FlushLine honors the same state at EOF.

A real banner line is tens of bytes, so newline-less content past 16 MB can never be one — dropping is safe, and the parser still recovers to parse the closing banner once a \n finally lands. buf's persistent size is now bounded by parserLineMax (peak is parserLineMax + one Feed chunk, transient).

Test

TestSummaryParser_BufferBoundedOnNewlinelessFlood feeds a >2×parserLineMax newline-less flood in bounded chunks, asserts buf never exceeds parserLineMax, then feeds a terminating \n + a real banner and asserts it still parses.

go test ./internal/submit/, go vet, gofmt all clean.

🤖 Generated with Claude Code


Note

Low Risk
Defensive bounds on log parsing only; legitimate banners are tiny and recovery after a terminating newline is covered by tests.

Overview
Caps SummaryParser memory when ingest logs never emit a newline — e.g. long tqdm \r redraws that the display path already drains but still pass through the TeeReader into Feed.

parserLineMax is set to displayLineMax (16 MB) so parser and display limits stay aligned. Once a partial line exceeds that, Feed resets the buffer and sets droppingLine until the next \n, so the rest of the junk line is skipped instead of being parsed as a new line. FlushLine applies the same rule at EOF so a truncated tail is not fed to feedLine.

Adds TestSummaryParser_BufferBoundedOnNewlinelessFlood: multi-chunk newline-less flood keeps bufparserLineMax, then \n + a real banner still parses correctly.

Reviewed by Cursor Bugbot for commit 2c92d92. Bugbot is set up for automated code reviews on this repo. Configure here.

…226)
SummaryParser.Feed accumulated bytes into p.buf until a '\n'. The
display path caps an oversized tqdm line via displayLineMax (16 MB) in
streamDisplayAndParse and drains the rest, but the drained bytes still
flow through the TeeReader into Feed — so an ingestor emitting many MB
of '\r'-redraws with no '\n' for the life of a (up to 1h) run grew
p.buf without the display-side cap ever applying to the parser.
Bound buf at parserLineMax (= displayLineMax; same package, referenced
directly so the two paths can't drift). When the partial newline-less
line passes the ceiling, drop it and enter drop-until-newline mode so
the oversized line's tail is discarded too rather than parsed as a
spurious fresh line; FlushLine honors the same state at EOF. A real
banner line is tens of bytes, so newline-less content past 16 MB can
never be one — dropping is safe and the parser still recovers to parse
the closing banner once a '\n' finally lands.
Adds a white-box test feeding a >2x-parserLineMax newline-less flood,
asserting buf stays bounded and a real banner after the flood parses.
Deferred finding D3 from the v0.8.0 review (#220). Low severity.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal
saadqbal merged commit e4d5ce5 into developJul 10, 2026
20 checks passed
@saadqbal
saadqbal deleted the fix/summary-parser-buf-cap branch July 10, 2026 16:34
Sign up for freeto 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

@saadqbal@LukasWodka