[windows] Fix null-deref in HostWindowPopup::UpdatePosition (Fixes #191478) - #191479
[windows] Fix null-deref in HostWindowPopup::UpdatePosition (Fixes #191478)#191479kryali wants to merge 1 commit into
Conversation
Guard against a null WindowRect* from the position callback, matching HostWindowTooltip::UpdatePosition's existing check for the same call. Fixes a reproducible access-violation crash when the callback returns null. Includes a regression test.
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a null check for the window position rectangle in HostWindowPopup::UpdatePosition to prevent crashes when the positioner fails, and adds a corresponding unit test to verify this behavior. The review feedback suggests initializing the RECT structures and asserting the success of the GetWindowRect Win32 API calls in the new unit test to avoid using uninitialized memory and prevent test flakiness.
| RECT initial_rect; | ||
| GetWindowRect(popup_window_handle, &initial_rect); |
There was a problem hiding this comment.
The GetWindowRect Win32 API call can fail, which would leave initial_rect uninitialized. To prevent using uninitialized memory and avoid potential test flakiness, initialize the RECT structure and assert that the API call succeeds.
| RECT initial_rect; | |
| GetWindowRect(popup_window_handle, &initial_rect); | |
| RECT initial_rect = {}; | |
| ASSERT_TRUE(GetWindowRect(popup_window_handle, &initial_rect)); |
| RECT rect_after_null_callback; | ||
| GetWindowRect(popup_window_handle, &rect_after_null_callback); |
There was a problem hiding this comment.
Similarly, initialize rect_after_null_callback and assert that GetWindowRect succeeds to ensure the test does not compare uninitialized memory if the API call fails.
| RECT rect_after_null_callback; | |
| GetWindowRect(popup_window_handle, &rect_after_null_callback); | |
| RECT rect_after_null_callback = {}; | |
| ASSERT_TRUE(GetWindowRect(popup_window_handle, &rect_after_null_callback)); |
1 participant
Hi there,
I recently upgraded from
flutter 3.44.9toflutter 3.47.0and noticed that the upgrade introduced a new crash on windows. This surfaced as a EXCEPTION_ACCESS_VIOLATION_READ crash in my application.I was able to trace the issue to
HostWindowPopupand have proposed a fix with this change. This fix is modeled after the existing code in host window tooltip.Fixes Issue #191478
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.