Skip to content

Conversation

@h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Jan 15, 2025

Description

Sending this in now when #8215 was finally merged as I've discovered this during development of #9697 but wanted to wait until I can fix the TODOs in the unit tests as well.

Fixes InvalidCastException thrown from CanConvertTo when unboxing cast is performed on value that was not null but was of a different type than Key or the underlying storage (int), e.g. an object or a different value type.
Can* methods should not (and converters do not intentionally) throw in them but this is simple a programmer's error, it is not documented behavior either, hence the fix for this.

https://learn.microsoft.com/en-us/dotnet/api/system.windows.input.keyconverter.canconvertto?view=windowsdesktop-8.0

Fixes the same issue in ConvertTo by throwing the proper (NotSupported) exception.

https://learn.microsoft.com/en-us/dotnet/api/system.windows.input.keyconverter.convertto?view=windowsdesktop-8.0

Customer Impact

Less unexpected exceptions thrown.

Regression

No.

Testing

Local build, unit test.

Risk

There might be a concern that a code was passing int instead of Key which is the underlying storage type, so it survives the unboxing hard cast that was being performed but not the type is check. We may therefore for back-compat include check for both Key and int but other converters just check for their type, so in my honest opinion it would be best to unify the behaviour and mention in release notes.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners January 15, 2025 20:12
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant