Repository navigation
feat: async pipeline, reverse proxy support and entry limit (2.3.0) - #6
Merged
Merged
Conversation
Addresses three points from the latest feedback round. 1. Thread.Sleep under load Thread.Sleep blocked an OS thread on the MVC filter (sync and async path) and the Razor Pages filter blocked via GetAwaiter().GetResult(), so a bot wave could starve the Kestrel thread pool. The IDistributedCache calls were synchronous on top of that, blocking a thread per Redis round trip. All delays now use await Task.Delay(...) and the service gained CheckRequestAsync / ReleaseRequestAsync / AutoProcessRequestReleaseAsync using GetAsync/SetAsync/RemoveAsync. The new interface members have default implementations delegating to the sync ones, so existing custom INoBrute implementations keep working. The sync filter methods are now empty: ASP.NET Core always prefers the async filter interfaces, so they were already dead code in the pipeline. 2. Client IP behind Cloudflare / reverse proxies New INoBruteClientIpResolver configured via NoBrute:ClientIp with header list (CF-Connecting-IP, X-Forwarded-For), ForwardLimit, KnownProxies and KnownNetworks (CIDR). Disabled by default on purpose: trusting these headers on a directly reachable app would let a bot send a random client IP per request and bypass the protection entirely. 3. Cache flooding New INoBruteEntryLimiter with NoBrute:MaxTrackedEntries. Once the limit is reached, known clients are served normally while new ones are answered with NoBrute:BlockedStatusCode (429) without allocating another cache entry. Cache entries were written without any expiration before, so every IP ever seen stayed in RAM/Redis forever - they now expire after TimeUntilReset, and releasing a client's last request deletes the entry. Also fixes TimeUntilResetUnit, which was parsed by enum member name and therefore never matched: every configured unit silently fell back to hours. Tests: 46 -> 109. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses three points from the latest feedback round.
All delays now use await Task.Delay(...) and the service gained CheckRequestAsync / ReleaseRequestAsync / AutoProcessRequestReleaseAsync using GetAsync/SetAsync/RemoveAsync. The new interface members have default implementations delegating to the sync ones, so existing custom INoBrute implementations keep working. The sync filter methods are now empty: ASP.NET Core always prefers the async filter interfaces, so they were already dead code in the pipeline.
Client IP behind Cloudflare / reverse proxies New INoBruteClientIpResolver configured via NoBrute:ClientIp with header list (CF-Connecting-IP, X-Forwarded-For), ForwardLimit, KnownProxies and KnownNetworks (CIDR). Disabled by default on purpose: trusting these headers on a directly reachable app would let a bot send a random client IP per request and bypass the protection entirely.
Cache flooding New INoBruteEntryLimiter with NoBrute:MaxTrackedEntries. Once the limit is reached, known clients are served normally while new ones are answered with NoBrute:BlockedStatusCode (429) without allocating another cache entry. Cache entries were written without any expiration before, so every IP ever seen stayed in RAM/Redis forever - they now expire after TimeUntilReset, and releasing a client's last request deletes the entry.
Also fixes TimeUntilResetUnit, which was parsed by enum member name and therefore never matched: every configured unit silently fell back to hours.
Tests: 46 -> 109.