Skip to content

parser: return error instead of panic when TZ prefix has no fields#574

Open
nghiack7 wants to merge 2 commits into
robfig:masterfrom
nghiack7:fix/tz-prefix-missing-fields-panic
Open

parser: return error instead of panic when TZ prefix has no fields#574
nghiack7 wants to merge 2 commits into
robfig:masterfrom
nghiack7:fix/tz-prefix-missing-fields-panic

Conversation

@nghiack7
Copy link
Copy Markdown

@nghiack7 nghiack7 commented May 9, 2026

Problem

Fixes #554

ParseStandard("TZ=0") (or any TZ=/CRON_TZ= prefix without trailing schedule fields) panics with:

panic: runtime error: slice bounds out of range [:-1]

at parser.go because strings.Index(spec, " ") returns -1 when there is no space, and the slice spec[eq+1 : -1] is invalid.

In production systems where cron expressions come from user input, this panic crashes the entire process.

Fix

One-line guard: if strings.Index(spec, " ") returns -1, return a descriptive error before touching any slice.

i := strings.Index(spec, " ")
if i < 0 {
    return nil, fmt.Errorf("timezone prefix missing schedule fields: %v", spec)
}

Tests

Added TestParseTZMissingFields in parser_test.go covering:

  • "TZ=0" (invalid zone, no fields)
  • "TZ=UTC" (valid zone, no fields)
  • "CRON_TZ=America/New_York" (valid zone, no fields)

Nghĩa Nguyễn Ngọc added 2 commits May 9, 2026 13:47
A step value larger than the field's own range (e.g. */90 in minutes
where max=59) would silently accept the expression and schedule the job
only at the range minimum, hiding a likely user error.

Add a validation check after the step is parsed:

  if step > r.max-r.min {
      return 0, fmt.Errorf("step (%d) above maximum (%d): ...", ...)
  }

This mirrors the existing checks for start/end bounds and returns a
clear error rather than silently mis-scheduling.

Add two test cases to TestStandardSpecSchedule exercising the new
validation for the minute and hour fields.

Fixes robfig#543
When spec is 'TZ=<zone>' with no trailing schedule fields, Index(' ')
returns -1 and the slice expression spec[eq+1 : -1] panics.

Add an explicit check: if there is no space separator after the timezone
prefix, return a descriptive error immediately.

Fixes robfig#554
@nghiack7
Copy link
Copy Markdown
Author

Friendly ping — this PR is ready for review. CI is clean ✅. Happy to make any changes requested.

Sign up for free to 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.

cron.ParseStandard("TZ=0") causes panic

1 participant