Implement normalised http routes - #4101
Conversation
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. If you need to update snapshots, please refer to CONTRIBUTING.md |
|
0cc69dd to
1fa5485
Compare
Benchmarks [ tracer ]Benchmark execution time: 2026-09-02 12:43:28 Comparing candidate commit 82db9a3 in PR branch Found 5 performance improvements and 43 performance regressions! Performance is the same for 145 metrics, 1 unstable metrics.
|
a2be41b to
7b7f971
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b7f9710a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Do you have a rough estimate on the overhead of the route normalization? It seems expensive. Laminas is obviously just slow because it does runtime route matching, but other stacks like Slim or Symfony rely on precompiled routes. |
ff7f8fe to
8632abe
Compare
|
Approving from the stance of IDM, due to the new field being introduced |
b57a9e9 to
2ab71bd
Compare
34e4e8b to
f42c408
Compare
cataphract
left a comment
There was a problem hiding this comment.
It seems there are many gaps in framework support. I asked AI for failing counterexamples against the spec and got many:
I already removed a few that were obviously invalid (like, changing the routing type at runtime and complained about cache staleness), but the rest seem mostly valid
5d1199f to
d5e909b
Compare
|
@cataphract pr is ready back to you with all your comments addressed |
bdadbff to
eddfd4f
Compare
Add missing Tag::APPSEC_NORMALIZED_ROUTE assertions to: - Symfony TraceSearchConfigTest (V4_4, V5_0, V5_1, V5_2, V6_2) - Laravel TraceSearchConfigTest (V4, V5_7, V5_8, V8_x) - Laravel V8_x RouteCachingTest and InternalExceptionsTest - Laravel Octane CommonScenariosTest - Yii ParameterizedRouteTest, ModuleTest, LazyLoadingIntegrationsFromYiiTest - CodeIgniter ExitTest and NoCI_ControllertTest - Fix UserAvailableConstantsTest tag ordering (APPSEC_NORMALIZED_ROUTE must appear after HTTP_ROUTE to match Tag.php declaration order) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…coding, regex constraints - expandBracketOptionals: use strpos > 0 (not != false) so optional sections whose text coincidentally appears in the mandatory route prefix are not falsely detected as present; add case-insensitive fallback for percent-encoded param values so %c3%a9 (lowercase hex from browsers) matches %C3%A9 (rawurlencode output) - normalizeFromLaminas: accept pre-computed $urlMatchedParams to skip inferSymfonyRouteParams when the caller has better information - LaminasIntegration cache key: use '/'+value prefix check at position > 0 so params whose default value equals the mandatory route text are not treated as present; include static-only optional sections ([/draft]) in the key so absent and present shapes get distinct entries; for Regex routes, extract the actual route regex via reflection and run it against the URL to get accurate named captures, avoiding inferSymfonyRouteParams which ignores Laminas constraints - SymfonyIntegration: use $route->compile()->getRegex() when available to determine URL-matched params instead of generic URL inference, so routes with requirements (e.g. format=html|json with a default) correctly exclude defaulted params from the normalized route Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4cbeed5 to
82db9a3
Compare
Description
Add
_dd.appsec.normalized_routetag to HTTP framework integrations (Laravel, Slim, Symfony, Laminas, CakePHP, Yii, CodeIgniter, WordPress) per RFC-1103. The tag exposes a normalized form of the matched route, stripping concrete parameter values and framework-specific syntax (regexconstraints, optional markers) into a canonical{param}notation suitable for security analysis.Reviewer checklist