From 4d5c9b9abe38d39cb159fbbf1a50eb43580440bd Mon Sep 17 00:00:00 2001
From: Thomas Orozco <thomas@orozco.fr>
Date: Wed, 1 Aug 2018 16:15:12 +0200
Subject: [PATCH] Fix bad shorthand parse

In crontab.go, we rely on https://github.com/gorhill/cronexpr to parse
cron expressions, and to tell us when they're invalid.

To choose what to feed into cronexpr, we pick out a number of tokens
from the start of the cron line that might yield a valid cron
expression, starting with the most tokens (we try 7, then 6, then 5,
then 1). When we hit a valid cron expression, we stop there.

Unfortunately, I overlooked the fact that cronexpr isn't exactly
designed for this use case, and will not return an error when
passed e.g. `@hourly foo` (it just ignores `foo` in this case).

This is a problem if the user is using a shorthand cron expression and a
command with 4 or more tokens: in this case, we'll attempt to parse the
shorthand and the 4 first tokens in cronexpr, which will work (it'll
ignore the tokens altogether), and we'll be left with a truncated
command (missing the 4 first tokens).

The ideal fix would be to have cronexpr reject this invalid expression
for us, so I'm opening a PR there. In the meantime, I'm also opening
this PR in Supercronic with a fix my fork of cronexpr.
---
 Gopkg.lock              | 43 ++++++++++++++++++++++++++++++++++-------
 Gopkg.toml              |  3 ++-
 crontab/crontab.go      |  2 +-
 crontab/crontab_test.go | 18 +++++++++++++++++
 4 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/Gopkg.lock b/Gopkg.lock
index 2fd0bd7..f5e6999 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -2,43 +2,72 @@
 
 
 [[projects]]
+  digest = "1:0a39ec8bf5629610a4bc7873a92039ee509246da3cef1a0ea60f1ed7e5f9cea5"
   name = "github.com/davecgh/go-spew"
   packages = ["spew"]
+  pruneopts = ""
   revision = "346938d642f2ec3594ed81d874461961cd0faa76"
   version = "v1.1.0"
 
 [[projects]]
+  branch = "strict"
+  digest = "1:e5d20cc67b8eb587f3f6e1c2be24a217f7eebec257977fc48034204faf38f9c2"
   name = "github.com/gorhill/cronexpr"
   packages = ["."]
-  revision = "a557574d6c024ed6e36acc8b610f5f211c91568a"
-  version = "1.0.0"
+  pruneopts = ""
+  revision = "b648cc9a908c22490de781dbe600459edd0ac533"
+  source = "github.com/krallin/cronexpr"
 
 [[projects]]
+  digest = "1:256484dbbcd271f9ecebc6795b2df8cad4c458dd0f5fd82a8c2fa0c29f233411"
   name = "github.com/pmezard/go-difflib"
   packages = ["difflib"]
+  pruneopts = ""
   revision = "792786c7400a136282c1664665ae0a8db921c6c2"
   version = "v1.0.0"
 
 [[projects]]
+  digest = "1:2c38661f5fb038bfb95197e0e5bc7a8f050d1f992c5ddaa01945e58fe2ef00de"
   name = "github.com/sirupsen/logrus"
   packages = ["."]
-  revision = "202f25545ea4cf9b191ff7f846df5d87c9382c2b"
-  version = "v1.0.0"
+  pruneopts = ""
+  revision = "3e01752db0189b9157070a0e1668a620f9a85da2"
+  version = "v1.0.6"
 
 [[projects]]
+  digest = "1:8e7d1910593db3b38b0c3e4bf3f82bc041dfddfed5f6d36a47f3ad7e72710d8d"
   name = "github.com/stretchr/testify"
   packages = ["assert"]
+  pruneopts = ""
   revision = "69483b4bd14f5845b5a1e55bca19e954e827f1d0"
   version = "v1.1.4"
 
 [[projects]]
+  branch = "master"
+  digest = "1:798981f04990c9da3f4fde5d7b717e9cdabed624450aaecf01e9a2896d076040"
+  name = "golang.org/x/crypto"
+  packages = ["ssh/terminal"]
+  pruneopts = ""
+  revision = "c126467f60eb25f8f27e5a981f32a87e3965053f"
+
+[[projects]]
+  branch = "master"
+  digest = "1:cfc5bd16b264cf9960abb45ecc09e4f3c70638d838f0d9c18e39b5b35af45e86"
   name = "golang.org/x/sys"
-  packages = ["unix"]
-  revision = "739734461d1c916b6c72a63d7efda2b27edb369f"
+  packages = [
+    "unix",
+    "windows",
+  ]
+  pruneopts = ""
+  revision = "bd9dbc187b6e1dacfdd2722a87e83093c2d7bd6e"
 
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "0078354ea3c0d22f2f5089053f9c6f93edbf5a7a5a3d32d195e9e2746510fa5f"
+  input-imports = [
+    "github.com/gorhill/cronexpr",
+    "github.com/sirupsen/logrus",
+    "github.com/stretchr/testify/assert",
+  ]
   solver-name = "gps-cdcl"
   solver-version = 1
diff --git a/Gopkg.toml b/Gopkg.toml
index 18e5b44..d40c76b 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -23,7 +23,8 @@
 
 [[constraint]]
   name = "github.com/gorhill/cronexpr"
-  version = "~1.0.0"
+  branch = "strict"
+  source = "github.com/krallin/cronexpr"
 
 [[constraint]]
   name = "github.com/sirupsen/logrus"
diff --git a/crontab/crontab.go b/crontab/crontab.go
index b9a592d..283a75d 100644
--- a/crontab/crontab.go
+++ b/crontab/crontab.go
@@ -36,7 +36,7 @@ func parseJobLine(line string) (*CrontabLine, error) {
 		// TODO: Should receive a logger?
 		logrus.Debugf("try parse(%d): %s[0:%d] = %s", count, line, scheduleEnds, line[0:scheduleEnds])
 
-		expr, err := cronexpr.Parse(line[:scheduleEnds])
+		expr, err := cronexpr.ParseStrict(line[:scheduleEnds])
 
 		if err != nil {
 			continue
diff --git a/crontab/crontab_test.go b/crontab/crontab_test.go
index 43deaca..73045f8 100644
--- a/crontab/crontab_test.go
+++ b/crontab/crontab_test.go
@@ -229,6 +229,24 @@ var parseCrontabTestCases = []struct {
 		},
 	},
 
+	{
+		"@hourly foo1 foo2 foo3 foo4 foo5 foo6",
+		&Crontab{
+			Context: &Context{
+				Shell:   "/bin/sh",
+				Environ: map[string]string{},
+			},
+			Jobs: []*Job{
+				{
+					CrontabLine: CrontabLine{
+						Schedule: "@hourly",
+						Command:  "foo1 foo2 foo3 foo4 foo5 foo6",
+					},
+				},
+			},
+		},
+	},
+
 	// Failure cases
 	{"* foo \n", nil},
 	{"* some * * *  more\n", nil},