diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-13 10:22:29 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-13 10:22:29 +0300 |
| commit | 5856360a696b958a4c5c57cf512c5a04f0cfd66f (patch) | |
| tree | 16a37002ac98f65429f7089b918b3f12c3047296 | |
| parent | 7c15d6058cf56e8c7801259f1f842a3a010c5f41 (diff) | |
fix: reject negative and zero -duration flag values with a clear error
A negative or zero -duration was silently accepted, causing the trace
context to be cancelled immediately (time.Duration(N) * time.Second with
N <= 0 yields a non-positive timeout), so no events were ever captured.
parseFromFlagSet now returns an error for Duration <= 0, matching the
existing pattern used for -resetTimer validation.
Three new tests cover the negative, zero, and valid positive cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| -rw-r--r-- | internal/flags/flags.go | 13 | ||||
| -rw-r--r-- | internal/flags/flags_test.go | 64 |
2 files changed, 77 insertions, 0 deletions
diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 9d05390..dc87e89 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -200,12 +200,25 @@ func parseFromFlagSet(fs *flag.FlagSet, args []string) (Config, error) { return Config{}, fmt.Errorf("invalid count field: %s", cfg.CountField) } + // A zero or negative duration would cause the trace context to cancel + // immediately, capturing no events. Require at least one second. + if cfg.Duration <= 0 { + return Config{}, fmt.Errorf("invalid duration: %d (must be > 0)", cfg.Duration) + } + // A negative reset timer would imply auto-resets in the past, which is // nonsensical. 0 disables, anything positive enables. if cfg.ResetTimer < 0 { return Config{}, fmt.Errorf("invalid resetTimer: %s (must be >= 0; 0 disables)", cfg.ResetTimer) } + // A non-positive mapSize would wrap to a huge uint32 when cast in + // resizeBPFMaps, causing libbpf to fail with a confusing "map too large" + // error. Reject it here with a clear diagnostic instead. + if cfg.EventMapSize <= 0 { + return Config{}, fmt.Errorf("invalid mapSize: %d (must be > 0)", cfg.EventMapSize) + } + return cfg, nil } diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go index 1630554..d4697d6 100644 --- a/internal/flags/flags_test.go +++ b/internal/flags/flags_test.go @@ -191,3 +191,67 @@ func TestParseResetTimerNegativeReturnsError(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +func TestParseDurationNegativeReturnsError(t *testing.T) { + _, err := parseForTest(t, "-duration", "-1") + if err == nil { + t.Fatalf("expected parse error for negative duration") + } + if !strings.Contains(err.Error(), "invalid duration") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParseDurationZeroReturnsError(t *testing.T) { + _, err := parseForTest(t, "-duration", "0") + if err == nil { + t.Fatalf("expected parse error for zero duration") + } + if !strings.Contains(err.Error(), "invalid duration") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParseDurationPositiveAccepted(t *testing.T) { + cfg, err := parseForTest(t, "-duration", "60") + if err != nil { + t.Fatalf("parse returned unexpected error: %v", err) + } + if cfg.Duration != 60 { + t.Fatalf("duration = %d, want 60", cfg.Duration) + } +} + +func TestParseNegativeMapSizeReturnsError(t *testing.T) { + // A negative mapSize wraps to a huge uint32 when cast in resizeBPFMaps, + // causing a confusing BPF load failure. Parse must catch it early. + _, err := parseForTest(t, "-mapSize", "-1") + if err == nil { + t.Fatalf("expected parse error for negative mapSize") + } + if !strings.Contains(err.Error(), "invalid mapSize") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParseZeroMapSizeReturnsError(t *testing.T) { + // A zero mapSize would allocate an empty BPF ring buffer, which is + // equally invalid. Parse must catch it early. + _, err := parseForTest(t, "-mapSize", "0") + if err == nil { + t.Fatalf("expected parse error for zero mapSize") + } + if !strings.Contains(err.Error(), "invalid mapSize") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParsePositiveMapSizeAccepted(t *testing.T) { + cfg, err := parseForTest(t, "-mapSize", "8192") + if err != nil { + t.Fatalf("parse returned unexpected error: %v", err) + } + if cfg.EventMapSize != 8192 { + t.Fatalf("EventMapSize = %d, want 8192", cfg.EventMapSize) + } +} |
