Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix InvalidCastException in Can/ConvertTo from MouseActionConverter #10381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Jan 31, 2025

Mirrors #10289

Description

Sending this in now when we merged and discussed #9891 as I've discovered this bug during development of #9676 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 MouseAction or the underlying storage (byte), e.g. an object or a different value type (int).
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.mouseactionconverter.canconvertto?view=windowsdesktop-9.0

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

https://learn.microsoft.com/en-us/dotnet/api/system.windows.input.mouseactionconverter.convertto?view=windowsdesktop-9.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 byte instead of MouseAction 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 MouseAction and byte but other converters (besides Key since it shares the bug) just check for their type, so in my honest opinion it would be best to unify the behaviour and mention in release notes.

Not to mention, unlike Key where the backing storage is int, here its byte, even lesser chance for such occurence.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners January 31, 2025 18:44
@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 31, 2025
@dipeshmsft dipeshmsft assigned dipeshmsft and unassigned dipeshmsft Feb 2, 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.

2 participants