Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| ->validate() | ||
| ->always(static function (array $value): array { | ||
| if (!\array_key_exists('mode', $value) && (\array_key_exists('request', $value) || \array_key_exists('response', $value))) { | ||
| unset($value['terms']); | ||
| } | ||
|
|
||
| return $value; | ||
| }) | ||
| ->end() |
There was a problem hiding this comment.
Bug: The validation for http_headers allows specifying both a top-level mode and nested request/response configurations, creating an ambiguous state that is not handled.
Severity: MEDIUM
Suggested Fix
Add validation to throw an InvalidConfigurationException if both a top-level mode and nested request or response keys are detected within the http_headers configuration. This will force the user to provide an unambiguous configuration. Additionally, add a test case to verify that this invalid mixed configuration is correctly rejected.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/DependencyInjection/Configuration.php#L179-L187
Potential issue: The validation logic for the `http_headers` configuration in
`Configuration.php` does not prevent a user from specifying both a top-level `mode` and
nested `request`/`response` configurations. The validation callback only handles the
case where `mode` is absent but `request` or `response` are present. It does not account
for the ambiguous scenario where both are defined. This mixed configuration is passed to
the base SDK's `DataCollectionOptions` class, which likely expects one structure or the
other, not both. This could result in incorrect header filtering or a runtime error
within the SDK. This scenario is not covered by any tests.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9ef4933. Configure here.
|
|
||
| return $value; | ||
| }) | ||
| ->end() |
There was a problem hiding this comment.
Header terms dropped without mode
Low Severity
The http_headers finalize step always unsets terms when request or response is present and mode is absent. That also discards terms the user actually configured, not just the empty terms array Symfony injects. Those filters never reach the SDK, so header collection can include values the config tried to restrict.
Reviewed by Cursor Bugbot for commit 9ef4933. Configure here.
1 participant
Adds the configuration for the new data collection. The changes are currently linked against the unreleased changes in the base PHP SDK