Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions internal/submit/summary.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -210,8 +210,28 @@ type SummaryParser struct {
// genuinely 0 (early failure case) — Bugbot caught this on
// PR #10 round 2.
sawAnyField bool

// droppingLine is set when buf outgrew parserLineMax before a
// '\n' arrived: the partial (newline-less) line was discarded
// (see Feed). It stays set until the terminating '\n' is seen,
// so the oversized line's tail is dropped too rather than
// parsed as a spurious fresh line.
droppingLine bool
}

// parserLineMax bounds the partial (newline-less) content the
// parser buffers in buf. It mirrors displayLineMax (watch.go)
// exactly — same package, so we reference it directly and the two
// paths can't drift. Rationale: the display side drains a tqdm
// '\r'-redraw burst that outgrows displayLineMax rather than
// failing, but those drained bytes still flow through the
// TeeReader into Feed. Without a matching bound here, a
// pathological ingestor emitting many MB of '\r' redraws with no
// '\n' for the life of a (up to 1h) run would grow buf without
// limit. A real banner line is tens of bytes; newline-less content
// past this ceiling cannot be one, so Feed drops it.
const parserLineMax = displayLineMax

// NewSummaryParser returns an initialized parser. Caller's
// goroutine owns it for the duration of the watch loop.
func NewSummaryParser() *SummaryParser {
Expand All@@ -229,10 +249,27 @@ func (p *SummaryParser) Feed(b []byte) {
for {
idx := bytes.IndexByte(p.buf.Bytes(), '\n')
if idx < 0 {
// No complete line yet — wait for more input.
// No complete line yet. If the partial (newline-less)
// content has outgrown parserLineMax it cannot be a
// banner line — drop it and enter drop-until-newline
// mode so the oversized line's tail is discarded too,
// not mistaken for a fresh line. Bounds buf at
// parserLineMax + one Feed chunk regardless of how long
// the ingestor withholds a '\n'.
if p.buf.Len() > parserLineMax {
p.buf.Reset()
p.droppingLine = true
}
return
}
line := p.buf.Next(idx + 1) // consume up to and including '\n'
if p.droppingLine {
// This '\n' terminates an oversized line whose head we
// already dropped; discard the buffered tail and resume
// normal parsing on the lines that follow.
p.droppingLine = false
continue
}
p.feedLine(string(bytes.TrimRight(line, "\n")))
}
}
Expand All@@ -242,7 +279,18 @@ func (p *SummaryParser) Feed(b []byte) {
// terminated without a final '\n' (rare but possible if the
// container's stdout was killed mid-write).
func (p *SummaryParser) FlushLine() {
if p.buf.Len() > 0 && !p.finalized {
if p.finalized {
return
}
if p.droppingLine {
// EOF landed mid-drop: buf holds the tail of an oversized
// line whose head was already discarded. Drop the tail too
// rather than parse it as a line.
p.buf.Reset()
p.droppingLine = false
return
}
if p.buf.Len() > 0 {
p.feedLine(p.buf.String())
p.buf.Reset()
}
Expand Down
45 changes: 45 additions & 0 deletions internal/submit/summary_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -174,6 +174,51 @@ func TestSummaryParser_PostBannerLogsIgnored(t *testing.T) {
}
}

// TestSummaryParser_BufferBoundedOnNewlinelessFlood pins finding D3
// (deferred from the v0.8.0 review): a pathological ingestor can emit
// many MB of tqdm '\r'-redraws with no '\n' for the life of a run. The
// display path drains those past displayLineMax, but the drained bytes
// still flow through the TeeReader into Feed — so without a matching
// bound the parser's buf would grow unbounded. Assert buf stays within
// parserLineMax under the flood, and that a real banner arriving after
// the flood terminates (a '\n' finally lands) still parses.
func TestSummaryParser_BufferBoundedOnNewlinelessFlood(t *testing.T) {
p := NewSummaryParser()

// tqdm-style redraw: '\r' + progress text, never a '\n'. Feed well
// past parserLineMax in bounded chunks so we also exercise the
// across-Feed-calls accumulation, not just one giant Write.
chunk := []byte("\r" + strings.Repeat("#", 512*1024-1)) // 512 KiB, no '\n'
for total := 0; total <= parserLineMax*2; total += len(chunk) {
p.Feed(chunk)
if p.buf.Len() > parserLineMax {
t.Fatalf("buf grew to %d bytes after a newline-less flood, exceeds parserLineMax=%d",
p.buf.Len(), parserLineMax)
}
}

// The flood is one newline-less line; none of it should have been
// mistaken for a banner.
if got := p.Result(); got != nil {
t.Fatalf("newline-less flood produced a non-nil Summary: %+v", got)
}

// The pathological line finally terminates and a real banner
// follows. The parser must recover: drop the oversized line's tail,
// then parse the banner that comes after.
p.Feed([]byte("\n"))
p.Feed([]byte(realIngestorBanner))

got := p.Result()
if got == nil {
t.Fatal("banner after a newline-less flood did not parse; Result is nil")
}
if got.TotalRecords != 1234 || got.FailedRecords != 30 {
t.Errorf("post-flood banner parsed wrong: TotalRecords=%d FailedRecords=%d, want 1234/30",
got.TotalRecords, got.FailedRecords)
}
}

// TestStripANSI: the parser strips ANSI SGR codes from each line
// before matching. Validate the regex handles common shapes.
func TestStripANSI(t *testing.T) {
Expand Down
Loading