-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PropertyGridView: Prevent out-of-range access when committing next enumerable value #14182
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
base: main
Are you sure you want to change the base?
PropertyGridView: Prevent out-of-range access when committing next enumerable value #14182
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14182 +/- ##
===================================================
- Coverage 77.15877% 77.14364% -0.01513%
===================================================
Files 3279 3279
Lines 645333 645339 +6
Branches 47720 47722 +2
===================================================
- Hits 497931 497838 -93
- Misses 143719 143813 +94
- Partials 3683 3688 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes IndexOutOfRangeException crashes in PropertyGridView when users double-click or scroll with the mouse wheel on enumerable properties whose StandardValues list becomes empty between index calculation and value commit. The fix adds defensive checks to ensure the values array is not empty before attempting to access it by index.
Key Changes:
- Added empty array validation in
DoubleClickRowmethod before cycling through enumerable values - Added empty array validation in
OnMouseWheelmethod before cycling through enumerable values with mouse wheel
...ws.Forms/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/PropertyGridView.cs
Show resolved
Hide resolved
...ws.Forms/System/Windows/Forms/Controls/PropertyGrid/PropertyGridInternal/PropertyGridView.cs
Show resolved
Hide resolved
ricardobossan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All LGTM!
Fixes #14169, #14170
Root cause:
The PropertyGrid assumes that the StandardValues list is stable and non‑empty once the current index is computed.
However, during both double‑click value cycling and mouse‑wheel value cycling, the control calls
GetPropertyValueList()a second time when committing the next value.In certain scenarios (dynamic TypeConverters, context‑dependent StandardValues, or volatile data sources), this second call may return an empty array, making the previously computed index invalid.
This leads to an out‑of‑range access when executing:
CommitValue(values[index])Proposed changes
DoubleClickRowandOnMouseWheel: After callinggridEntry.GetPropertyValueList(), checkvalues.Length > 0before trying to accessvalues[index].Customer Impact
Regression?
Risk
Screenshots
WinFormsApp18.zip
Before
Double‑clicking or scroll the mouse wheel in the attribute value editing box,
GetPropertyValueList()may return an empty arrayBeforeChanges.mp4
After
Logic re‑verifies the StandardValues list before using it.
If
values.Length == 0, cycling is safely skipped.No exceptions are thrown
AfterChanges.mp4
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow