Pin the rate-limit key for an empty RemoteAddr (closes #168)
check / check (push) Waiting to run
check / check (push) Waiting to run
A request whose RemoteAddr is empty has no peer identity, so the rate limiters' key falls back to the raw empty string and every such request shares one bucket: it fails closed rather than giving each its own. net/http always fills RemoteAddr for a TCP listener, so normal serving never reaches this. The behaviour is unchanged and now deliberate: a test pins the shared key, and a one-sentence comment at the fallback tells the empty case apart from a Unix-socket listener, where every peer legitimately carries the same address. Model: opus-5-5
This commit was merged in pull request #448.
This commit is contained in:
@@ -219,7 +219,11 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
||||
// path cannot silently collapse unrelated clients
|
||||
// together. On a Unix-socket listener every peer
|
||||
// carries the same RemoteAddr and so shares one bucket,
|
||||
// which is the fail-closed direction.
|
||||
// which is the fail-closed direction. An empty RemoteAddr
|
||||
// is a different case, which net/http never produces for
|
||||
// a TCP listener and only a hand-built request carries,
|
||||
// but it fails closed the same way: every such request
|
||||
// shares the one bucket keyed on the empty string.
|
||||
return r.RemoteAddr
|
||||
}
|
||||
|
||||
|
||||
@@ -1012,6 +1012,23 @@ func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets(
|
||||
)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_EmptyPeerSharesOneBucket pins what the fallback
|
||||
// does with an empty RemoteAddr: it keys on the empty string, so every
|
||||
// such request shares one bucket. That is the fail-closed direction
|
||||
// and is kept on purpose; only a hand-built request carries an empty
|
||||
// RemoteAddr.
|
||||
func TestRateLimitKey_EmptyPeerSharesOneBucket(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
assert.Empty(
|
||||
t, clientKeyFor(t, m, ""),
|
||||
"every peer with an empty RemoteAddr must key on the "+
|
||||
"empty string and so share one bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// TestPostRateLimit_IPv6SharesBucketWithinSlash64 is the behavioural
|
||||
// half, and the regression test for the bypass itself: a client that
|
||||
// rotates source addresses inside its own routed /64 must stay in one
|
||||
|
||||
Reference in New Issue
Block a user