dashboard/app: slightly relax command parsing

Users have misspelled test: multiple times.
Accept commands without the colon.

6 files changed, 153 insertions, 79 deletions

diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go
index 07a242e22..2d848c9fa 100644
--- a/dashboard/app/email_test.go
+++ b/dashboard/app/email_test.go
@@ -434,7 +434,7 @@ func TestEmailUndup(t *testing.T) {
 	msg2 := c.pollEmailBug()
 
 	// Dup crash2 to crash1.
-	c.incomingEmail(msg2.Sender, "#syz dup: BUG: slightly more elaborate title")
+	c.incomingEmail(msg2.Sender, "#syz dup BUG: slightly more elaborate title")
 	c.expectNoEmail()
 
 	// Undup crash2.
diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go
index 73e324733..b6614d3ff 100644
--- a/dashboard/app/jobs_test.go
+++ b/dashboard/app/jobs_test.go
@@ -282,7 +282,7 @@ func TestJobWithoutPatch(t *testing.T) {
 	_, extBugID, err := email.RemoveAddrContext(sender)
 	c.expectOK(err)
 
-	c.incomingEmail(sender, "#syz test: git://mygit.com/git.git 5e6a2eea\n", EmailOptMessageID(1))
+	c.incomingEmail(sender, "#syz test git://mygit.com/git.git 5e6a2eea\n", EmailOptMessageID(1))
 	pollResp := c.client2.pollJobs(build.Manager)
 	c.expectEQ(pollResp.Type, dashapi.JobTestPatch)
 	testBuild := testBuild(2)
diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go
index 12e9aae73..093c6a73e 100644
--- a/dashboard/app/notifications_test.go
+++ b/dashboard/app/notifications_test.go
@@ -79,7 +79,7 @@ func TestEmailNotifBadFix(t *testing.T) {
 	report := c.pollEmailBug()
 	c.expectEQ(report.To, []string{"test@syzkaller.com"})
 
-	c.incomingEmail(report.Sender, "#syz fix: some: commit title")
+	c.incomingEmail(report.Sender, "#syz fix some: commit title")
 	c.expectNoEmail()
 
 	// Notification about bad fixing commit should be send after 90 days.
diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go
index d05b0cd59..ae0e597ca 100644
--- a/dashboard/app/reporting_email.go
+++ b/dashboard/app/reporting_email.go
@@ -267,9 +267,9 @@ func incomingMail(c context.Context, r *http.Request) error {
 	}
 	log.Infof(c, "received email: subject %q, from %q, cc %q, msg %q, bug %q, cmd %q, link %q",
 		msg.Subject, msg.From, msg.Cc, msg.MessageID, msg.BugID, msg.Command, msg.Link)
-	if msg.Command == "fix:" && msg.CommandArgs == "exact-commit-title" {
+	if msg.Command == email.CmdFix && msg.CommandArgs == "exact-commit-title" {
 		// Sometimes it happens that somebody sends us our own text back, ignore it.
-		msg.Command, msg.CommandArgs = "", ""
+		msg.Command, msg.CommandArgs = email.CmdNone, ""
 	}
 	bug, _, reporting := loadBugInfo(c, msg)
 	if bug == nil {
@@ -282,55 +282,39 @@ func incomingMail(c context.Context, r *http.Request) error {
 	fromMailingList := email.CanonicalEmail(msg.From) == mailingList
 	mailingListInCC := checkMailingListInCC(c, msg, mailingList)
 	log.Infof(c, "from/cc mailing list: %v/%v", fromMailingList, mailingListInCC)
-	if msg.Command == "test:" {
-		args := strings.Split(msg.CommandArgs, " ")
-		if len(args) != 2 {
-			return replyTo(c, msg, fmt.Sprintf("want 2 args (repo, branch), got %v",
-				len(args)), nil)
-		}
-		reply := handleTestRequest(c, msg.BugID, email.CanonicalEmail(msg.From),
-			msg.MessageID, msg.Link, msg.Patch, args[0], args[1], msg.Cc)
-		if reply != "" {
-			return replyTo(c, msg, reply, nil)
-		}
-		return nil
+	if msg.Command == email.CmdTest {
+		return handleTestCommand(c, msg)
 	}
-	if fromMailingList && msg.Command != "" {
+	if fromMailingList && msg.Command != email.CmdNone {
 		log.Infof(c, "duplicate email from mailing list, ignoring")
 		return nil
 	}
 	cmd := &dashapi.BugUpdate{
-		ID:    msg.BugID,
-		ExtID: msg.MessageID,
-		Link:  msg.Link,
-		CC:    msg.Cc,
+		Status: emailCmdToStatus[msg.Command],
+		ID:     msg.BugID,
+		ExtID:  msg.MessageID,
+		Link:   msg.Link,
+		CC:     msg.Cc,
 	}
 	switch msg.Command {
-	case "":
-		cmd.Status = dashapi.BugStatusUpdate
-	case "upstream":
-		cmd.Status = dashapi.BugStatusUpstream
-	case "invalid":
-		cmd.Status = dashapi.BugStatusInvalid
-	case "undup":
-		cmd.Status = dashapi.BugStatusOpen
-	case "fix:":
+	case email.CmdNone, email.CmdUpstream, email.CmdInvalid, email.CmdUnDup:
+	case email.CmdFix:
 		if msg.CommandArgs == "" {
 			return replyTo(c, msg, fmt.Sprintf("no commit title"), nil)
 		}
-		cmd.Status = dashapi.BugStatusOpen
 		cmd.FixCommits = []string{msg.CommandArgs}
-	case "dup:":
+	case email.CmdDup:
 		if msg.CommandArgs == "" {
 			return replyTo(c, msg, fmt.Sprintf("no dup title"), nil)
 		}
-		cmd.Status = dashapi.BugStatusDup
 		cmd.DupOf = msg.CommandArgs
-	case "uncc", "uncc:":
-		cmd.Status = dashapi.BugStatusUnCC
+	case email.CmdUnCC:
 		cmd.CC = []string{email.CanonicalEmail(msg.From)}
 	default:
-		return replyTo(c, msg, fmt.Sprintf("unknown command %q", msg.Command), nil)
+		if msg.Command != email.CmdUnknown {
+			log.Errorf(c, "unknown email command %v %q", msg.Command, msg.CommandArgs)
+		}
+		return replyTo(c, msg, fmt.Sprintf("unknown command %q", msg.CommandArgs), nil)
 	}
 	ok, reply, err := incomingCommand(c, cmd)
 	if err != nil {
@@ -339,12 +323,35 @@ func incomingMail(c context.Context, r *http.Request) error {
 	if !ok && reply != "" {
 		return replyTo(c, msg, reply, nil)
 	}
-	if !mailingListInCC && msg.Command != "" && cmd.Status != dashapi.BugStatusUnCC {
+	if !mailingListInCC && msg.Command != email.CmdNone && msg.Command != email.CmdUnCC {
 		warnMailingListInCC(c, msg, mailingList)
 	}
 	return nil
 }
 
+var emailCmdToStatus = map[email.Command]dashapi.BugStatus{
+	email.CmdNone:     dashapi.BugStatusUpdate,
+	email.CmdUpstream: dashapi.BugStatusUpstream,
+	email.CmdInvalid:  dashapi.BugStatusInvalid,
+	email.CmdUnDup:    dashapi.BugStatusOpen,
+	email.CmdFix:      dashapi.BugStatusOpen,
+	email.CmdDup:      dashapi.BugStatusDup,
+	email.CmdUnCC:     dashapi.BugStatusUnCC,
+}
+
+func handleTestCommand(c context.Context, msg *email.Email) error {
+	args := strings.Split(msg.CommandArgs, " ")
+	if len(args) != 2 {
+		return replyTo(c, msg, fmt.Sprintf("want 2 args (repo, branch), got %v", len(args)), nil)
+	}
+	reply := handleTestRequest(c, msg.BugID, email.CanonicalEmail(msg.From),
+		msg.MessageID, msg.Link, msg.Patch, args[0], args[1], msg.Cc)
+	if reply != "" {
+		return replyTo(c, msg, reply, nil)
+	}
+	return nil
+}
+
 func handleEmailBounce(w http.ResponseWriter, r *http.Request) {
 	c := appengine.NewContext(r)
 	body, err := ioutil.ReadAll(r.Body)
@@ -365,7 +372,7 @@ var nonCriticalBounceRe = regexp.MustCompile(`\*\* Address not found \*\*|550 #5
 
 func loadBugInfo(c context.Context, msg *email.Email) (bug *Bug, bugReporting *BugReporting, reporting *Reporting) {
 	if msg.BugID == "" {
-		if msg.Command == "" {
+		if msg.Command == email.CmdNone {
 			// This happens when people CC syzbot on unrelated emails.
 			log.Infof(c, "no bug ID (%q)", msg.Subject)
 		} else {
diff --git a/pkg/email/parser.go b/pkg/email/parser.go
index ce652c5da..2566ae1d5 100644
--- a/pkg/email/parser.go
+++ b/pkg/email/parser.go
@@ -25,12 +25,28 @@ type Email struct {
 	Subject     string
 	From        string
 	Cc          []string
-	Body        string // text/plain part
-	Patch       string // attached patch, if any
-	Command     string // command to bot (#syz is stripped)
-	CommandArgs string // arguments for the command
+	Body        string  // text/plain part
+	Patch       string  // attached patch, if any
+	Command     Command // command to bot
+	CommandArgs string  // arguments for the command
 }
 
+type Command int
+
+const (
+	CmdUnknown Command = iota
+	CmdNone
+	CmdUpstream
+	CmdFix
+	CmdDup
+	CmdUnDup
+	CmdTest
+	CmdInvalid
+	CmdUnCC
+
+	cmdTest5
+)
+
 const commandPrefix = "#syz "
 
 var groupsLinkRe = regexp.MustCompile("\nTo view this discussion on the web visit" +
@@ -87,7 +103,8 @@ func Parse(r io.Reader, ownEmails []string) (*Email, error) {
 		return nil, err
 	}
 	bodyStr := string(body)
-	patch, cmd, cmdArgs := "", "", ""
+	cmd := CmdNone
+	patch, cmdArgs := "", ""
 	if !fromMe {
 		for _, a := range attachments {
 			_, patch, _ = ParsePatch(string(a))
@@ -176,9 +193,10 @@ func CanonicalEmail(email string) string {
 // extractCommand extracts command to syzbot from email body.
 // Commands are of the following form:
 // ^#syz cmd args...
-func extractCommand(body []byte) (cmd, args string) {
+func extractCommand(body []byte) (cmd Command, args string) {
 	cmdPos := bytes.Index(append([]byte{'\n'}, body...), []byte("\n"+commandPrefix))
 	if cmdPos == -1 {
+		cmd = CmdNone
 		return
 	}
 	cmdPos += len(commandPrefix)
@@ -195,18 +213,41 @@ func extractCommand(body []byte) (cmd, args string) {
 	if cmdEnd1 := bytes.IndexByte(body[cmdPos:], ' '); cmdEnd1 != -1 && cmdEnd1 < cmdEnd {
 		cmdEnd = cmdEnd1
 	}
-	cmd = string(body[cmdPos : cmdPos+cmdEnd])
+	switch string(body[cmdPos : cmdPos+cmdEnd]) {
+	default:
+		cmd = CmdUnknown
+	case "":
+		cmd = CmdNone
+	case "upstream":
+		cmd = CmdUpstream
+	case "fix", "fix:":
+		cmd = CmdFix
+	case "dup", "dup:":
+		cmd = CmdDup
+	case "undup":
+		cmd = CmdUnDup
+	case "test", "test:":
+		cmd = CmdTest
+	case "invalid":
+		cmd = CmdInvalid
+	case "uncc", "uncc:":
+		cmd = CmdUnCC
+	case "test_5_arg_cmd":
+		cmd = cmdTest5
+	}
 	// Some email clients split text emails at 80 columns are the transformation is irrevesible.
 	// We try hard to restore what was there before.
 	// For "test:" command we know that there must be 2 tokens without spaces.
 	// For "fix:"/"dup:" we need a whole non-empty line of text.
 	switch cmd {
-	case "test:":
+	case CmdTest:
 		args = extractArgsTokens(body[cmdPos+cmdEnd:], 2)
-	case "test_5_arg_cmd":
+	case cmdTest5:
 		args = extractArgsTokens(body[cmdPos+cmdEnd:], 5)
-	case "fix:", "dup:":
+	case CmdFix, CmdDup:
 		args = extractArgsLine(body[cmdPos+cmdEnd:])
+	case CmdUnknown:
+		args = extractArgsLine(body[cmdPos:])
 	}
 	return
 }
diff --git a/pkg/email/parser_test.go b/pkg/email/parser_test.go
index 7df593642..274fcd9e7 100644
--- a/pkg/email/parser_test.go
+++ b/pkg/email/parser_test.go
@@ -17,14 +17,14 @@ func TestExtractCommand(t *testing.T) {
 		t.Run(fmt.Sprint(i), func(t *testing.T) {
 			cmd, args := extractCommand([]byte(test.body))
 			if cmd != test.cmd || !reflect.DeepEqual(args, test.args) {
-				t.Logf("expect: %q %q", test.cmd, test.args)
-				t.Logf("got   : %q %q", cmd, args)
+				t.Logf("expect: %v %q", test.cmd, test.args)
+				t.Logf("got   : %v %q", cmd, args)
 				t.Fail()
 			}
 			cmd, args = extractCommand([]byte(strings.Replace(test.body, "\n", "\r\n", -1)))
 			if cmd != test.cmd || !reflect.DeepEqual(args, test.args) {
-				t.Logf("expect: %q %q", test.cmd, test.args)
-				t.Logf("got   : %q %q", cmd, args)
+				t.Logf("expect: %v %q", test.cmd, test.args)
+				t.Logf("got   : %v %q", cmd, args)
 				t.Fail()
 			}
 		})
@@ -138,7 +138,7 @@ func TestParse(t *testing.T) {
 
 var extractCommandTests = []struct {
 	body string
-	cmd  string
+	cmd  Command
 	args string
 }{
 	{
@@ -146,17 +146,17 @@ var extractCommandTests = []struct {
 
 line1
 #syz  fix:  bar baz 	`,
-		cmd:  "fix:",
+		cmd:  CmdFix,
 		args: "bar baz",
 	},
 	{
 		body: `Hello,
 
 line1
-#syz fix:  bar  	 baz
+#syz fix  bar  	 baz
 line 2
 `,
-		cmd: "fix:",
+		cmd: CmdFix,
 		args: "bar  	 baz",
 	},
 	{
@@ -165,7 +165,7 @@ line1
 > #syz fix: bar   baz
 line 2
 `,
-		cmd:  "",
+		cmd:  CmdNone,
 		args: "",
 	},
 	// This is unfortunate case when a command is split by email client
@@ -175,15 +175,15 @@ line 2
 #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
 locking/core
 `,
-		cmd:  "test:",
+		cmd:  CmdTest,
 		args: "git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core",
 	},
 	{
 		body: `
-#syz test:
+#syz test
 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core
 `,
-		cmd:  "test:",
+		cmd:  CmdTest,
 		args: "git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core",
 	},
 	{
@@ -193,7 +193,7 @@ git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
 locking/core
 locking/core
 `,
-		cmd:  "test:",
+		cmd:  CmdTest,
 		args: "git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core",
 	},
 	{
@@ -205,14 +205,14 @@ locking/core
 arg4
 arg5
 `,
-		cmd:  "test_5_arg_cmd",
+		cmd:  cmdTest5,
 		args: "arg1 arg2 arg3 arg4 arg5",
 	},
 	{
 		body: `
 #syz test_5_arg_cmd arg1
 arg2`,
-		cmd:  "test_5_arg_cmd",
+		cmd:  cmdTest5,
 		args: "arg1 arg2",
 	},
 	{
@@ -220,7 +220,7 @@ arg2`,
 #syz test_5_arg_cmd arg1
 arg2
 `,
-		cmd:  "test_5_arg_cmd",
+		cmd:  cmdTest5,
 		args: "arg1 arg2",
 	},
 	{
@@ -230,7 +230,7 @@ arg2
 
  
 `,
-		cmd:  "test_5_arg_cmd",
+		cmd:  cmdTest5,
 		args: "arg1 arg2",
 	},
 	{
@@ -240,7 +240,7 @@ arg1 arg2 arg3
 arg4 arg5
  
 `,
-		cmd:  "fix:",
+		cmd:  CmdFix,
 		args: "arg1 arg2 arg3",
 	},
 	{
@@ -248,9 +248,34 @@ arg4 arg5
 #syz  fix: arg1 arg2 arg3
 arg4 arg5 
 `,
-		cmd:  "fix:",
+		cmd:  CmdFix,
 		args: "arg1 arg2 arg3",
 	},
+	{
+		body: `
+#syz dup: title goes here
+baz
+`,
+		cmd:  CmdDup,
+		args: "title goes here",
+	},
+	{
+		body: `
+#syz dup 
+title on the next line goes here  
+but not this one
+`,
+		cmd:  CmdDup,
+		args: "title on the next line goes here",
+	},
+	{
+		body: `
+#syz foo bar
+baz
+`,
+		cmd:  CmdUnknown,
+		args: "foo bar",
+	},
 }
 
 type ParseTest struct {
@@ -295,7 +320,7 @@ To post to this group, send email to syzkaller@googlegroups.com.
 To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller/abcdef@google.com.
 For more options, visit https://groups.google.com/d/optout.`,
 			Patch:       "",
-			Command:     "fix:",
+			Command:     CmdFix,
 			CommandArgs: "arg1 arg2 arg3",
 		}},
 
@@ -316,7 +341,8 @@ last line`,
 			Cc:        []string{"bob@example.com"},
 			Body: `text body
 last line`,
-			Patch: "",
+			Patch:   "",
+			Command: CmdNone,
 		}},
 
 	{`Date: Sun, 7 May 2017 19:54:00 -0700
@@ -339,7 +365,7 @@ text body
 second line
 last line`,
 			Patch:       "",
-			Command:     "invalid",
+			Command:     CmdInvalid,
 			CommandArgs: "",
 		}},
 
@@ -364,8 +390,8 @@ second line
 last line
 #syz command`,
 			Patch:       "",
-			Command:     "command",
-			CommandArgs: "",
+			Command:     CmdUnknown,
+			CommandArgs: "command",
 		}},
 
 	{`Date: Sun, 7 May 2017 19:54:00 -0700
@@ -413,7 +439,7 @@ IHQpKSB7CiAJCXNwaW5fdW5sb2NrKCZrY292LT5sb2NrKTsKIAkJcmV0dXJuOwo=
  		spin_unlock(&kcov->lock);
  		return;
 `,
-			Command:     "",
+			Command:     CmdNone,
 			CommandArgs: "",
 		}},
 
@@ -523,8 +549,8 @@ index 3d85747bd86e..a257b872a53d 100644
   error = vfs_statx(dfd, filename, flags, &stat, mask);
   if (error)
 `,
-			Command:     "test",
-			CommandArgs: "",
+			Command:     CmdTest,
+			CommandArgs: "commit 59372bbf3abd5b24a7f6f676a3968685c280f955",
 		}},
 
 	{`Sender: syzkaller-bugs@googlegroups.com
@@ -571,7 +597,7 @@ d
 
 #syz dup: BUG: unable to handle kernel NULL pointer dereference in corrupted
 `,
-		Command:     "dup:",
+		Command:     CmdDup,
 		CommandArgs: "BUG: unable to handle kernel NULL pointer dereference in corrupted",
 	}},
 
@@ -587,7 +613,7 @@ BUG: unable to handle kernel NULL pointer dereference in corrupted
 		Body: `#syz dup:
 BUG: unable to handle kernel NULL pointer dereference in corrupted
 `,
-		Command:     "dup:",
+		Command:     CmdDup,
 		CommandArgs: "BUG: unable to handle kernel NULL pointer dereference in corrupted",
 	}},
 
@@ -603,7 +629,7 @@ When freeing a lockf struct that already is part of a linked list, make sure to
 		Body: `#syz fix:
 When freeing a lockf struct that already is part of a linked list, make sure to
 `,
-		Command:     "fix:",
+		Command:     CmdFix,
 		CommandArgs: "When freeing a lockf struct that already is part of a linked list, make sure to",
 	}},
 }