Say that rproxy passes a refused question's 400 to the visitor (closes #326)

The comment above readQuestion said rproxy treats a 400 without a
disposition as no answer. It passes that 400 to the visitor, and its
default disposition does not apply, as SPEC.md and internal/rproxy/waf.go
say. Comment only.

Model: opus-5-5

README.md

@@ -509,8 +509,7 @@ request reaches the app.
 
 `waf` takes the checks:
 
-3. The header limits, then the tag lists. A client in `SWW_ALLOW_NETS` skips the
+3. The tag lists.
-   header limits.
 4. The static lists, `SWW_ALLOW_NETS` and `SWW_DENY_NETS`.
 5. The bans.
 6. The lookup of the client's AS number and country.
@@ -522,7 +521,7 @@ request reaches the app.
 11. The rate limits.
 12. The trap paths.
 13. The rule files.
-14. The Core Rule Set.
+14. The header limits, and then the Core Rule Set.
 15. The tag refusals again, so that `exploitsignature` can refuse too.
 16. A path under `/_sww/` is answered `404`, so that the app never sees that
     prefix.
@@ -538,7 +537,7 @@ request reaches the app.
 A refused visitor is answered with `SWW_BAN_RESPONSE`. A `block` rule or a Core
 Rule Set match answers `403` and bans nobody, since the Core Rule Set has false
 positives; such refusals count toward `SWW_ERROR_BURST_THRESHOLD` instead. A
-request over the header limits of step 3 is answered `431`, and bans nobody
+request over the header limits of step 14 is answered `431`, and bans nobody
 either. `rproxy` itself answers `405` to a method it does not pass on, and the
 status for a size or time limit, such as `413` for a body too large.
 

SPEC.md

@@ -149,12 +149,10 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in
     - An IPv6 client is its group: the netblock of `SWW_IPV6_GROUP_PREFIX`
       around its address, a /64 by default, since one abuser usually controls a
       whole /64. A client is therefore one IPv4 address or one IPv6 group.
-3. The header limits (`header_limits`; see "Answering in time"). They come first
+3. The tag lists (`tag_lists`; see "Request tags"). Each list that holds the
-   after `SWW_ALLOW_NETS`, since they only count header fields and cookies.
+   client adds its tag. Next, `routewatch_tags` adds the client's `asn=` and
-   Next, the tag lists (`tag_lists`; see "Request tags"). Each list that holds
-   the client adds its tag. Next, `routewatch_tags` adds the client's `asn=` and
    `prefix=` tags while `SWW_ROUTEWATCH_URL` is set. A client in
-   `SWW_ALLOW_NETS` skips the header limits and gets no tag.
+   `SWW_ALLOW_NETS` gets no tag.
 4. The static lists. A client in `SWW_ALLOW_NETS` skips steps 5 to 15, the byte
    limits and the error burst. A client in `SWW_DENY_NETS` is refused
    (`deny_nets`); a client in both lists is let through.
@@ -187,9 +185,10 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in
     refused and bans the client.
 13. The rule files (`rule_files`; see "Rule files"). Matching stops at the first
     rule that refuses.
-14. The Core Rule Set (`core_rule_set`; see "Attack detection"). A match is
+14. The header limits (`header_limits`; see "Answering in time"), and then the
-    refused with `403` while `SWW_WAF_MODE` is `block`, and only logged and
+    Core Rule Set (`core_rule_set`; see "Attack detection"). A Core Rule Set
-    alerted while it is `detect`.
+    match is refused with `403` while `SWW_WAF_MODE` is `block`, and only logged
+    and alerted while it is `detect`.
 15. The tag refusals again (`tag_deny`), so that `exploitsignature` can refuse
     too.
 16. A path under `/_sww/` is answered `404`, so that the app never sees that
@@ -242,16 +241,11 @@ apply.
 ## Bans
 
 Until a ban ends, every request from its netblock is refused with
