Add scope filter to the typed throttle API - #124
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a first-class scope filter to the typed throttle configuration API, allowing throttles to only apply/count for requests that match a predicate while preserving the default “keyless = client IP” behavior (including the configured IP resolver).
Changes:
- Extend
ThrottleSection::{add, sliding, multi}with an optionalscopeclosure, wrapped as aClosureRequestMatcherand passed intoThrottleRule. - Add a focused PHPUnit suite validating scoped throttling behavior (including interaction with default IP-keying, explicit keys, sliding, and multi-window throttles).
- Update README + example script + changelog to document and demonstrate the new
scopeusage (replacing the old “null-returning key closure” pattern).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/ThrottleScopeTest.php | Adds unit tests covering scope semantics across throttle modes and keying behavior. |
| src/Config/Section/ThrottleSection.php | Extends typed throttle API to accept scope and forwards it into ThrottleRule. |
| README.md | Documents scoped throttling and updates login-throttle example to use scope + keyless IP behavior. |
| examples/03-api-rate-limiting.php | Refactors endpoint/method-specific throttles to use scope instead of null-returning key closures. |
| CHANGELOG.md | Adds an Unreleased entry describing the new scope parameter on the typed throttle API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b957dc9 to
ba886cf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CHANGELOG.md:13
- The Unreleased section is now under "### Added", but it also contains a security-related fix (PCRE engine error handling). For consistency with the rest of this changelog (and Keep a Changelog), split this into an "Added" section for the new
scopeAPI and a separate "Security" section for the regex failure-policy change.
### Added
- **`scope` filter on `throttles->add()`, `sliding()` and `multi()`.** An optional closure restricting which requests the throttle counts; non-matching requests skip the rule. Combined with an omitted `$key` the rule counts per client IP through the Config IP resolver, replacing the null-returning key closure that bypasses the resolver. `multi()` applies the scope to every window sub-rule. Exposes the `scope` already supported by `ThrottleRule` and `PortableConfig::throttle()` on the typed section API.
- **Pattern blocklist regex matching honours the failure policy on a PCRE engine error.** A compile-valid pattern that errors at match time (e.g. the backtrack limit exceeded) previously counted as "no match" unconditionally, letting an attacker who could force the error slip past a block rule. `SnapshotBlocklistMatcher` now follows `Config::setFailOpen()`: under the fail-open default an engine error still counts as no match (a broken pattern must not block legitimate traffic), while a fail-closed firewall (`setFailOpen(false)`) treats the error as a match at the `PATH_REGEX`, `REQUEST_REGEX`, and `HEADER_REGEX` sites via the new `RegexMatcher::matchesFailClosed()`. The policy reaches the matcher through the new `Matchers\FailOpenAware` capability, injected by the `Firewall` at construction like `CompiledDataCacheAware`. Generic regex filters (the `PortableConfig` filter factories, also used by safelists) are unchanged: there an engine error must never count as a match, since a filter match can safelist a request.
0956890 to
40a3615
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CHANGELOG.md:18
- The Unreleased section has a duplicated "### Security" heading and repeats the same security bullet twice (once above "### Added" and again below). This will confuse changelog readers and makes it look like the security fix is two separate changes.
### Added
- **`scope` filter on `throttles->add()`, `sliding()` and `multi()`.** An optional closure restricting which requests the throttle counts; non-matching requests skip the rule. Combined with an omitted `$key` the rule counts per client IP through the Config IP resolver, replacing the null-returning key closure that bypasses the resolver. `multi()` applies the scope to every window sub-rule. Exposes the `scope` already supported by `ThrottleRule` and `PortableConfig::throttle()` on the typed section API.
### Security
- **Pattern blocklist regex matching honours the failure policy on a PCRE engine error.** A compile-valid pattern that errors at match time (e.g. the backtrack limit exceeded) previously counted as "no match" unconditionally, letting an attacker who could force the error slip past a block rule. `SnapshotBlocklistMatcher` now follows `Config::setFailOpen()`: under the fail-open default an engine error still counts as no match (a broken pattern must not block legitimate traffic), while a fail-closed firewall (`setFailOpen(false)`) treats the error as a match at the `PATH_REGEX`, `REQUEST_REGEX`, and `HEADER_REGEX` sites via the new `RegexMatcher::matchesFailClosed()`. The policy reaches the matcher through the new `Matchers\FailOpenAware` capability, injected by the `Firewall` at construction like `CompiledDataCacheAware`. Generic regex filters (the `PortableConfig` filter factories, also used by safelists) are unchanged: there an engine error must never count as a match, since a filter match can safelist a request.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CHANGELOG.md:18
- Under "Unreleased" the "### Security" section and its bullet are duplicated (one before "### Added" and the same one again after). This will publish the same security note twice and breaks the changelog structure; keep a single Security section and remove the duplicate header+entry.
### Security
- **Pattern blocklist regex matching honours the failure policy on a PCRE engine error.** A compile-valid pattern that errors at match time (e.g. the backtrack limit exceeded) previously counted as "no match" unconditionally, letting an attacker who could force the error slip past a block rule. `SnapshotBlocklistMatcher` now follows `Config::setFailOpen()`: under the fail-open default an engine error still counts as no match (a broken pattern must not block legitimate traffic), while a fail-closed firewall (`setFailOpen(false)`) treats the error as a match at the `PATH_REGEX`, `REQUEST_REGEX`, and `HEADER_REGEX` sites via the new `RegexMatcher::matchesFailClosed()`. The policy reaches the matcher through the new `Matchers\FailOpenAware` capability, injected by the `Firewall` at construction like `CompiledDataCacheAware`. Generic regex filters (the `PortableConfig` filter factories, also used by safelists) are unchanged: there an engine error must never count as a match, since a filter match can safelist a request.
No description provided.