gorhill / cronexpr

Cron expression parser in Go language (golang)
683 stars 168 forks source link

Hyphenated hours #35

Closed sevagh closed 6 years ago

sevagh commented 6 years ago

Hello, tracing a bug via Nomad (which uses this library), I've discovered that the following expression causes a nil pointer exception:

1 15-0 * * 1-5

Digging into the code, it's this:

func (expr *Expression) hourFieldHandler(s string) error {
    var err error
    expr.hourList, err = genericFieldHandler(s, hourDescriptor)
        fmt.Println(genericFieldHandler(s, hourDescriptor))
    return err
}

It produces an empty list when trying to parse 15-0:

func (expr *Expression) hourFieldHandler(s string) error {
    var err error
    expr.hourList, err = genericFieldHandler(s, hourDescriptor)
        fmt.Printf("hour: %v\tparsed: %v\n", s, expr.hourList)
    return err
}

hour: 15-0 parsed: [] - this leads to expr.hourList[0] producing a nil pointer over here: https://github.com/gorhill/cronexpr/blob/master/cronexpr_next.go#L115

Is this syntax supposed to be supported? Your doc says hyphens are.

Here's the stacktrace:

panic: runtime error: index out of range [recovered]
        panic: runtime error: index out of range

goroutine 5 [running]:
testing.tRunner.func1(0xc4200bc0f0)
        /usr/local/go/src/testing/testing.go:711 +0x2d2
panic(0x5281e0, 0x5fe0c0)
        /usr/local/go/src/runtime/panic.go:491 +0x283
github.com/hashicorp/nomad/vendor/github.com/gorhill/cronexpr.(*Expression).nextDayOfMonth(0xc4200c8000, 0xbeb0a5b636ed7f9b, 0x592b5, 0x605ae0, 0x0, 0x0, 0x0)
        /home/shanssian/go/src/github.com/hashicorp/nomad/vendor/github.com/gorhill/cronexpr/cronexpr_next.go:118 +0x2be
gorhill commented 6 years ago

15-0 is not valid range. The documentation says allowed values for hour are between 0 and 23, and further states:

As of now, the behavior of the code is undetermined if a malformed cron expression is supplied

sevagh commented 6 years ago

Thanks for the reply.

Field name     Mandatory?   Allowed values    Allowed special characters
----------     ----------   --------------    --------------------------
Hours          Yes          0-23              * / , -

But it says - is an allowed special character for the hour field, am I misunderstanding that?

gorhill commented 6 years ago

15-23 would be valid. But what is 15-0 supposed to mean?

sevagh commented 6 years ago

Aw crap, you're right. Crontab.guru rejects it: https://crontab.guru/#1_15-0_*_*_1-5 15-0 is indeed garbage. I must have mistyped it into crontab.guru the first time.

sevagh commented 6 years ago

Thanks much for the help!

dadgar commented 6 years ago

@gorhill Can we have this be returned as an error from Parse?