diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..c507d449 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,10 @@ +version: 2 +updates: + - package-ecosystem: "gomod" + directory: "/" + schedule: + interval: "weekly" + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..252e41f9 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,213 @@ +name: CI + +on: + push: + branches: [main] + tags: ['**'] + pull_request: + branches: [main] + workflow_dispatch: + schedule: + - cron: '0 0 * * 0' + +permissions: + contents: read + +jobs: + unit: + name: unit tests + runs-on: ${{ matrix.platform }} + strategy: + fail-fast: false + matrix: + go-version: [1.25.x] + platform: [ubuntu-latest] + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go-version }} + + - name: Verify go.mod is tidy + run: | + go mod tidy + git diff --exit-code -- go.mod go.sum + + - name: Unit tests + run: go test -race -covermode=atomic -coverprofile=coverage.out ./... + + - name: Coverage summary + run: go tool cover -func=coverage.out | tail -n 1 || true + + - name: Enforce coverage threshold + run: | + THRESHOLD=70.0 + TOTAL=$(go tool cover -func=coverage.out | awk '/^total:/ {gsub("%","",$3); print $3}') + echo "Total coverage: ${TOTAL}% (threshold ${THRESHOLD}%)" + awk -v t="$THRESHOLD" -v a="$TOTAL" 'BEGIN { if (a+0 < t+0) { exit 1 } }' + + - name: Upload coverage + uses: actions/upload-artifact@v4 + with: + name: coverage + path: coverage.out + + lint: + name: golangci-lint + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Download modules + run: go mod download + + - name: Warm build cache + run: go build ./... + + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: latest + install-mode: goinstall + args: --timeout=5m --out-format=github-actions --allow-parallel-runners + only-new-issues: false + + vulncheck: + name: govulncheck + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Run govulncheck + run: | + set +e + TMP_OUT=$(mktemp) + go run golang.org/x/vuln/cmd/govulncheck@latest ./... | tee "$TMP_OUT" + status=$? + set -e + if grep -E "^\s*Fixed in:\s+" "$TMP_OUT" | grep -v "Fixed in: N/A" >/dev/null; then + echo "govulncheck: vulnerabilities with available fixes detected" + exit 1 + fi + echo "govulncheck: no vulnerabilities with available fixes" + exit 0 + + codeql: + if: (github.event_name != 'push') || (!startsWith(github.ref, 'refs/heads/gh-readonly-queue/')) + name: CodeQL Analysis + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: go + + - name: Download dependencies + run: go mod download + + - name: Build + uses: github/codeql-action/autobuild@v3 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + + gosec: + name: gosec + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Install gosec + run: go install github.com/securego/gosec/v2/cmd/gosec@latest + + - name: Run gosec + run: | + $(go env GOPATH)/bin/gosec ./... + + gitleaks: + name: gitleaks + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Download gitleaks + run: | + set -e + VERSION="8.18.4" + URL="https://github.com/gitleaks/gitleaks/releases/download/v${VERSION}/gitleaks_${VERSION}_linux_x64.tar.gz" + curl -sSL "$URL" | tar -xz gitleaks + chmod +x gitleaks + + - name: Run gitleaks + run: ./gitleaks detect --source . --no-banner --redact + + trivy: + name: trivy scan (fs) + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@v4 + - name: Run Trivy filesystem scan + uses: aquasecurity/trivy-action@0.24.0 + with: + scan-type: fs + ignore-unfixed: true + format: sarif + output: trivy-results.sarif + vuln-type: 'os,library' + severity: CRITICAL,HIGH + exit-code: 1 + - name: Upload Trivy SARIF + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: trivy-results.sarif + + workflow-lint: + name: actionlint + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + steps: + - uses: actions/checkout@v4 + - uses: reviewdog/action-actionlint@v1 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..92e53e1c --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,55 @@ +version: "2" +run: + tests: true +linters: + default: none + enable: + - gocyclo + - govet + - ineffassign + - misspell + - staticcheck + - unused + settings: + gocyclo: + min-complexity: 25 + misspell: + locale: US + staticcheck: + checks: + - all + - -S1000 # Allow single-case select for readability + - -S1037 # Allow select with timeout channel for test patterns + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - gocyclo + path: _test\.go + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gci + - gofmt + - gofumpt + - goimports + settings: + gci: + sections: + - standard + - default + - prefix(github.com/netresearch/go-cron) + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/README.md b/README.md index 38c4d8a0..bcd9c917 100644 --- a/README.md +++ b/README.md @@ -1,46 +1,54 @@ -[![GoDoc](http://godoc.org/github.com/robfig/cron?status.png)](http://godoc.org/github.com/robfig/cron) -[![Build Status](https://travis-ci.org/robfig/cron.svg?branch=master)](https://travis-ci.org/robfig/cron) +[![Go Reference](https://pkg.go.dev/badge/github.com/netresearch/go-cron.svg)](https://pkg.go.dev/github.com/netresearch/go-cron) -# cron +# go-cron -Cron V3 has been released! +A maintained fork of [robfig/cron](https://github.com/robfig/cron) with bug fixes and improvements. + +## Installation -To download the specific tagged release, run: ```bash -go get github.com/robfig/cron/v3@v3.0.0 +go get github.com/netresearch/go-cron ``` + Import it in your program as: ```go -import "github.com/robfig/cron/v3" +import "github.com/netresearch/go-cron" ``` -It requires Go 1.11 or later due to usage of Go Modules. -Refer to the documentation here: -http://godoc.org/github.com/robfig/cron +Requires Go 1.25 or later. -The rest of this document describes the the advances in v3 and a list of -breaking changes for users that wish to upgrade from an earlier version. +## Why This Fork? -## Upgrading to v3 (June 2019) +The original robfig/cron has been unmaintained since 2020, with 50+ open PRs and several +critical panic bugs. This fork addresses: -cron v3 is a major upgrade to the library that addresses all outstanding bugs, -feature requests, and rough edges. It is based on a merge of master which -contains various fixes to issues found over the years and the v2 branch which -contains some backwards-incompatible features like the ability to remove cron -jobs. In addition, v3 adds support for Go Modules, cleans up rough edges like -the timezone support, and fixes a number of bugs. +- **Panic fixes**: Fixed TZ= parsing panics (issues #554, #555) +- **Chain behavior**: Added `Entry.Run()` method to properly invoke chain decorators (issue #551) +- **DST handling**: Jobs scheduled during DST "spring forward" now run immediately (ISC cron behavior, PR #541) +- **Go 1.25**: Updated to latest Go version with modern toolchain -New features: +## Migrating from robfig/cron -- Support for Go modules. Callers must now import this library as - `github.com/robfig/cron/v3`, instead of `gopkg.in/...` +Simply update your import path: +```go +// Before +import "github.com/robfig/cron/v3" + +// After +import cron "github.com/netresearch/go-cron" +``` -- Fixed bugs: - - 0f01e6b parser: fix combining of Dow and Dom (#70) - - dbf3220 adjust times when rolling the clock forward to handle non-existent midnight (#157) - - eeecf15 spec_test.go: ensure an error is returned on 0 increment (#144) - - 70971dc cron.Entries(): update request for snapshot to include a reply channel (#97) - - 1cba5e6 cron: fix: removing a job causes the next scheduled job to run too late (#206) +The API is fully compatible with robfig/cron v3. + +## Documentation + +Refer to the [package documentation](https://pkg.go.dev/github.com/netresearch/go-cron). + +## Features (inherited from robfig/cron v3) + +This fork maintains full compatibility with robfig/cron v3 while adding fixes. + +Original v3 features: - Standard cron spec parsing by default (first field is "minute"), with an easy way to opt into the seconds field (quartz-compatible). Although, note that the @@ -57,14 +65,8 @@ New features: - Log each job's invocations - Notification when jobs are completed -It is backwards incompatible with both v1 and v2. These updates are required: - -- The v1 branch accepted an optional seconds field at the beginning of the cron - spec. This is non-standard and has led to a lot of confusion. The new default - parser conforms to the standard as described by [the Cron wikipedia page]. +## Usage Examples - UPDATING: To retain the old behavior, construct your Cron with a custom - parser: ```go // Seconds field, required cron.New(cron.WithSeconds()) @@ -73,40 +75,26 @@ cron.New(cron.WithSeconds()) cron.New(cron.WithParser(cron.NewParser( cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor, ))) -``` -- The Cron type now accepts functional options on construction rather than the - previous ad-hoc behavior modification mechanisms (setting a field, calling a setter). - - UPDATING: Code that sets Cron.ErrorLogger or calls Cron.SetLocation must be - updated to provide those values on construction. - -- CRON_TZ is now the recommended way to specify the timezone of a single - schedule, which is sanctioned by the specification. The legacy "TZ=" prefix - will continue to be supported since it is unambiguous and easy to do so. - - UPDATING: No update is required. -- By default, cron will no longer recover panics in jobs that it runs. - Recovering can be surprising (see issue #192) and seems to be at odds with - typical behavior of libraries. Relatedly, the `cron.WithPanicLogger` option - has been removed to accommodate the more general JobWrapper type. - - UPDATING: To opt into panic recovery and configure the panic logger: -```go +// With panic recovery cron.New(cron.WithChain( - cron.Recover(logger), // or use cron.DefaultLogger + cron.Recover(logger), )) -``` -- In adding support for https://github.com/go-logr/logr, `cron.WithVerboseLogger` was - removed, since it is duplicative with the leveled logging. - UPDATING: Callers should use `WithLogger` and specify a logger that does not - discard `Info` logs. For convenience, one is provided that wraps `*log.Logger`: -```go +// With verbose logging cron.New( cron.WithLogger(cron.VerbosePrintfLogger(logger))) ``` +## Timezone Support + +CRON_TZ is the recommended way to specify the timezone of a single schedule: +``` +CRON_TZ=America/New_York 0 0 * * * +``` + +The legacy "TZ=" prefix is also supported. + ### Background - Cron spec format There are two cron spec formats in common usage: diff --git a/chain.go b/chain.go index 9c087b7b..209b82a6 100644 --- a/chain.go +++ b/chain.go @@ -24,9 +24,12 @@ func NewChain(c ...JobWrapper) Chain { // Then decorates the given job with all JobWrappers in the chain. // // This: -// NewChain(m1, m2, m3).Then(job) +// +// NewChain(m1, m2, m3).Then(job) +// // is equivalent to: -// m1(m2(m3(job))) +// +// m1(m2(m3(job))) func (c Chain) Then(j Job) Job { for i := range c.wrappers { j = c.wrappers[len(c.wrappers)-i-1](j) @@ -77,7 +80,7 @@ func DelayIfStillRunning(logger Logger) JobWrapper { // still running. It logs skips to the given logger at Info level. func SkipIfStillRunning(logger Logger) JobWrapper { return func(j Job) Job { - var ch = make(chan struct{}, 1) + ch := make(chan struct{}, 1) ch <- struct{}{} return FuncJob(func() { select { diff --git a/chain_test.go b/chain_test.go index ec910975..4b4f970c 100644 --- a/chain_test.go +++ b/chain_test.go @@ -1,7 +1,7 @@ package cron import ( - "io/ioutil" + "io" "log" "reflect" "sync" @@ -57,13 +57,13 @@ func TestChainRecover(t *testing.T) { }) t.Run("Recovering JobWrapper recovers", func(t *testing.T) { - NewChain(Recover(PrintfLogger(log.New(ioutil.Discard, "", 0)))). + NewChain(Recover(PrintfLogger(log.New(io.Discard, "", 0)))). Then(panickingJob). Run() }) t.Run("composed with the *IfStillRunning wrappers", func(t *testing.T) { - NewChain(Recover(PrintfLogger(log.New(ioutil.Discard, "", 0)))). + NewChain(Recover(PrintfLogger(log.New(io.Discard, "", 0)))). Then(panickingJob). Run() }) @@ -99,7 +99,6 @@ func (j *countJob) Done() int { } func TestChainDelayIfStillRunning(t *testing.T) { - t.Run("runs immediately", func(t *testing.T) { var j countJob wrappedJob := NewChain(DelayIfStillRunning(DiscardLogger)).Then(&j) @@ -149,11 +148,9 @@ func TestChainDelayIfStillRunning(t *testing.T) { t.Error("expected both jobs done, got", started, done) } }) - } func TestChainSkipIfStillRunning(t *testing.T) { - t.Run("runs immediately", func(t *testing.T) { var j countJob wrappedJob := NewChain(SkipIfStillRunning(DiscardLogger)).Then(&j) @@ -238,5 +235,4 @@ func TestChainSkipIfStillRunning(t *testing.T) { t.Error("expected both jobs executed once, got", done1, "and", done2) } }) - } diff --git a/cron.go b/cron.go index c7e91766..18b252e4 100644 --- a/cron.go +++ b/cron.go @@ -74,6 +74,17 @@ type Entry struct { // Valid returns true if this is not the zero entry. func (e Entry) Valid() bool { return e.ID != 0 } +// Run executes the entry's job through the configured chain wrappers. +// This ensures that chain decorators like SkipIfStillRunning, DelayIfStillRunning, +// and Recover are properly applied. Use this method instead of Entry.Job.Run() +// when you need chain behavior to be respected. +// Fix for issue #551: Provides a proper way to run jobs with chain decorators. +func (e Entry) Run() { + if e.WrappedJob != nil { + e.WrappedJob.Run() + } +} + // byTime is a wrapper for sorting the entry array by time // (with zero time at the end). type byTime []*Entry @@ -97,17 +108,17 @@ func (s byTime) Less(i, j int) bool { // // Available Settings // -// Time Zone -// Description: The time zone in which schedules are interpreted -// Default: time.Local +// Time Zone +// Description: The time zone in which schedules are interpreted +// Default: time.Local // -// Parser -// Description: Parser converts cron spec strings into cron.Schedules. -// Default: Accepts this spec: https://en.wikipedia.org/wiki/Cron +// Parser +// Description: Parser converts cron spec strings into cron.Schedules. +// Default: Accepts this spec: https://en.wikipedia.org/wiki/Cron // -// Chain -// Description: Wrap submitted jobs to customize behavior. -// Default: A chain that recovers panics and logs them to stderr. +// Chain +// Description: Wrap submitted jobs to customize behavior. +// Default: A chain that recovers panics and logs them to stderr. // // See "cron.With*" to modify the default behavior. func New(opts ...Option) *Cron { @@ -337,7 +348,7 @@ func (c *Cron) Stop() context.Context { // entrySnapshot returns a copy of the current cron entry list. func (c *Cron) entrySnapshot() []Entry { - var entries = make([]Entry, len(c.entries)) + entries := make([]Entry, len(c.entries)) for i, e := range c.entries { entries[i] = *e } diff --git a/cron_test.go b/cron_test.go index 36f06bf7..32cf6dd0 100644 --- a/cron_test.go +++ b/cron_test.go @@ -388,7 +388,7 @@ func TestBlockingRun(t *testing.T) { cron := newWithSeconds() cron.AddFunc("* * * * * ?", func() { wg.Done() }) - var unblockChan = make(chan struct{}) + unblockChan := make(chan struct{}) go func() { cron.Run() @@ -407,7 +407,7 @@ func TestBlockingRun(t *testing.T) { // Test that double-running is a no-op func TestStartNoop(t *testing.T) { - var tickChan = make(chan struct{}, 2) + tickChan := make(chan struct{}, 2) cron := newWithSeconds() cron.AddFunc("* * * * * ?", func() { @@ -667,7 +667,6 @@ func TestStopAndWait(t *testing.T) { case <-time.After(time.Millisecond): t.Error("context not done even when cron Stop is completed") } - }) } @@ -700,3 +699,83 @@ func stop(cron *Cron) chan bool { func newWithSeconds() *Cron { return New(WithParser(secondParser), WithChain()) } + +// TestEntryRunWithChain tests fix for issue #551 +// Entry.Run() should execute through the chain wrappers +func TestEntryRunWithChain(t *testing.T) { + var callCount int64 + var mu sync.Mutex + + // Create cron with SkipIfStillRunning to test chain behavior + cron := New( + WithParser(secondParser), + WithChain(SkipIfStillRunning(DiscardLogger)), + ) + + // Add a job that blocks and counts calls + _, err := cron.AddFunc("* * * * * *", func() { + mu.Lock() + atomic.AddInt64(&callCount, 1) + mu.Unlock() + time.Sleep(100 * time.Millisecond) + }) + if err != nil { + t.Fatal(err) + } + + entries := cron.Entries() + if len(entries) != 1 { + t.Fatalf("expected 1 entry, got %d", len(entries)) + } + + entry := entries[0] + + // Test 1: Entry.Run() uses WrappedJob (respects chain) + // Start the job via Entry.Run() + go entry.Run() + time.Sleep(10 * time.Millisecond) + + // Try to run again while first is still running + // With SkipIfStillRunning, this should be skipped + entry.Run() + + // Wait for first job to complete + time.Sleep(150 * time.Millisecond) + + count := atomic.LoadInt64(&callCount) + if count != 1 { + t.Errorf("Entry.Run() with SkipIfStillRunning: expected 1 call, got %d (chain not respected)", count) + } + + // Test 2: Entry.Job.Run() bypasses chain (documenting existing behavior) + atomic.StoreInt64(&callCount, 0) + + // Start job via Entry.Job.Run() (bypasses chain) + go entry.Job.Run() + time.Sleep(10 * time.Millisecond) + + // Run again - this should NOT be skipped because chain is bypassed + entry.Job.Run() + + // Wait for both to complete + time.Sleep(150 * time.Millisecond) + + count = atomic.LoadInt64(&callCount) + if count != 2 { + t.Errorf("Entry.Job.Run() (bypass chain): expected 2 calls, got %d", count) + } +} + +// TestEntryRunNilWrappedJob tests Entry.Run() with nil WrappedJob +func TestEntryRunNilWrappedJob(t *testing.T) { + entry := Entry{} + + // Should not panic when WrappedJob is nil + defer func() { + if r := recover(); r != nil { + t.Errorf("Entry.Run() panicked with nil WrappedJob: %v", r) + } + }() + + entry.Run() +} diff --git a/doc.go b/doc.go index fa5d08b4..b9d00f52 100644 --- a/doc.go +++ b/doc.go @@ -1,7 +1,7 @@ /* Package cron implements a cron spec parser and job runner. -Installation +# Installation To download the specific tagged release, run: @@ -13,7 +13,7 @@ Import it in your program as: It requires Go 1.11 or later due to usage of Go Modules. -Usage +# Usage Callers may register Funcs to be invoked on a given schedule. Cron will run them in their own goroutines. @@ -36,7 +36,7 @@ them in their own goroutines. .. c.Stop() // Stop the scheduler (does not stop any jobs already running). -CRON Expression Format +# CRON Expression Format A cron expression represents a set of times, using 5 space-separated fields. @@ -54,7 +54,7 @@ Month and Day-of-week field values are case insensitive. "SUN", "Sun", and The specific interpretation of the format is based on the Cron Wikipedia page: https://en.wikipedia.org/wiki/Cron -Alternative Formats +# Alternative Formats Alternative Cron expression formats support other fields like seconds. You can implement that by creating a custom Parser as follows. @@ -73,7 +73,7 @@ parser you saw earlier, except that its seconds field is REQUIRED: That emulates Quartz, the most popular alternative Cron schedule format: http://www.quartz-scheduler.org/documentation/quartz-2.x/tutorials/crontrigger.html -Special Characters +# Special Characters Asterisk ( * ) @@ -105,7 +105,7 @@ Question mark ( ? ) Question mark may be used instead of '*' for leaving either day-of-month or day-of-week blank. -Predefined schedules +# Predefined schedules You may use one of several pre-defined schedules in place of a cron expression. @@ -117,12 +117,12 @@ You may use one of several pre-defined schedules in place of a cron expression. @daily (or @midnight) | Run once a day, midnight | 0 0 * * * @hourly | Run once an hour, beginning of hour | 0 * * * * -Intervals +# Intervals You may also schedule a job to execute at fixed intervals, starting at the time it's added or cron is run. This is supported by formatting the cron spec like this: - @every + @every where "duration" is a string accepted by time.ParseDuration (http://golang.org/pkg/time/#ParseDuration). @@ -134,13 +134,13 @@ Note: The interval does not take the job runtime into account. For example, if a job takes 3 minutes to run, and it is scheduled to run every 5 minutes, it will have only 2 minutes of idle time between each run. -Time zones +# Time zones By default, all interpretation and scheduling is done in the machine's local time zone (time.Local). You can specify a different time zone on construction: - cron.New( - cron.WithLocation(time.UTC)) + cron.New( + cron.WithLocation(time.UTC)) Individual cron schedules may also override the time zone they are to be interpreted in by providing an additional space-separated field at the beginning @@ -169,7 +169,7 @@ The prefix "TZ=(TIME ZONE)" is also supported for legacy compatibility. Be aware that jobs scheduled during daylight-savings leap-ahead transitions will not be run! -Job Wrappers +# Job Wrappers A Cron runner may be configured with a chain of job wrappers to add cross-cutting functionality to all submitted jobs. For example, they may be used @@ -192,7 +192,7 @@ Install wrappers for individual jobs by explicitly wrapping them: cron.SkipIfStillRunning(logger), ).Then(job) -Thread safety +# Thread safety Since the Cron service runs concurrently with the calling code, some amount of care must be taken to ensure proper synchronization. @@ -200,7 +200,7 @@ care must be taken to ensure proper synchronization. All cron methods are designed to be correctly synchronized as long as the caller ensures that invocations have a clear happens-before ordering between them. -Logging +# Logging Cron defines a Logger interface that is a subset of the one defined in github.com/go-logr/logr. It has two logging levels (Info and Error), and @@ -216,16 +216,15 @@ Activate it with a one-off logger as follows: cron.WithLogger( cron.VerbosePrintfLogger(log.New(os.Stdout, "cron: ", log.LstdFlags)))) - -Implementation +# Implementation Cron entries are stored in an array, sorted by their next activation time. Cron sleeps until the next job is due to be run. Upon waking: - - it runs each entry that is active on that second - - it calculates the next run times for the jobs that were run - - it re-sorts the array of entries by next activation time. - - it goes to sleep until the soonest job. + - it runs each entry that is active on that second + - it calculates the next run times for the jobs that were run + - it re-sorts the array of entries by next activation time. + - it goes to sleep until the soonest job. */ package cron diff --git a/go.mod b/go.mod index 8c95bf47..050373ae 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ -module github.com/robfig/cron/v3 +module github.com/netresearch/go-cron -go 1.12 +go 1.25 + +toolchain go1.25.0 diff --git a/logger.go b/logger.go index b4efcc05..bfb89f36 100644 --- a/logger.go +++ b/logger.go @@ -1,7 +1,7 @@ package cron import ( - "io/ioutil" + "io" "log" "os" "strings" @@ -12,7 +12,7 @@ import ( var DefaultLogger Logger = PrintfLogger(log.New(os.Stdout, "cron: ", log.LstdFlags)) // DiscardLogger can be used by callers to discard all log messages. -var DiscardLogger Logger = PrintfLogger(log.New(ioutil.Discard, "", 0)) +var DiscardLogger Logger = PrintfLogger(log.New(io.Discard, "", 0)) // Logger is the interface used in this package for logging, so that any backend // can be plugged in. It is a subset of the github.com/go-logr/logr interface. diff --git a/option_test.go b/option_test.go index 8aef1682..aa5884af 100644 --- a/option_test.go +++ b/option_test.go @@ -15,7 +15,7 @@ func TestWithLocation(t *testing.T) { } func TestWithParser(t *testing.T) { - var parser = NewParser(Dow) + parser := NewParser(Dow) c := New(WithParser(parser)) if c.parser != parser { t.Error("expected provided parser") @@ -24,7 +24,7 @@ func TestWithParser(t *testing.T) { func TestWithVerboseLogger(t *testing.T) { var buf syncWriter - var logger = log.New(&buf, "", log.LstdFlags) + logger := log.New(&buf, "", log.LstdFlags) c := New(WithLogger(VerbosePrintfLogger(logger))) if c.logger.(printfLogger).logger != logger { t.Error("expected provided logger") diff --git a/parser.go b/parser.go index 8da6547a..40774dc6 100644 --- a/parser.go +++ b/parser.go @@ -56,18 +56,17 @@ type Parser struct { // // Examples // -// // Standard parser without descriptors -// specParser := NewParser(Minute | Hour | Dom | Month | Dow) -// sched, err := specParser.Parse("0 0 15 */3 *") +// // Standard parser without descriptors +// specParser := NewParser(Minute | Hour | Dom | Month | Dow) +// sched, err := specParser.Parse("0 0 15 */3 *") // -// // Same as above, just excludes time fields -// specParser := NewParser(Dom | Month | Dow) -// sched, err := specParser.Parse("15 */3 *") -// -// // Same as above, just makes Dow optional -// specParser := NewParser(Dom | Month | DowOptional) -// sched, err := specParser.Parse("15 */3") +// // Same as above, just excludes time fields +// specParser := NewParser(Dom | Month | Dow) +// sched, err := specParser.Parse("15 */3 *") // +// // Same as above, just makes Dow optional +// specParser := NewParser(Dom | Month | DowOptional) +// sched, err := specParser.Parse("15 */3") func NewParser(options ParseOption) Parser { optionals := 0 if options&DowOptional > 0 { @@ -91,15 +90,23 @@ func (p Parser) Parse(spec string) (Schedule, error) { } // Extract timezone if present - var loc = time.Local + loc := time.Local if strings.HasPrefix(spec, "TZ=") || strings.HasPrefix(spec, "CRON_TZ=") { var err error i := strings.Index(spec, " ") eq := strings.Index(spec, "=") + // Fix for issue #554: Check if space exists after timezone + if i == -1 { + return nil, fmt.Errorf("missing fields after timezone in spec %q", spec) + } if loc, err = time.LoadLocation(spec[eq+1 : i]); err != nil { return nil, fmt.Errorf("provided bad location %s: %v", spec[eq+1:i], err) } spec = strings.TrimSpace(spec[i:]) + // Fix for issue #555: Check if spec has content after timezone + if len(spec) == 0 { + return nil, fmt.Errorf("missing fields after timezone in spec") + } } // Handle named schedules (descriptors), if configured @@ -247,7 +254,9 @@ func getField(field string, r bounds) (uint64, error) { } // getRange returns the bits indicated by the given expression: -// number | number "-" number [ "/" number ] +// +// number | number "-" number [ "/" number ] +// // or error parsing range. func getRange(expr string, r bounds) (uint64, error) { var ( diff --git a/parser_test.go b/parser_test.go index 41c8c520..a8c48070 100644 --- a/parser_test.go +++ b/parser_test.go @@ -119,7 +119,7 @@ func TestBits(t *testing.T) { } func TestParseScheduleErrors(t *testing.T) { - var tests = []struct{ expr, err string }{ + tests := []struct{ expr, err string }{ {"* 5 j * * *", "failed to parse int from"}, {"@every Xm", "failed to parse duration"}, {"@unrecognized", "unrecognized descriptor"}, @@ -381,3 +381,86 @@ func annual(loc *time.Location) *SpecSchedule { Location: loc, } } + +// TestTimezoneParsingPanic tests fix for issues #554 and #555 +// These bugs caused panics when parsing malformed timezone specs +func TestTimezoneParsingPanic(t *testing.T) { + // Issue #554: TZ= without space causes slice bounds panic + // Previously: strings.Index(spec, " ") returns -1, then spec[eq+1 : i] panics + panicSpecs := []struct { + name string + spec string + err string + }{ + { + name: "TZ_without_space", + spec: "TZ=0", + err: "missing fields after timezone", + }, + { + name: "TZ_equals_only", + spec: "TZ=", + err: "missing fields after timezone", + }, + { + name: "CRON_TZ_without_space", + spec: "CRON_TZ=UTC", + err: "missing fields after timezone", + }, + { + name: "TZ_with_only_spaces", + spec: "TZ=UTC ", + err: "missing fields after timezone", + }, + { + name: "TZ_valid_timezone_no_fields", + spec: "TZ=America/New_York", + err: "missing fields after timezone", + }, + } + + for _, tc := range panicSpecs { + t.Run(tc.name, func(t *testing.T) { + // This should NOT panic - it should return an error + defer func() { + if r := recover(); r != nil { + t.Errorf("ParseStandard(%q) panicked: %v", tc.spec, r) + } + }() + + _, err := ParseStandard(tc.spec) + if err == nil { + t.Errorf("ParseStandard(%q) expected error, got nil", tc.spec) + return + } + if !strings.Contains(err.Error(), tc.err) { + t.Errorf("ParseStandard(%q) error = %q, want error containing %q", tc.spec, err.Error(), tc.err) + } + }) + } +} + +// TestTimezoneValidParsing ensures valid timezone specs still work +func TestTimezoneValidParsing(t *testing.T) { + validSpecs := []string{ + "TZ=UTC * * * * *", + "CRON_TZ=UTC * * * * *", + "TZ=America/New_York 0 5 * * *", + "CRON_TZ=Asia/Tokyo 30 4 * * *", + } + + for _, spec := range validSpecs { + t.Run(spec, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Errorf("ParseStandard(%q) panicked: %v", spec, r) + } + }() + + _, err := ParseStandard(spec) + if err != nil { + t.Errorf("ParseStandard(%q) unexpected error: %v", spec, err) + } + }) + } +} diff --git a/spec.go b/spec.go index fa1e241e..597546fa 100644 --- a/spec.go +++ b/spec.go @@ -140,7 +140,16 @@ WRAP: added = true t = time.Date(t.Year(), t.Month(), t.Day(), t.Hour(), 0, 0, 0, loc) } + prev := t t = t.Add(1 * time.Hour) + // ISC cron behavior: If time was adjusted one hour forward due to DST, + // jobs that would have run in the skipped interval will run immediately. + // Fix for PR #541: Don't skip crons when time jumps forward due to DST. + if t.Hour()-prev.Hour() == 2 { + if 1< 0 { + break + } + } if t.Hour() == 0 { goto WRAP @@ -178,8 +187,8 @@ WRAP: // restrictions are satisfied by the given time. func dayMatches(s *SpecSchedule, t time.Time) bool { var ( - domMatch bool = 1< 0 - dowMatch bool = 1< 0 + domMatch = 1< 0 + dowMatch = 1< 0 ) if s.Dom&starBit > 0 || s.Dow&starBit > 0 { return domMatch && dowMatch diff --git a/spec_test.go b/spec_test.go index 1b8a503e..b70e82e7 100644 --- a/spec_test.go +++ b/spec_test.go @@ -64,7 +64,7 @@ func TestActivation(t *testing.T) { } actual := sched.Next(getTime(test.time).Add(-1 * time.Second)) expected := getTime(test.time) - if test.expected && expected != actual || !test.expected && expected == actual { + if test.expected && expected != actual || !test.expected && expected.Equal(actual) { t.Errorf("Fail evaluating %s on %s: (expected) %s != %s (actual)", test.spec, test.time, expected, actual) } @@ -111,7 +111,8 @@ func TestNext(t *testing.T) { {"Mon Jul 9 23:35 2012", "0 0 0 29 Feb ?", "Mon Feb 29 00:00 2016"}, // Daylight savings time 2am EST (-5) -> 3am EDT (-4) - {"2012-03-11T00:00:00-0500", "TZ=America/New_York 0 30 2 11 Mar ?", "2013-03-11T02:30:00-0400"}, + // ISC cron behavior: Jobs in skipped DST hour run immediately at 3am + {"2012-03-11T00:00:00-0500", "TZ=America/New_York 0 30 2 11 Mar ?", "2012-03-11T03:30:00-0400"}, // hourly job {"2012-03-11T00:00:00-0500", "TZ=America/New_York 0 0 * * * ?", "2012-03-11T01:00:00-0500"}, @@ -129,8 +130,8 @@ func TestNext(t *testing.T) { {"2012-03-11T00:00:00-0500", "TZ=America/New_York 0 0 1 * * ?", "2012-03-11T01:00:00-0500"}, {"2012-03-11T01:00:00-0500", "TZ=America/New_York 0 0 1 * * ?", "2012-03-12T01:00:00-0400"}, - // 2am nightly job (skipped) - {"2012-03-11T00:00:00-0500", "TZ=America/New_York 0 0 2 * * ?", "2012-03-12T02:00:00-0400"}, + // 2am nightly job - ISC cron behavior: runs at 3am when DST skips 2am + {"2012-03-11T00:00:00-0500", "TZ=America/New_York 0 0 2 * * ?", "2012-03-11T03:00:00-0400"}, // Daylight savings time 2am EDT (-4) => 1am EST (-5) {"2012-11-04T00:00:00-0400", "TZ=America/New_York 0 30 2 04 Nov ?", "2012-11-04T02:30:00-0500"}, @@ -219,7 +220,7 @@ func getTime(value string) time.Time { return time.Time{} } - var location = time.Local + location := time.Local if strings.HasPrefix(value, "TZ=") { parts := strings.Fields(value) loc, err := time.LoadLocation(parts[0][len("TZ="):]) @@ -230,7 +231,7 @@ func getTime(value string) time.Time { value = parts[1] } - var layouts = []string{ + layouts := []string{ "Mon Jan 2 15:04 2006", "Mon Jan 2 15:04:05 2006", }