internal/coreruleset/coreruleset_test.go

@@ -625,11 +625,12 @@ const passwordField = "password="
 var hardValues = []string{"python3", "powershell", "ls%20-la", ".htaccess"}
 
 // wantRecordedRequestsPass checks that none of the recorded ordinary
-// requests in the file name in testdata is refused, at the defaults and
+// requests in the file name in testdata is refused. A request with a body
-// while bodies are read. A request with neither a body nor a Content-Type
+// or a Content-Type header is checked while bodies are read, and any other
-// header is checked only at the defaults: with neither, the set that reads
+// request at the defaults. Reading bodies only adds rule matches, so a
-// bodies has nothing more to check. Each request runs as its own parallel
+// request that passes while bodies are read passes at the defaults too;
-// subtest.
+// TestReadingBodiesOnlyAddsMatches checks that. Each request runs as its
+// own parallel subtest.
 func wantRecordedRequestsPass(t *testing.T, name string) {
 	t.Helper()
 
@@ -643,21 +644,21 @@ func wantRecordedRequestsPass(t *testing.T, name string) {
 
 			r := request{recorded.Method, recorded.Target, recorded.Headers}
 
-			got, _ := inspectBody(t, defaults, r, recorded.Body)
-			if got.Score >= threshold {
-				t.Errorf("%s %s refused: %+v", r.method, r.target, got)
-			}
-
 			typed := slices.ContainsFunc(recorded.Headers, func(header string) bool {
 				name, _, _ := strings.Cut(header, ":")
 
 				return strings.EqualFold(name, "Content-Type")
 			})
 			if recorded.Body == "" && !typed {
+				got, _ := inspectBody(t, defaults, r, "")
+				if got.Score >= threshold {
+					t.Errorf("%s %s refused: %+v", r.method, r.target, got)
+				}
+
 				return
 			}
 
-			got, _ = inspectBody(t, bodies, r, recorded.Body)
+			got, _ := inspectBody(t, bodies, r, recorded.Body)
 			if got.Score >= threshold {
 				t.Errorf("%s %s %.200q refused while bodies are read: %+v",
 					r.method, r.target, recorded.Body, got)
@@ -666,6 +667,78 @@ func wantRecordedRequestsPass(t *testing.T, name string) {
 	}
 }
 
+// TestReadingBodiesOnlyAddsMatches checks that every rule a recorded
+// request with a body matches at the defaults also matches while bodies
+// are read, with a score no lower. It checks every 40th such request of
+// each app, as recorded and with an SQL injection added to its query, which
+// the defaults must match.
+func TestReadingBodiesOnlyAddsMatches(t *testing.T) {
+	t.Parallel()
+
+	defaults, bodies := atDefaults(t), readingBodies(t)
+
+	for _, name := range []string{
+		"drupal-requests.txt",
+		"mastodon-requests.txt",
+		"mediawiki-requests.txt",
+		"nextcloud-requests.txt",
+		"vaultwarden-requests.txt",
+		"wordpress-requests.txt",
+	} {
+		withBody := 0
+
+		for _, recorded := range recordedRequests(t, name) {
+			if recorded.Body == "" {
+				continue
+			}
+
+			withBody++
+			if withBody%40 != 1 {
+				continue
+			}
+
+			separator := "?"
+			if strings.Contains(recorded.Target, "?") {
+				separator = "&"
+			}
+
+			r := request{recorded.Method, recorded.Target, recorded.Headers}
+			wantNoMatchLost(t, defaults, bodies, r, recorded.Body)
+
+			r.target += separator + "id=" + sqlInjection
+			if atDefault := wantNoMatchLost(t, defaults, bodies, r,
+				recorded.Body); len(atDefault.RuleIDs) == 0 {
+				t.Errorf("%s %s matched nothing at the defaults", r.method, r.target)
+			}
+		}
+	}
+}
+
+// wantNoMatchLost checks that every rule defaults matches in r with body,
+// bodies matches too, with a score no lower, and returns what defaults
+// found.
+func wantNoMatchLost(
+	t *testing.T, defaults, bodies *coreruleset.CoreRuleSet, r request, body string,
+) coreruleset.Result {
+	t.Helper()
+
+	atDefault, _ := inspectBody(t, defaults, r, body)
+	whileRead, _ := inspectBody(t, bodies, r, body)
+
+	for _, id := range atDefault.RuleIDs {
+		if !slices.Contains(whileRead.RuleIDs, id) {
+			t.Errorf("%s %s: %d matched at the defaults only", r.method, r.target, id)
+		}
+	}
+
+	if whileRead.Score < atDefault.Score {
+		t.Errorf("%s %s scored %d at the defaults and %d while bodies are read",
+			r.method, r.target, atDefault.Score, whileRead.Score)
+	}
+
+	return atDefault
+}
+
 func TestOrdinaryGiteaRequestsPassAndAttacksAreRefused(t *testing.T) {
 	t.Parallel()
 
@@ -849,17 +922,17 @@ func TestOrdinaryWordPressRequestsPassAndAttacksAreRefused(t *testing.T) {
 
 			r := request{recorded.Method, recorded.Target, recorded.Headers}
 
-			crsSets := []*coreruleset.CoreRuleSet{defaults}
+			// As in wantRecordedRequestsPass, a request with a body passes at
+			// the defaults if it passes while bodies are read.
+			crs := defaults
 			if recorded.Body != "" {
-				crsSets = append(crsSets, bodies)
+				crs = bodies
 			}
 
-			for _, crs := range crsSets {
 			got, _ := inspectBody(t, crs, r, recorded.Body)
 			if got.Score >= threshold {
 				t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got)
 			}
-			}
 		})
 	}