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 System.OverflowException in WindowChromeWorker._HandleNCHitTest #6779

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

lindexi
Copy link
Member

@lindexi lindexi commented Jul 12, 2022

Fixes #6777

Description

The lParam may be the int 64 value which will throw OverflowException when cast to int 32. We only use the int32 part inside the int64, so it's safe for us to use ToInt64 first and then cast to int32.

And I think this fix is safe. Because the HandleWmNcHitTestMsg which handle the NCHitTest will call the NativeMethods.SignedLOWORD method. The SignedLOWORD will cast to long then cast to int32.

public static int SignedLOWORD(IntPtr n) {
return SignedLOWORD( unchecked((int)(long)n) );
}

Customer Impact

See #6777

Regression

None.

Testing

Just the CI.

And I pick the code to dotnet-campus/dotnetCampus.CustomWpf#20 and I build the private version to test this by my application.

Risk

Low. It is safe for us to use IntPtr.ToInt64 and cast the long to int.

Microsoft Reviewers: Open in CodeFlow

The lParam may be the int 64 value which will throw OverflowException when cast to int 32. We only use the int32 part inside the int64, so it's safe for us to use ToInt64 first and then cast to int32.

See dotnet#6777
@lindexi lindexi requested a review from a team as a code owner July 12, 2022 06:21
@ghost ghost assigned lindexi Jul 12, 2022
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jul 12, 2022
@ghost ghost requested review from dipeshmsft, singhashish-wpf and SamBent July 12, 2022 06:22
@ghost ghost added the Community Contribution A label for all community Contributions label Jul 12, 2022
@miloush
Copy link
Contributor

miloush commented Jul 28, 2022

The comments are not very useful on their own. If you want to include any, it would be better to say what's going on, like "// avoid overflow for negative coordinates (#6777)".

Otherwise looks good to me.

@lindexi
Copy link
Member Author

lindexi commented Jul 29, 2022

Thank you @miloush and I fixed it.

@dipeshmsft dipeshmsft merged commit a56c017 into dotnet:main Dec 14, 2022
@dipeshmsft
Copy link
Member

@lindexi thank you for the contribution. @miloush thanks for your review comments.

@lindexi
Copy link
Member Author

lindexi commented Dec 14, 2022

Thank you @dipeshmsft

@ghost ghost locked as resolved and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

System.OverflowException in PresentationFramework.dll in System.Windows.Shell.WindowChromeWorker
4 participants