Repository navigation
Keep rejected identifier values out of validation messages [patch] - #425
Merged
Merged
Conversation
A failed SemanticString creation quoted the whole canonical value in its ArgumentException, so a mistyped card number, a tampered JWT or an IBAN went straight into logs. The message also dropped the reason each validation attribute gives. Add a protected internal IncludeValueInErrors hook, set to false on CreditCardNumber, JwtToken and Iban, and append the errors of every rejecting attribute to the message. Fixes #333 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01QbGcYckSzgr6jPyoNYhWvA
|
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
2 participants
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.
Fixes #333
What changed
SemanticString.PerformValidationthrewCannot convert "<canonical value>" to T. That put a rejected card number (PAN), JWT or IBAN into logs in full, and it dropped the reason the validation attribute gave.protected internal virtual bool IncludeValueInErrors => trueonSemanticString<T>.CreditCardNumber,JwtTokenandIbanoverride it tofalse, so their message readsCannot convert the value to CreditCardNumber: ….AttributeValidation.GetValidationErrorscollects the errors of every attribute that rejects the value: the failing attributes under[ValidateAll], and the union of all of them under[ValidateAny]. A native attribute contributes itsValidationResult.Errors. An attribute that reports no reason is named instead. Example:Cannot convert the value to CreditCardNumber: The value must be a valid credit card number (Luhn check failed).ErrorMessages_ContainTypeInformationtest still passes.Tests
Semantics.Test/Identifiers/ValidationMessageTests.cs:CreditCardNumber,JwtToken,Iban: the message omits the canonical input and contains the reason (Luhn,JWT header,mod-97)Uuid(not sensitive): the message keeps the value and adds the reasonWith the library change stashed, all four tests fail. With it applied, they pass. The full
Semantics.Testsuite passes on net10.0 (1645 passed, 0 failed).Semantics.Stringsbuilds for every target framework, netstandard2.0 included.🤖 Generated with Claude Code
https://claude.ai/code/session_01QbGcYckSzgr6jPyoNYhWvA
Generated by Claude Code