-`SWW_BAN_RESPONSE`, unless it is over the header limits. The netblock is the
+`SWW_BAN_RESPONSE`. The netblock is the client: its IPv4 address, widened to
-client: its IPv4 address, widened to `SWW_BAN_SCOPE_V4_PREFIX`, or its IPv6
+`SWW_BAN_SCOPE_V4_PREFIX`, or its IPv6 group. At the defaults a ban covers one
-group. At the defaults a ban covers one address or one /64, so a permanent ban
+address or one /64, so a permanent ban does not reach neighbours who did
-does not reach neighbours who did nothing. Requests refused under a ban are not
+nothing. Requests refused under a ban are not counted against limits. They are
-counted against limits. They are counted in the client's history and in the
+counted in the client's history and in the ban's notes.
-ban's notes.
-
-A request over the header limits is refused with `431` before the bans are
-checked (see "Answering in time"). It makes no ban, makes no ban permanent and
-is not counted in a ban's notes.
 
 An offence is a request `waf` holds against the client. It is one of:
 
@@ -2031,6 +2025,7 @@ waits on these, which run in the background:
 
 In `enforce` mode, a question does not wait for GeoJS when it is a request:
 
+- over the header limits;
 - from a client on a fetched blocklist, while `SWW_BLOCKLIST_ACTION` is `deny`;
 - from a client on the CrowdSec decision list;
 - that would take its client over a rate limit at 100% of it, since a percentage
@@ -2042,8 +2037,8 @@ number and country stay unknown until GeoJS has answered. So while
 `SWW_UNKNOWN_LIMIT_PERCENT` does not apply to it until then, so not waiting
 never lowers its limit.
 
-The header limits, the static lists and bans come before the lookup, so they
+The static lists and bans come before the lookup, so they never wait. These can
-never wait. These can still wait for GeoJS:
+still wait for GeoJS:
 
 - a country list, which needs the answer;
 - a rate limit a percentage has lowered, which needs the answer;
@@ -2062,17 +2057,13 @@ either one:
 
 Real clients never reach these limits. The answer is `deny` with `431`, and the
 log line's action is `request_header_too_large`. The refusal makes no ban and is
-no offence.
+no offence. The rate limits count the request as usual, so a client that keeps
-
+sending such requests breaks a limit.
-The header limits are the first check after `SWW_ALLOW_NETS`. A request over
-them is refused before any later check runs. So it causes no lookup, no query to
-an outside source and no rule match, and the rate limits do not count it. A
-client under a ban or in `SWW_DENY_NETS` also gets `431` for such a request.
 
 The limits apply on every path and in every `SWW_WAF_MODE`. A client in
 `SWW_ALLOW_NETS` skips them, as it skips the Core Rule Set. In `observe` mode
-such a request is answered `allow` with `would_action`, and no other check runs
+such a request is answered `allow` with `would_action`, and the Core Rule Set
-on it.
+does not read it.
 
 The Core Rule Set runs each of its rules on each cookie, so thousands of small
 cookies would keep it busy for over a second. Within the limits, one long cookie

internal/rproxy/rproxy_test.go

@@ -1117,7 +1117,6 @@ func TestAnAnswerThatCannotBePassedOnIsAnswered502(t *testing.T) {
 					t.Error("the app was asked")
 				})
 				addr, out := startRProxy(t, backend(t), app.URL, map[string]string{
-					timeoutSetting:     "60000",
 					defaultDisposition: setting,
 				})
 
