Repository navigation
Refuse a lone surrogate when saving instead of storing U+FFFD [patch] - #218
Open
matt-edmondson wants to merge 2 commits into
Open
matt-edmondson wants to merge 2 commits into
matt-edmondson wants to merge 2 commits into
Conversation
Utf8JsonWriter silently writes U+FFFD for a lone surrogate, so a credential holding ill-formed UTF-16 was persisted as a different secret, while the cache kept returning the original until a restart. Serialize and SerializeToString now refuse such a value, in a plain string or a SemanticString, anywhere in the credential, with an ArgumentException that does not quote it. The in-memory store and AddOrReplace run the same check, so every store refuses it. Fixes #215 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW6R8MN1ipg19Mih4Z3
Covers the property-name paths of the well-formed text converters for string and SemanticString keys, and IsWellFormed directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW6R8MN1ipg19Mih4Z3
|
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 #215
What changed
Utf8JsonWritersilently writes U+FFFD in place of a lone surrogate. So a credential holding ill-formed UTF-16 was saved as a different secret, and the cache kept returning the original until the process restarted.WellFormedTextConverterFactory(new, internal) wrapsRoundTripStringJsonConverterFactoryand adds astringconverter. Before writing a string, aSemanticString(by its string form), or a dictionary key of either, it throwsJsonExceptionif the value has an unpaired surrogate.Serialize/SerializeToStringalready turn that into anArgumentException. The message does not quote the value, since it is usually a secret. Reading is unchanged.InMemoryCredentialStore.SaveandCredentialCache.AddOrReplacenow call a newThrowIfNotSerializablehelper (serialize, then zero the bytes), so the in-memory store refuses the same credentials as the platform stores. The cache and the store stay unchanged.Overlap with #212
#212 adds the same
ThrowIfNotSerializablehelper and the same in-memory/AddOrReplacewiring, for null non-nullable properties. The code is identical, but the XML doc text differs, so whichever PR merges second needs a small doc-only conflict fix. Once both merge, the in-memory store refuses null properties and lone surrogates.Tests
IllFormedTextTests:SerializeandSerializeToStringrefuse lone high and lone low surrogates, and a reversed pair, in aSemanticString, a plainstring, and a list of strings. The error message does not include the value.AddOrReplacerefuses the credential and stores nothing on a serializing store (RawBlobCredentialStore) and onInMemoryCredentialStore, and so doesInMemoryCredentialStore.Save.With the source changes reverted, 4 of the 5 tests fail. The fifth, the round-trip test, passes either way, as it should. The inputs are built in code, not with
[DataRow], because attribute strings are stored as UTF-8 and a lone surrogate would already be U+FFFD by the time the test ran.Full suite: 108 passed, 5 skipped (native-store and Windows-only tests), 0 failed.
Generated by Claude Code