Repository navigation
Refuse a null non-nullable property in the in-memory store too [patch] - #212
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
InMemoryCredentialStore.Save and CredentialCache.AddOrReplace only checked for an unregistered type, so a credential such as `Token = null!` saved fine in memory and TryGet handed it back, while every platform store throws ArgumentException for it. Both now run the same serialization the platform stores do (zeroing the bytes) before storing anything. Fixes #208 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW4nQyg9qMFhK6fYVqQ
|
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 #208
What changed
InMemoryCredentialStore.SaveandCredentialCache.AddOrReplaceonly checked for an unregistered type. A credential likenew CredentialWithToken { Token = null! }therefore saved fine in memory, andTryGethanded it back. Every platform store throwsArgumentExceptionfor that same credential since #204.CredentialSerialization.ThrowIfNotSerializable(credential)runsSerializeand zeroes the bytes it gets back. It throws whatever a platform store's save would throw:CredentialStoreExceptionfor an unknown type,ArgumentExceptionfor a null non-nullable property.AddOrReplacecalls it in place ofThrowIfUnknownType, so no store sees the credential before the check.InMemoryCredentialStore.Savecalls it too, which covers direct calls to the store.ArgumentExceptiondoc onAddOrReplace: the exception is now thrown whichever store is configured.Tests
AddOrReplaceRejectsANullNonNullablePropertyAndLeavesTheStoreAndCacheUnchangednow runs against both the serializing stub andInMemoryCredentialStore.InMemoryStoreSaveRejectsANullNonNullablePropertyAsAPlatformStoreWould, plus a check that a complete credential still saves and round-trips.Generated by Claude Code