@@ -1141,9 +1140,7 @@ func TestAnAnswerThatCannotBePassedOnGetsNoReport(t *testing.T) {
 	t.Parallel()
 
 	standIn := startWAF(t, overOneMiB(dispositionDeny))
-	addr, out := startRProxy(t, standIn.URL, nowhere, map[string]string{
+	addr, out := startRProxy(t, standIn.URL, nowhere, nil)
-		timeoutSetting: "60000",
-	})
 
 	if got := get(t, addr, "/"); got.status != http.StatusBadGateway {
 		t.Errorf("answered %+v, want 502", got)

internal/sww/sww_test.go

@@ -92,31 +92,6 @@ func (o *output) line(t *testing.T, key, value string) map[string]any {
 	return nil
 }
 
-// lineBeforeExit returns the first line whose msg is msg, waiting for it
-// while sww runs. It waits as long as that takes, so that a slow start
-// cannot fail the test, and fails the test if sww exits first.
-func (o *output) lineBeforeExit(
-	t *testing.T, msg string, exited <-chan int,
-) map[string]any {
-	t.Helper()
-
-	for {
-		fields := o.findLine(t, func(fields map[string]any) bool {
-			return fields["msg"] == msg
-		})
-		if fields != nil {
-			return fields
-		}
-
-		select {
-		case status := <-exited:
-			t.Fatalf("exit status %d while waiting for a line with msg %q; output:\n%s",
-				status, msg, o.text())
-		case <-time.After(pollInterval):
-		}
-	}
-}
-
 // findLine returns the first line written so far for which matches is true, or
 // nil.
 func (o *output) findLine(
@@ -261,12 +236,11 @@ func TestServesUntilToldToStop(t *testing.T) {
 	}()
 
 	// The default rule file is read.
-	readRules := out.lineBeforeExit(t, "read the rule files", exited)
+	if rules := out.line(t, "msg", "read the rule files")["rules"]; rules != 12.0 {
-	if rules := readRules["rules"]; rules != 12.0 {
 		t.Errorf("read %v rules from the default rule file, want 12", rules)
 	}
 
-	starting := out.lineBeforeExit(t, "starting", exited)
+	starting := out.line(t, "msg", "starting")
 	wantStartingLine(t, starting, dir)
 
 	addr, _ := starting["address"].(string)
@@ -312,7 +286,7 @@ func TestLogLevelHoldsBackTheLessSevereProcessLines(t *testing.T) {
 		}, out)
 	}()
 
-	out.lineBeforeExit(t, "fetching a list failed", exited)
+	out.line(t, "msg", "fetching a list failed")
 	stop()
 
 	select {
@@ -1287,7 +1261,7 @@ func runUntilStopped(
 		exited <- run(ctx, env, out)
 	}()
 
-	addr, _ := out.lineBeforeExit(t, "starting", exited)["address"].(string)
+	addr, _ := out.line(t, "msg", "starting")["address"].(string)
 	use("http://" + addr + "/")
 	stop()
 

internal/waf/answertime_test.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
-	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
@@ -17,7 +16,6 @@ import (
 
 	"sneak.berlin/go/smallwebwaf/internal/bans"
 	"sneak.berlin/go/smallwebwaf/internal/lookup"
-	"sneak.berlin/go/smallwebwaf/internal/lookup/lookuptest"
 	"sneak.berlin/go/smallwebwaf/internal/ratelimit"
 	"sneak.berlin/go/smallwebwaf/internal/reputation"
 	"sneak.berlin/go/smallwebwaf/internal/requestlog"
@@ -218,199 +216,93 @@ func TestObserveModeAllowsARequestOverTheHeaderLimitsUnread(t *testing.T) {
 	}
 }
 
-func TestTheRateLimitsDoNotCountRequestsOverTheHeaderLimits(t *testing.T) {
+func TestRequestsOverTheHeaderLimitsCountTowardTheRateLimits(t *testing.T) {
 	t.Parallel()
 
 	s := startWAF(t, "", map[string]string{rateLimitPerMinute: "1"})
-
+	ask := func() answer {
-	for range 2 {
 		req := s.question(siteA, client, "/")
 		req.Header.Set(cookieHeader, cookies(cookieLimit+1))
-		wantDisposition(t, do(t, req), tooLarge, waf.DispositionDeny)
+
+		return do(t, req)
 	}
 
-	wantDisposition(t, s.ask(client, "/"), http.StatusOK, waf.DispositionAllow)
+	wantDisposition(t, ask(), tooLarge, waf.DispositionDeny)
+	wantDisposition(t, ask(), http.StatusForbidden, waf.DispositionDeny)
+
+	lines := s.out.requestLines(t, 2)
+	wantLine(t, lines[1], http.StatusForbidden, requestlog.ActionClientIsRateLimited)
 }
 
-func TestTheHeaderLimitsComeBeforeEveryCheckThatRefuses(t *testing.T) {
+func TestTheHeaderLimitsComeAfterTheTrapPathsAndTheRuleFiles(t *testing.T) {
 	t.Parallel()
 
-	// The lookup database places client in Germany.
-	lookupDB := filepath.Join(t.TempDir(), "ipinfo_lite.mmdb")
-	lookuptest.Write(t, lookupDB, map[string]lookuptest.Network{
-		client + "/32": {ASN: asnDE, ASName: asNameDE, Country: "DE"},
-	})
-
 	for _, tc := range []struct {
-		name string
+		name, path, action string
-		env  map[string]string
-		path string
-		// prepare gets the server ready, at now, before the question.
-		prepare func(t *testing.T, server *waf.Server, now time.Time)
 	}{
-		{"SWW_DENY_NETS", map[string]string{denyNets: client}, "/", leaveAsItIs},
+		{"a trap path", "/wp-login.php", requestlog.ActionTemporarilyBanned},
-		{"a ban", nil, "/", banClient},
+		{"a block rule", "/blocked", requestlog.ActionBlockedByBlockRule},
-		{
-			"the country list", map[string]string{
-				lookupSource: fileSource, lookupDBPath: lookupDB, allowedCountries: "FR",
-			},
-			"/", leaveAsItIs,
-		},
-		{"a blocklist", map[string]string{blocklistURLs: dropURL}, "/", listClient},
-		{
-			"the CrowdSec decision list",
-			map[string]string{crowdSecURL: lapi, crowdSecKey: bouncerKey}, "/", decideOnClient,
-		},
-		{
-			"a DNS blocklist", map[string]string{
-				dnsblZones: dnsblZone, dnsblResolver: noResolver, reputationAction: actionDeny,
-			},
-			"/", func(_ *testing.T, server *waf.Server, _ time.Time) {
-				loadVerdicts(server, map[string][]string{client: {dnsblZone}})
-			},
-		},
-		{
-			"AbuseIPDB",
-			map[string]string{abuseIPDBKey: accountKey, reputationAction: actionDeny},
-			"/", func(_ *testing.T, server *waf.Server, _ time.Time) {
-				// Over SWW_ABUSEIPDB_MIN_SCORE, 75 by default.
-				loadScores(server, map[string]int64{client: 80})
-			},
-		},
-		{
-			"SWW_TAG_DENY", map[string]string{tagLists: "vpn=" + vpnList, tagDeny: "vpn"},
-			"/", func(t *testing.T, server *waf.Server, _ time.Time) {
-				t.Helper()
-				loadLists(t, server, map[string][]string{vpnList: {client}})
-			},
-		},
-		{"the rate limits", map[string]string{rateLimitPerMinute: "1"}, "/", countClient},
-		{
-			"a trap path", map[string]string{trapPaths: trapPathList}, "/wp-login.php",
-			leaveAsItIs,
-		},
-		{
-			"a block rule", map[string]string{rulesDir: writeRules(t, testRules)}, "/blocked",
-			leaveAsItIs,
-		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			s := startWAF(t, "", tc.env)
+			s := startWAF(t, "", map[string]string{
-			tc.prepare(t, s.server, s.clock.Now())
+				trapPaths: trapPathList, rulesDir: writeRules(t, testRules),
-
+			})
 			req := s.question(siteA, client, tc.path)
 			req.Header.Set(cookieHeader, cookies(cookieLimit+1))
 
-			wantDisposition(t, do(t, req), tooLarge, waf.DispositionDeny)
+			wantDisposition(t, do(t, req), http.StatusForbidden, waf.DispositionDeny)
-			wantLine(t, s.out.requestLine(t), tooLarge, requestlog.ActionRequestHeaderTooLarge)
+			wantLine(t, s.out.requestLine(t), http.StatusForbidden, tc.action)
 		})
 	}
 }
 
-func TestARequestOverTheHeaderLimitsRunsNoOtherCheck(t *testing.T) {
-	t.Parallel()
-
-	// Each check after the header limits leaves a mark on the log line of a
-	// request it reads: a tag, routewatch_tags_missing, a rule id, the
-	// counts, or the Core Rule Set's time.
-	path := filepath.Join(t.TempDir(), "ipinfo_lite.mmdb")
-	lookuptest.Write(t, path, map[string]lookuptest.Network{
-		client + "/32": {ASN: asnDE, ASName: asNameDE, Country: "DE"},
-	})
-	s := startWAF(t, "", map[string]string{
-		lookupSource: fileSource, lookupDBPath: path,
-		tagLists:      "vpn=" + vpnList,
-		routeWatchURL: startRouteWatch(t, map[string]standInRoute{client: {prefixDE, 64496}}),
-		dnsblZones:    dnsblZone, reputationAction: actionLog,
-		rulesDir: writeRules(t, "noted path log ^/\n"),
-		wafMode:  block,
-	})
-	loadLists(t, s.server, map[string][]string{vpnList: {client}})
-	loadVerdicts(s.server, map[string][]string{client: {dnsblZone}})
-
-	over := s.question(siteA, client, "/")
-	over.Header.Set(cookieHeader, cookies(cookieLimit+1))
-	wantDisposition(t, do(t, over), tooLarge, waf.DispositionDeny)
-
-	within := s.question(siteA, client, "/")
-	within.Header.Set(cookieHeader, cookies(cookieLimit))
-	wantDisposition(t, do(t, within), http.StatusOK, waf.DispositionAllow)
-
-	lines := s.out.requestLines(t, 2)
-
-	wantTags(t, lines[0])
-	wantTagsMissing(t, lines[0], "")
-
-	if lines[0].RuleIDs != nil || lines[0].Counts != (ratelimit.Counts{}) ||
-		lines[0].DurationCoreRuleSetMS != nil {
-		t.Errorf("rule ids %v, counts %+v and Core Rule Set time %v, want none",
-			lines[0].RuleIDs, lines[0].Counts, lines[0].DurationCoreRuleSetMS)
-	}
-
-	// The request within the limits shows that each check leaves its mark.
-	wantTags(t, lines[1],
-		"asn=64496", "country=DE", "dnsbl="+dnsblZone, "exploitsignature", "vpn")
-	wantTagsMissing(t, lines[1], "no_answer_yet")
-
-	if lines[1].RuleIDs == nil || lines[1].Counts.Minute != 1 ||
-		lines[1].DurationCoreRuleSetMS == nil {
-		t.Errorf("rule ids %v, counts %+v and Core Rule Set time %v, want each",
-			lines[1].RuleIDs, lines[1].Counts, lines[1].DurationCoreRuleSetMS)
-	}
-}
-
 func TestNotWaitingForGeoJSNeverLowersAClientsLimit(t *testing.T) {
 	t.Parallel()
 
-	// The client's next request takes it over the whole limit, so it does
-	// not wait.
 	geojsURL, _ := startGeoJS(t)
 	s := startWAF(t, geojsURL, map[string]string{
-		unknownLimitPercent: "0", lookupTimeout: "1h", rateLimitPerMinute: "1",
+		unknownLimitPercent: "0", lookupTimeout: "1h",
 	})
-	countClient(t, s.server, s.clock.Now())
+	req := s.question(siteA, fromDE, "/")
+	req.Header.Set(cookieHeader, cookies(cookieLimit+1))
 
-	wantDisposition(t, s.ask(fromDE, "/"), http.StatusForbidden, waf.DispositionDeny)
+	wantDisposition(t, do(t, req), tooLarge, waf.DispositionDeny)
 
 	line := s.out.requestLine(t)
-	wantLine(t, line, http.StatusForbidden, requestlog.ActionClientIsRateLimited)
+	wantLine(t, line, tooLarge, requestlog.ActionRequestHeaderTooLarge)
 
 	if line.RateLimitPercent != nil {
 		t.Errorf("limit percentage %d, want none", *line.RateLimitPercent)
 	}
+
+	// The client is under no ban, and GeoJS places it in a country.
+	wantDisposition(t, s.ask(fromDE, "/"), http.StatusOK, waf.DispositionAllow)
 }
 
 func TestAClientGeoJSPlacedInNoCountryCountsAsUnknownWithoutWaiting(t *testing.T) {
 	t.Parallel()
 
-	// The client's next request takes it over the whole limit, so it does
-	// not wait.
 	geojsURL, _ := startGeoJS(t)
-	s := startWAF(t, geojsURL, map[string]string{
+	s := startWAF(t, geojsURL, map[string]string{unknownLimitPercent: "0"})
-		unknownLimitPercent: "0", rateLimitPerMinute: "1",
-	})
 	s.server.GeoJS.Load([]lookup.Answer{{
 		Client: netip.MustParsePrefix(client + "/32"), ASN: "AS64512",
 		Answered: s.clock.Now(), Used: s.clock.Now(),
 	}})
-	countClient(t, s.server, s.clock.Now())
 
-	wantDisposition(t, s.ask(client, "/"), http.StatusForbidden, waf.DispositionDeny)
+	req := s.question(siteA, client, "/")
+	req.Header.Set(cookieHeader, cookies(cookieLimit+1))
 
-	line := s.out.requestLine(t)
+	wantDisposition(t, do(t, req), http.StatusForbidden, waf.DispositionDeny)
-	wantLine(t, line, http.StatusForbidden, requestlog.ActionClientIsRateLimited)
+	wantLine(t, s.out.requestLine(t), http.StatusForbidden,
-
+		requestlog.ActionClientIsRateLimited)
-	if line.RateLimitPercent == nil || *line.RateLimitPercent != 0 {
-		t.Errorf("limit percentage %v, want 0", line.RateLimitPercent)
-	}
 }
 
-func TestObserveModeAsksGeoJSNothingForARequestOverTheHeaderLimits(t *testing.T) {
+func TestObserveModeWaitsForGeoJSForARequestOverTheHeaderLimits(t *testing.T) {
 	t.Parallel()
 
-	geojsURL, asked := startGeoJS(t)
+	geojsURL, _ := startGeoJS(t)
 	s := startWAF(t, geojsURL, map[string]string{
 		mode: observe, addLookupHeaders: on, lookupTimeout: "1h",
 	})
@@ -420,9 +312,10 @@ func TestObserveModeAsksGeoJSNothingForARequestOverTheHeaderLimits(t *testing.T)
 	got := do(t, req)
 	wantDisposition(t, got, http.StatusOK, waf.DispositionAllow)
 
-	if len(asked()) != 0 || got.header.Get("X-Client-Asn") != "" {
+	asn, country := got.header.Get("X-Client-Asn"), got.header.Get("X-Client-Country")
-		t.Errorf("GeoJS was asked about %v, and the answer gave the AS number %q, "+
+	if asn != asnDE || country != "DE" {
-			"want neither", asked(), got.header.Get("X-Client-Asn"))
+		t.Errorf("answered with the AS number %q and the country %q, want %q and DE",
+			asn, country, asnDE)
 	}
 }
 

internal/waf/headerlimits.go

@@ -10,23 +10,22 @@ import (
 	"sneak.berlin/go/smallwebwaf/internal/requestlog"
 )
 
-// The header limits: the most cookies and header fields a request may have.
+// The header limits: the most cookies and header fields a request may have
-// Real clients never reach them.
+// when the checks reach the Core Rule Set. Real clients never reach them.
 const (
 	maxCookies      = 50
 	maxHeaderFields = 100
 )
 
-// headerLimits refuses a request with more than maxHeaderFields header fields
+// headerLimits refuses a request over the header limits, with 431. Its figures
-// or more than maxCookies cookies, with 431. Its figures are the request's
+// are the request's header fields and cookies, and the limit of each.
-// header fields and cookies, and the limit of each.
 type headerLimits struct{}
 
 func (headerLimits) name() string { return "header_limits" }
 
 func (headerLimits) check(_ context.Context, rq *request, _ time.Time) *verdict {
 	fields, cookies := rq.headerCounts()
-	if fields <= maxHeaderFields && cookies <= maxCookies {
+	if !overHeaderLimits(fields, cookies) {
 		return nil
 	}
 
@@ -39,6 +38,13 @@ func (headerLimits) check(_ context.Context, rq *request, _ time.Time) *verdict
 	}
 }
 
+// overHeaderLimits reports whether a request with these counts of header fields
+// and cookies, as headerCounts counts them, is over the header limits: it has
+// more than maxHeaderFields fields or more than maxCookies cookies.
+func overHeaderLimits(fields, cookies int) bool {
+	return fields > maxHeaderFields || cookies > maxCookies
+}
+
 // headerCounts returns the request's header fields, each value of a header sent
 // twice being one, and its cookies, each part of a Cookie header between
 // semicolons being one unless it is empty or only spaces and tabs. The Core

internal/waf/heuristics.go

@@ -41,11 +41,8 @@ const (
 
 // beforeLookup are the heuristics that run before the client's AS number and
 // country are looked up, so that a client they refuse is not looked up.
-// header_limits comes first: it only counts, and a request it refuses causes no
-// other work.
 func beforeLookup() []heuristic {
 	return []heuristic{
-		headerLimits{},
 		tagLists{},
 		routeWatchTags{},
 		denyNets{},
@@ -67,6 +64,7 @@ func afterLookup() []heuristic {
 		rateLimits{},
 		trapPaths{},
 		ruleFiles{},
+		headerLimits{},
 		coreRuleSet{},
 		tagDeny{},
 	}

internal/waf/waf.go

@@ -143,9 +143,10 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 // refusedWhateverTheAnswer reports whether the request is one that does not wait
 // for GeoJS, since the checks will refuse it at now whatever GeoJS answers about
-// its client: the heuristic crowdsec, blocklists or rate_limits will refuse it,
+// its client: the heuristic crowdsec, header_limits, blocklists or rate_limits
-// as a function in that heuristic's file says. In observe mode, which refuses
+// will refuse it, as a function in that heuristic's file says. In observe mode,
-// nothing, none is. rproxy lets a request through when waf answers too late.
+// which refuses nothing, none is. rproxy lets a request through when
+// waf answers too late.
 func (rq *request) refusedWhateverTheAnswer(now time.Time) bool {
 	if rq.h.config.Observe {
 		return false
@@ -153,7 +154,8 @@ func (rq *request) refusedWhateverTheAnswer(now time.Time) bool {
 
 	_, decided := rq.crowdSecDecisionInForce(now)
 
-	return decided || rq.refusedByBlocklists(rq.h.lists.ListedBy(rq.client)) ||
+	return decided || overHeaderLimits(rq.headerCounts()) ||
+		rq.refusedByBlocklists(rq.h.lists.ListedBy(rq.client)) ||
 		rq.overAWholeRateLimit(now)
 }
 
@@ -214,7 +216,7 @@ func (h *handler) answerQuestion(w http.ResponseWriter, r *http.Request) {
 // readQuestion returns the visitor's address and the body a question carries,
 // and whether waf takes the question. It answers a question it does not
 // take with 400 and no disposition. rproxy passes that 400 to the visitor,
-// and SWRP_DEFAULT_DISPOSITION does not apply. The id of a question waf takes
+// and its default disposition does not apply. The id of a question it takes
 // is held from here on, without a decision yet, so that another question with
 // that id is refused while this one is being handled.
 //