Name request log actions so they say what they mean (closes #229)
check / check (push) Successful in 6m30s
check / check (push) Successful in 6m30s
The request log's `action` values now say what they mean, as sneak named them: `request_header_too_large`, `client_is_rate_limited`, `blocked_by_block_rule` and `client_denied`, with no aliases. `banned` splits into `temporarily_banned` and `permanently_banned`, by whether the ban runs out. When a permanent ban also covers a client, that ban is the one reported. Trips a reader: a request over a rate limit stays `client_is_rate_limited` even when the ban it makes is permanent. Disclosure: the offence key `rule_blocked` is renamed in `clients.json` too, so an existing file holding it stops the start and must be removed. Its `version` stays 1. Model: opus-5-5
This commit was merged in pull request #233.
This commit is contained in:
1 parent
6850cb0e9f
commit
aede434f6a
48 files changed
+522
-300
No files matched your search
@@ -59,7 +59,7 @@ func TestRateLimitRefusesBeforeTheApp(t *testing.T) {
|
||||
if sent.want == http.StatusOK {
|
||||
wantLine(t, line, http.StatusOK, requestlog.ActionForward)
|
||||
} else {
|
||||
wantLine(t, line, http.StatusForbidden, requestlog.ActionRateLimited)
|
||||
wantLine(t, line, http.StatusForbidden, requestlog.ActionClientIsRateLimited)
|
||||
|
||||
if line.LimitHit != minute {
|
||||
t.Errorf("log line has limit_hit %q, want minute", line.LimitHit)
|
||||
@@ -79,7 +79,7 @@ func TestRateLimitRefusalNamesItsHeuristicWindowCountAndLimit(t *testing.T) {
|
||||
|
||||
s.get(client, http.StatusOK, requestlog.ActionForward)
|
||||
|
||||
line := s.get(client, http.StatusForbidden, requestlog.ActionRateLimited)
|
||||
line := s.get(client, http.StatusForbidden, requestlog.ActionClientIsRateLimited)
|
||||
wantHeuristic(t, line, "rate_limits", `{"window": "minute", "count": 2, "limit": 1}`)
|
||||
}
|
||||
|
||||
@@ -102,7 +102,7 @@ func TestIPv6GroupPrefixSetsTheClientTheLimitsCount(t *testing.T) {
|
||||
status int
|
||||
action string
|
||||
}{
|
||||
{rateLimitPerMinute, "1", http.StatusForbidden, requestlog.ActionRateLimited},
|
||||
{rateLimitPerMinute, "1", http.StatusForbidden, requestlog.ActionClientIsRateLimited},
|
||||
{bytesLimitPerMinute, byteLimit, http.StatusOK, requestlog.ActionForward},
|
||||
} {
|
||||
t.Run(tc.setting, func(t *testing.T) {
|
||||
@@ -160,14 +160,17 @@ func TestRateLimitExemptPathsAreNeitherCountedNorRefused(t *testing.T) {
|
||||
|
||||
// A path outside every prefix is counted: /assets is not under
|
||||
// /assets/, and breaks the limit.
|
||||
s.request(client, "/assets", http.StatusForbidden, requestlog.ActionRateLimited)
|
||||
s.request(client, "/assets", http.StatusForbidden,
|
||||
requestlog.ActionClientIsRateLimited)
|
||||
|
||||
// A ban, SWW_DENY_NETS and SWW_TAG_DENY still refuse a path under a
|
||||
// prefix.
|
||||
s.request(client, "/assets/app.js", http.StatusForbidden, requestlog.ActionBanned)
|
||||
s.request(denied, "/assets/app.js", http.StatusForbidden, requestlog.ActionDenied)
|
||||
s.request(client, "/assets/app.js", http.StatusForbidden,
|
||||
requestlog.ActionTemporarilyBanned)
|
||||
s.request(denied, "/assets/app.js", http.StatusForbidden,
|
||||
requestlog.ActionClientDenied)
|
||||
s.request(fromKP, "/assets/app.js",
|
||||
http.StatusForbidden, requestlog.ActionDenied)
|
||||
http.StatusForbidden, requestlog.ActionClientDenied)
|
||||
}
|
||||
|
||||
func TestRateLimitCountsPathsThatAreNotExempt(t *testing.T) {
|
||||
@@ -211,7 +214,7 @@ func TestRateLimitCountsPathsThatAreNotExempt(t *testing.T) {
|
||||
// Counted, the second request breaks the limit of one request
|
||||
// a minute.
|
||||
s.request(client, sent, http.StatusOK, requestlog.ActionForward)
|
||||
s.request(client, sent, http.StatusForbidden, requestlog.ActionRateLimited)
|
||||
s.request(client, sent, http.StatusForbidden, requestlog.ActionClientIsRateLimited)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user