-
Notifications
You must be signed in to change notification settings - Fork 384
Add defensive validation to tag parsing in dotnet-counters #5685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add defensive validation to tag parsing in dotnet-counters #5685
Conversation
Fixes dotnet#5683 The ConsoleWriter was not properly validating tag parsing, causing IndexOutOfRangeException when processing tags. This prevented subsequent tags from being displayed. Changes: - Split tags with max count of 2 to handle values containing '=' - Validate array length before accessing elements - Trim whitespace from tag keys - Skip malformed tags gracefully instead of crashing This ensures all tags in multi-dimensional metrics are displayed correctly in dotnet-counters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes an IndexOutOfRangeException that occurred when processing multi-dimensional metric tags containing '=' characters in their values. The fix adds robust validation and error handling to the tag parsing logic in dotnet-counters.
Changes:
- Modified tag splitting to limit to 2 parts, preventing issues when tag values contain '=' characters
- Added validation to skip malformed tags gracefully instead of crashing
- Added key trimming and empty key filtering for robust tag processing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| string key = keyValue[0].Trim(); | ||
| string value = keyValue[1]; |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider trimming the value as well to handle potential whitespace around the '=' delimiter, similar to how the key is trimmed. For example, if a tag is formatted as "key = value" with spaces around the equals sign, the value would currently preserve the leading space.
| string value = keyValue[1]; | |
| string value = keyValue[1].Trim(); |
|
@steveisok - Issue #5683 didn't use the '=' sign in the tag names that were part of the provided repro and it sounds like that is what this PR fixes. If there is a bug displaying such tags I'm glad to see us fix it, but its not clear if we are addressing 5683 or this is an unrelated bug in the same scenario. Do we know whether the original repro code that doesn't use '=' demonstrates some other problem? |
217c863 to
a6ed77d
Compare
I don't think this is a fix to #5683, but instead a defensive improvement discovered in the investigation. |
Adds a regression test that verifies malformed tags (tags without '=', whitespace-only tags, etc.) are handled gracefully without crashing. Without the defensive code fix: - Test FAILS with IndexOutOfRangeException at line 204 - Crash occurs when trying to access keyValue[1] on split result With the defensive code fix: - Test PASSES - malformed tags are skipped gracefully - Valid tags in mixed scenarios are still processed correctly This demonstrates the defensive improvements have real value in preventing crashes on malformed input.
Summary
Defensive improvements to tag parsing in dotnet-counters (Related to #5683)
While investigating issue #5683, I found that the
RenderTagSetsInColumnModemethod inConsoleWriter.cswas parsing tags without proper validation, which could lead to crashes on malformed input.Note: This PR does not fix the originally reported issue in #5683. Investigation revealed that multi-tag functionality was already working correctly (existing test
LongMultidimensionalTagsAreTruncateddemonstrates this). The originally reported issue may be environment-specific,version-specific, or related to a different code path. This PR provides defensive improvements discovered during the investigation.
Changes
=charactersIndexOutOfRangeExceptionBenefits
=, leading/trailing whitespace, etc.)Testing
Added
MalformedTagsDoNotCrashtest that demonstrates the defensive fix prevents real crashes:Scenarios tested:
=character (e.g.,"BadTagNoEquals")"InvalidTag,color=blue")Evidence:
IndexOutOfRangeExceptionat line 204 inRenderTagSetsInColumnModewhen trying to accesskeyValue[1]on a malformed tag