From 8bb99fd852f1ca95eb34114270d337c0c830a969 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Mon, 21 Nov 2022 14:03:13 +0000 Subject: dashboard: always send replies from the full email Don't take the bug ID from the parsed incoming message, as it's empty if we received a request from some mailing list. Use the ID from the BugReporting structure instead. --- dashboard/app/email_test.go | 19 +++++++++++-------- dashboard/app/reporting_email.go | 31 +++++++++++++++++-------------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index c0c321f6f..c0003d2f0 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -887,7 +887,7 @@ func TestBugFromSubjectInference(t *testing.T) { const crashTitle = "WARNING in corrupted" - upstreamCrash := func(title string) { + upstreamCrash := func(title string) string { // Upload some garbage crashes. crash := testCrash(build, 1) crash.Title = title @@ -897,11 +897,12 @@ func TestBugFromSubjectInference(t *testing.T) { sender := c.pollEmailBug().Sender c.incomingEmail(sender, "#syz upstream\n") - c.pollEmailBug() + + return c.pollEmailBug().Sender } upstreamCrash("unrelated crash") - upstreamCrash(crashTitle) + origSender := upstreamCrash(crashTitle) upstreamCrash("unrelated crash 2") mailingList := "<" + config.Namespaces["access-public"].Reporting[1].Config.(*EmailConfig).Email + ">" @@ -913,8 +914,9 @@ func TestBugFromSubjectInference(t *testing.T) { EmailOptOrigFrom("test@requester.com"), EmailOptFrom(mailingList), EmailOptSubject(subject), ) - body := c.pollEmailBug().Body - c.expectEQ(strings.Contains(body, "can't find the corresponding bug"), true) + syzbotReply := c.pollEmailBug() + c.expectNE(syzbotReply.Sender, origSender) + c.expectEQ(strings.Contains(syzbotReply.Body, "can't find the corresponding bug"), true) // Now try to test the exiting bug, but with the wrong mailing list. subject = "Re: " + crashTitle @@ -923,7 +925,7 @@ func TestBugFromSubjectInference(t *testing.T) { EmailOptOrigFrom("test@requester.com"), EmailOptFrom(""), EmailOptSubject(subject), ) - body = c.pollEmailBug().Body + body := c.pollEmailBug().Body c.expectEQ(strings.Contains(body, "can't find the corresponding bug"), true) // Now try to test the exiting bug with the proper mailing list. @@ -932,8 +934,9 @@ func TestBugFromSubjectInference(t *testing.T) { EmailOptFrom(mailingList), EmailOptOrigFrom("test@requester.com"), EmailOptSubject(subject), ) - body = c.pollEmailBug().Body - c.expectEQ(strings.Contains(body, "This crash does not have a reproducer"), true) + syzbotReply = c.pollEmailBug() + c.expectEQ(syzbotReply.Sender, origSender) + c.expectEQ(strings.Contains(syzbotReply.Body, "This crash does not have a reproducer"), true) // Test that a different type of email headers is also parsed fine. c.incomingEmail("bugs@syzkaller.com", diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go index cf7880fd5..2324e2ac3 100644 --- a/dashboard/app/reporting_email.go +++ b/dashboard/app/reporting_email.go @@ -328,18 +328,19 @@ func incomingMail(c context.Context, r *http.Request) error { Link: msg.Link, CC: msg.Cc, } + bugID := bugInfo.bugReporting.ID switch msg.Command { case email.CmdNone, email.CmdUpstream, email.CmdInvalid, email.CmdUnDup: case email.CmdFix: if msg.CommandArgs == "" { - return replyTo(c, msg, "no commit title") + return replyTo(c, msg, bugID, "no commit title") } cmd.FixCommits = []string{msg.CommandArgs} case email.CmdUnFix: cmd.ResetFixCommits = true case email.CmdDup: if msg.CommandArgs == "" { - return replyTo(c, msg, "no dup title") + return replyTo(c, msg, bugID, "no dup title") } cmd.DupOf = msg.CommandArgs cmd.DupOf = strings.TrimSpace(strings.TrimPrefix(cmd.DupOf, replySubjectPrefix)) @@ -350,17 +351,17 @@ func incomingMail(c context.Context, r *http.Request) error { if msg.Command != email.CmdUnknown { log.Errorf(c, "unknown email command %v %q", msg.Command, msg.CommandStr) } - return replyTo(c, msg, fmt.Sprintf("unknown command %q", msg.CommandStr)) + return replyTo(c, msg, bugID, fmt.Sprintf("unknown command %q", msg.CommandStr)) } ok, reply, err := incomingCommand(c, cmd) if err != nil { return nil // the error was already logged } if !ok && reply != "" { - return replyTo(c, msg, reply) + return replyTo(c, msg, bugID, reply) } if !mailingListInCC && msg.Command != email.CmdNone && msg.Command != email.CmdUnCC { - warnMailingListInCC(c, msg, mailingList) + warnMailingListInCC(c, msg, bugID, mailingList) } return nil } @@ -379,14 +380,15 @@ var emailCmdToStatus = map[email.Command]dashapi.BugStatus{ func handleTestCommand(c context.Context, info *bugInfoResult, 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))) + return replyTo(c, msg, info.bugReporting.ID, + fmt.Sprintf("want 2 args (repo, branch), got %v", len(args))) } reply := handleTestRequest(c, &testReqArgs{ bug: info.bug, bugKey: info.bugKey, bugReporting: info.bugReporting, user: msg.Author, extID: msg.MessageID, link: msg.Link, patch: msg.Patch, repo: args[0], branch: args[1], jobCC: msg.Cc}) if reply != "" { - return replyTo(c, msg, reply) + return replyTo(c, msg, info.bugReporting.ID, reply) } return nil } @@ -437,7 +439,7 @@ func loadBugInfo(c context.Context, msg *email.Email) *bugInfoResult { log.Errorf(c, "failed to format sender email address: %v", err) from = "ERROR" } - if err := replyTo(c, msg, fmt.Sprintf(replyNoBugID, from)); err != nil { + if err := replyTo(c, msg, "", fmt.Sprintf(replyNoBugID, from)); err != nil { log.Errorf(c, "failed to send reply: %v", err) } } @@ -451,7 +453,7 @@ func loadBugInfo(c context.Context, msg *email.Email) *bugInfoResult { log.Errorf(c, "failed to format sender email address: %v", err) from = "ERROR" } - if err := replyTo(c, msg, fmt.Sprintf(replyBadBugID, from)); err != nil { + if err := replyTo(c, msg, "", fmt.Sprintf(replyBadBugID, from)); err != nil { log.Errorf(c, "failed to send reply: %v", err) } return nil @@ -459,7 +461,7 @@ func loadBugInfo(c context.Context, msg *email.Email) *bugInfoResult { bugReporting, _ := bugReportingByID(bug, msg.BugID) if bugReporting == nil { log.Errorf(c, "can't find bug reporting: %v", err) - if err := replyTo(c, msg, "Can't find the corresponding bug."); err != nil { + if err := replyTo(c, msg, "", "Can't find the corresponding bug."); err != nil { log.Errorf(c, "failed to send reply: %v", err) } return nil @@ -609,12 +611,12 @@ func checkMailingListInCC(c context.Context, msg *email.Email, mailingList strin return false } -func warnMailingListInCC(c context.Context, msg *email.Email, mailingList string) { +func warnMailingListInCC(c context.Context, msg *email.Email, bugID, mailingList string) { reply := fmt.Sprintf("Your '%v' command is accepted, but please keep %v mailing list"+ " in CC next time. It serves as a history of what happened with each bug report."+ " Thank you.", msg.CommandStr, mailingList) - if err := replyTo(c, msg, reply); err != nil { + if err := replyTo(c, msg, bugID, reply); err != nil { log.Errorf(c, "failed to send email reply: %v", err) } } @@ -636,9 +638,10 @@ func sendMailText(c context.Context, cfg *EmailConfig, subject, from string, to return sendEmail(c, msg) } -func replyTo(c context.Context, msg *email.Email, reply string) error { - from, err := email.AddAddrContext(fromAddr(c), msg.BugID) +func replyTo(c context.Context, msg *email.Email, bugID, reply string) error { + from, err := email.AddAddrContext(fromAddr(c), bugID) if err != nil { + log.Errorf(c, "failed to build the From address: %v", err) return err } log.Infof(c, "sending reply: to=%q cc=%q subject=%q reply=%q", -- cgit mrf-deployment