From b648cc9a908c22490de781dbe600459edd0ac533 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Wed, 1 Aug 2018 16:02:27 +0200 Subject: [PATCH 1/4] Expose ParseStrict to error on too many fields Parse currently ignores any tokens after the 7th, which means invalid cron expressions might be allowed as long as invalid tokens are found after the 7th character. In some use cases (e.g. validating cron expressions provided by a user), this might not be desirable. To allow for this use case, this adds a ParseStrict function that returns an error if too many fields are provided (it retains backwards compatibility by not touching Parse). --- cronexpr.go | 21 +++++++++++++++++++-- cronexpr_test.go | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cronexpr.go b/cronexpr.go index 58b518f..3bb6571 100644 --- a/cronexpr.go +++ b/cronexpr.go @@ -62,11 +62,22 @@ func MustParse(cronLine string) *Expression { /******************************************************************************/ // Parse returns a new Expression pointer. An error is returned if a malformed -// cron expression is supplied. +// cron expression is supplied. ParseStrict does the same thing, but unlike +// Parse, it will return an error if the provided cron line has too many +// tokens. // See for documentation // about what is a well-formed cron expression from this library's point of // view. + func Parse(cronLine string) (*Expression, error) { + return parse(cronLine, false) +} + +func ParseStrict(cronLine string) (*Expression, error) { + return parse(cronLine, true) +} + +func parse(cronLine string, strict bool) (*Expression, error) { // Maybe one of the built-in aliases is being used cron := cronNormalizer.Replace(cronLine) @@ -76,8 +87,14 @@ func Parse(cronLine string) (*Expression, error) { if fieldCount < 5 { return nil, fmt.Errorf("missing field(s)") } - // ignore fields beyond 7th + if fieldCount > 7 { + // In strict mode, error out + if strict { + return nil, fmt.Errorf("too many fields") + } + + // In non-strict mode, ignore fields beyond 7th fieldCount = 7 } diff --git a/cronexpr_test.go b/cronexpr_test.go index 6ccf7ab..0af28c4 100644 --- a/cronexpr_test.go +++ b/cronexpr_test.go @@ -307,6 +307,22 @@ func TestInterval_Interval60Issue(t *testing.T) { /******************************************************************************/ +func TestTooManyFields(t *testing.T) { + cronLine := "* * * * * * * foobar" + + _, err := ParseStrict(cronLine) + if err == nil { + t.Errorf("ParseStrict with too many fields should return err ") + } + + _, err = Parse(cronLine) + if err != nil { + t.Errorf("Parse with too many fields should not return err ") + } +} + +/******************************************************************************/ + var benchmarkExpressions = []string{ "* * * * *", "@hourly", From ee361fc97ab3885ba09de55e3aaa5a20cacca510 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Sat, 14 Nov 2020 14:13:55 +0000 Subject: [PATCH 2/4] Set up go modules --- cronexpr/go.mod | 7 +++++++ cronexpr/go.sum | 2 ++ go.mod | 3 +++ 3 files changed, 12 insertions(+) create mode 100644 cronexpr/go.mod create mode 100644 cronexpr/go.sum create mode 100644 go.mod diff --git a/cronexpr/go.mod b/cronexpr/go.mod new file mode 100644 index 0000000..6f458ff --- /dev/null +++ b/cronexpr/go.mod @@ -0,0 +1,7 @@ +module github.com/gorhill/cronexpr/cronexpr + +go 1.14 + +replace github.com/gorhill/cronexpr => ../ + +require github.com/gorhill/cronexpr v0.0.0-00010101000000-000000000000 diff --git a/cronexpr/go.sum b/cronexpr/go.sum new file mode 100644 index 0000000..a4a51bf --- /dev/null +++ b/cronexpr/go.sum @@ -0,0 +1,2 @@ +github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75 h1:f0n1xnMSmBLzVfsMMvriDyA75NB/oBgILX2GcHXIQzY= +github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75/go.mod h1:g2644b03hfBX9Ov0ZBDgXXens4rxSxmqFBbhvKv2yVA= diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..a1c711f --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/gorhill/cronexpr + +go 1.14 From cff9279a986857e46684ca2c9fb4ec2e49f0b355 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Sat, 14 Nov 2020 14:36:16 +0000 Subject: [PATCH 3/4] Reject intervals that are out of order If an interval if out of order, it ends up emitting 0000-00-00 00:00:00 as the "next" time, which is undesirable. This patch updates Parse to simply reject such intervals. Since the rejection reason might not be super obvious (the range 6-7 is actually translated to 6-0, which makes it invalid), let's also print the original vs. normalized form for clarity. See: https://github.com/aptible/supercronic/issues/63 --- cronexpr_parse.go | 3 +++ cronexpr_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/cronexpr_parse.go b/cronexpr_parse.go index a9fe746..27683c6 100644 --- a/cronexpr_parse.go +++ b/cronexpr_parse.go @@ -438,6 +438,9 @@ func genericFieldParse(s string, desc fieldDescriptor) ([]*cronDirective, error) directive.first = desc.atoi(snormal[pairs[2]:pairs[3]]) directive.last = desc.atoi(snormal[pairs[4]:pairs[5]]) directive.step = 1 + if directive.last < directive.first { + return nil, fmt.Errorf("invalid interval %s (normalized to %d-%d)", snormal, directive.first, directive.last) + } directives = append(directives, &directive) continue } diff --git a/cronexpr_test.go b/cronexpr_test.go index 0af28c4..8bf8d4d 100644 --- a/cronexpr_test.go +++ b/cronexpr_test.go @@ -305,6 +305,19 @@ func TestInterval_Interval60Issue(t *testing.T) { } } +// Issue: https://github.com/aptible/supercronic/issues/63 +func TestRange_OutOfOrder(t *testing.T) { + _, err := Parse("45 4 * * 6-7") + if err == nil { + t.Errorf("parsing with range 6-7 should return err") + } + + _, err = Parse("45 4 * * 6-5") + if err == nil { + t.Errorf("parsing with range 6-5 should return err") + } +} + /******************************************************************************/ func TestTooManyFields(t *testing.T) { From 0373391ef0a2868e45b9b005f494e70284a7f522 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Sat, 14 Nov 2020 14:51:02 +0000 Subject: [PATCH 4/4] Update import paths to krallin/cronexpr This fork is to support Supercronic, and that's how it's imported there. --- cronexpr/go.mod | 6 +++--- cronexpr/go.sum | 2 -- cronexpr/main.go | 2 +- go.mod | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cronexpr/go.mod b/cronexpr/go.mod index 6f458ff..128fd22 100644 --- a/cronexpr/go.mod +++ b/cronexpr/go.mod @@ -1,7 +1,7 @@ -module github.com/gorhill/cronexpr/cronexpr +module github.com/krallin/cronexpr/cronexpr go 1.14 -replace github.com/gorhill/cronexpr => ../ +replace github.com/krallin/cronexpr => ../ -require github.com/gorhill/cronexpr v0.0.0-00010101000000-000000000000 +require github.com/krallin/cronexpr v0.0.0-00010101000000-000000000000 diff --git a/cronexpr/go.sum b/cronexpr/go.sum index a4a51bf..e69de29 100644 --- a/cronexpr/go.sum +++ b/cronexpr/go.sum @@ -1,2 +0,0 @@ -github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75 h1:f0n1xnMSmBLzVfsMMvriDyA75NB/oBgILX2GcHXIQzY= -github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75/go.mod h1:g2644b03hfBX9Ov0ZBDgXXens4rxSxmqFBbhvKv2yVA= diff --git a/cronexpr/main.go b/cronexpr/main.go index 1d9abe5..e33213e 100644 --- a/cronexpr/main.go +++ b/cronexpr/main.go @@ -18,7 +18,7 @@ import ( "os" "time" - "github.com/gorhill/cronexpr" + "github.com/krallin/cronexpr" ) /******************************************************************************/ diff --git a/go.mod b/go.mod index a1c711f..1af9dc3 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ -module github.com/gorhill/cronexpr +module github.com/krallin/cronexpr go 1.14