From 11dc42f69d9a72d318a45c8b068deddd8541daee Mon Sep 17 00:00:00 2001
From: Dmitry Vyukov <dvyukov@google.com>
Date: Mon, 8 Jan 2018 13:39:47 +0100
Subject: dashboard/app: fix reporting filter skipping

We must not skip reporting if we already reported a bug to it.
This leaves orphaned bugs.
---
 dashboard/app/app_test.go       |  8 ++++++++
 dashboard/app/reporting.go      |  7 +++++--
 dashboard/app/reporting_test.go | 45 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index 990247a02..cf9347516 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -7,6 +7,7 @@ package dash
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/google/syzkaller/dashboard/dashapi"
@@ -31,6 +32,13 @@ var config = GlobalConfig{
 					Config: &TestConfig{
 						Index: 1,
 					},
+					Filter: func(bug *Bug) FilterResult {
+						if strings.HasPrefix(bug.Title, "skip without repro") &&
+							bug.ReproLevel != dashapi.ReproLevelNone {
+							return FilterSkip
+						}
+						return FilterReport
+					},
 				},
 				{
 					Name:       "reporting2",
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index f7aa6c8d7..5c694cdc6 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -153,12 +153,15 @@ func currentReporting(c context.Context, bug *Bug) (*Reporting, *BugReporting, i
 			return nil, nil, 0, "", fmt.Errorf("%v: missing in config", bugReporting.Name)
 		}
 		switch reporting.Filter(bug) {
+		case FilterSkip:
+			if bugReporting.Reported.IsZero() {
+				continue
+			}
+			fallthrough
 		case FilterReport:
 			return reporting, bugReporting, i, "", nil
 		case FilterHold:
 			return nil, nil, 0, fmt.Sprintf("%v: reporting suspended", bugReporting.Name), nil
-		case FilterSkip:
-			continue
 		}
 	}
 	return nil, nil, 0, "", fmt.Errorf("no reporting left")
diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go
index 32cb725cc..c3a5bd901 100644
--- a/dashboard/app/reporting_test.go
+++ b/dashboard/app/reporting_test.go
@@ -478,3 +478,48 @@ func TestReportingDupCrossReporting(t *testing.T) {
 		c.expectEQ(reply.OK, false)
 	}
 }
+
+func TestReportingFilter(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	build := testBuild(1)
+	c.expectOK(c.API(client1, key1, "upload_build", build, nil))
+
+	crash1 := testCrash(build, 1)
+	crash1.Title = "skip without repro 1"
+	c.expectOK(c.API(client1, key1, "report_crash", crash1, nil))
+
+	// This does not skip first reporting, because it does not have repro.
+	rep1 := reportAllBugs(c, 1)[0]
+	c.expectEQ(string(rep1.Config), `{"Index":1}`)
+
+	crash1.ReproSyz = []byte("getpid()")
+	c.expectOK(c.API(client1, key1, "report_crash", crash1, nil))
+
+	// This has repro but was already reported to first reporting,
+	// so repro must go to the first reporting as well.
+	rep2 := reportAllBugs(c, 1)[0]
+	c.expectEQ(string(rep2.Config), `{"Index":1}`)
+
+	// Now upstream it and it must go to the second reporting.
+	cmd := &dashapi.BugUpdate{
+		ID:     rep1.ID,
+		Status: dashapi.BugStatusUpstream,
+	}
+	reply := new(dashapi.BugUpdateReply)
+	c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply))
+	c.expectEQ(reply.OK, true)
+
+	rep3 := reportAllBugs(c, 1)[0]
+	c.expectEQ(string(rep3.Config), `{"Index":2}`)
+
+	// Now report a bug that must go to the second reporting right away.
+	crash2 := testCrash(build, 2)
+	crash2.Title = "skip without repro 2"
+	crash2.ReproSyz = []byte("getpid()")
+	c.expectOK(c.API(client1, key1, "report_crash", crash2, nil))
+
+	rep4 := reportAllBugs(c, 1)[0]
+	c.expectEQ(string(rep4.Config), `{"Index":2}`)
+}
-- 
cgit mrf-deployment