Repository navigation
Report a missing or blank Command Id or Name in JSON as a JsonException [patch] - #228
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…on [patch]
Command's [JsonConstructor] was its public constructor, so deserializing
{"Name":"Save"} or {"Id":" ","Name":"Save"} threw ArgumentNullException or
ArgumentException. Chord, Phrase and Profile report malformed JSON as a
JsonException through JsonArguments; Command now does too, through a private
[JsonConstructor] that checks Id and Name with JsonArguments.RequireText and
then delegates to the public constructor's normalization.
Fixes #226
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW3JF2xHwwm6qrw3Bh7
|
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
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 #226
What was wrong
Command's[JsonConstructor]was its public constructor. That constructor callsEnsure.NotNulland throwsArgumentExceptionfor blank values. SoJsonSerializer.Deserialize<Command>threwArgumentNullException/ArgumentExceptionfor{"Name":"Save"},{"Id":" ","Name":"Save"},{"Id":"file.save"}and{"Id":"file.save","Name":""}. A host handling bad input withcatch (JsonException)would crash on them.Chord,PhraseandProfilealready report malformed JSON as aJsonExceptionthroughJsonArguments.Change
[JsonConstructor]moves off the public constructor to a new private one. The private constructor checksIdandNamewithJsonArguments.RequireText, then delegates to the public constructor, so trimming and normalization stay the same.namebeforeid. That is the only way to make its signature differ from the public one while keeping the property types the serializer binds by. System.Text.Json binds parameters by name, so the order doesn't matter to it. A comment says so.ArgumentException, as documented.Tests
CommandJsonValidationTests:nullId/Name, each expectJsonException.ArgumentException.Command.cschange reverted, all 6JsonExceptioncases fail. With it, the full suite passes: 389/389 on Linux / .NET 10.Keybinding.slnbuilds with 0 warnings.Generated by Claude Code