[Regression] Fixing MonthCalendar NRE when MinDate is set (#3041)

Add protection for MonthCalendarAccessibleObject against incorrect parameters of methods to avoid exception throwing

Related Issues #2912 and #2475
Related PR #2911

src/Common/src/UnsafeNativeMethods.cs

@@ -579,6 +579,9 @@ public static StringBuilder GetModuleFileNameLongPath(HandleRef hModule)
         [DllImport(ExternDll.User32, CharSet = CharSet.Auto)]
         public static extern IntPtr SendMessage(HandleRef hWnd, int msg, int wParam, NativeMethods.SYSTEMTIMEARRAY lParam);
 
+        [DllImport(ExternDll.User32, CharSet = CharSet.Auto)]
+        public static extern IntPtr SendMessage(HandleRef hWnd, int msg, int wParam, NativeMethods.NMSELCHANGE lParam);
+
         [DllImport(ExternDll.User32, CharSet = CharSet.Auto)]
         public static extern IntPtr SendMessage(HandleRef hWnd, int msg, int wParam, ref NativeMethods.LOGFONTW lParam);
 

src/System.Windows.Forms/src/System/Windows/Forms/DateTimePicker.cs

@@ -1108,9 +1108,9 @@ public event EventHandler DropDown
         }
 
         /// <summary>
-            ///  Constructs the new instance of the accessibility object for this control. Subclasses
+        ///  Constructs the new instance of the accessibility object for this control. Subclasses
         ///  should not call base.CreateAccessibilityObject.
-            /// </summary>
+        /// </summary>
         protected override AccessibleObject CreateAccessibilityInstance()
         {
             return new DateTimePickerAccessibleObject(this);
@@ -1767,6 +1767,12 @@ internal static Kernel32.SYSTEMTIME DateTimeToSysTime(DateTime time)
         /// </summary>
         internal static DateTime SysTimeToDateTime(Kernel32.SYSTEMTIME s)
         {
+            if (s.wYear <= 0 || s.wMonth <= 0 || s.wDay <= 0)
+            {
+                Debug.WriteLine("Incorrect SYSTEMTIME info!");
+                return DateTime.MinValue;
+            }
+
             return new DateTime(s.wYear, s.wMonth, s.wDay, s.wHour, s.wMinute, s.wSecond);
         }
 

src/System.Windows.Forms/src/System/Windows/Forms/MonthCalendar.CalendarBodyAccessibleObject.cs

@@ -54,6 +54,12 @@ public CalendarChildAccessibleObject GetFromPoint(ComCtl32.MCHITTESTINFO hitTest
                     case ComCtl32.MCHT.CALENDARDATE:
                         AccessibleObject rowAccessibleObject =
                             _calendarAccessibleObject.GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarRow, this, hitTestInfo.iRow);
+
+                        if (rowAccessibleObject == null)
+                        {
+                            return null;
+                        }
+
                         return _calendarAccessibleObject.GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarCell, rowAccessibleObject, hitTestInfo.iCol);
                 }
 

src/System.Windows.Forms/src/System/Windows/Forms/MonthCalendar.CalendarChildAccessibleObject.cs

@@ -15,13 +15,13 @@ public partial class MonthCalendar
         /// </summary>
         internal abstract class CalendarChildAccessibleObject : AccessibleObject
         {
-            protected MonthCalendarAccessibleObject _calendarAccessibleObject;
+            protected readonly MonthCalendarAccessibleObject _calendarAccessibleObject;
             protected int _calendarIndex;
             protected CalendarChildType _itemType;
 
             public CalendarChildAccessibleObject(MonthCalendarAccessibleObject calendarAccessibleObject, int calendarIndex, CalendarChildType itemType)
             {
-                _calendarAccessibleObject = calendarAccessibleObject;
+                _calendarAccessibleObject = calendarAccessibleObject ?? throw new ArgumentNullException(nameof(calendarAccessibleObject));
                 _calendarIndex = calendarIndex;
                 _itemType = itemType;
             }
@@ -64,7 +64,7 @@ public void RaiseMouseClick()
                     return;
                 }
 
-                var rectangle = CalculateBoundingRectangle();
+                RECT rectangle = CalculateBoundingRectangle();
                 int x = rectangle.left + ((rectangle.right - rectangle.left) / 2);
                 int y = rectangle.top + ((rectangle.bottom - rectangle.top) / 2);
 

src/System.Windows.Forms/src/System/Windows/Forms/MonthCalendar.CalendarGridChildAccessibleObject.cs

@@ -11,11 +11,16 @@ public partial class MonthCalendar
         /// </summary>
         internal abstract class CalendarGridChildAccessibleObject : CalendarChildAccessibleObject
         {
-            protected AccessibleObject _parentAccessibleObject;
+            protected readonly AccessibleObject _parentAccessibleObject;
 
             public CalendarGridChildAccessibleObject(MonthCalendarAccessibleObject calendarAccessibleObject, int calendarIndex, CalendarChildType itemType,
                 AccessibleObject parentAccessibleObject, int itemIndex) : base(calendarAccessibleObject, calendarIndex, itemType)
             {
+                if (parentAccessibleObject == null)
+                {
+                    throw new ArgumentNullException(nameof(parentAccessibleObject));
+                }
+
                 _parentAccessibleObject = parentAccessibleObject;
             }
 

src/System.Windows.Forms/src/System/Windows/Forms/MonthCalendar.MonthCalendarAccessibleObject.cs

@@ -25,7 +25,7 @@ internal class MonthCalendarAccessibleObject : ControlAccessibleObject
             public MonthCalendarAccessibleObject(Control owner)
                 : base(owner)
             {
-                _owner = owner as MonthCalendar;
+                _owner = (MonthCalendar)owner;
             }
 
             public int ControlType =>
@@ -290,7 +290,7 @@ internal override UnsafeNativeMethods.IRawElementProviderFragment ElementProvide
                     case ComCtl32.MCHT.CALENDARDATE:
                         // Get calendar body's child.
                         CalendarBodyAccessibleObject calendarBodyAccessibleObject = (CalendarBodyAccessibleObject)GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarBody);
-                        return calendarBodyAccessibleObject.GetFromPoint(hitTestInfo);
+                        return calendarBodyAccessibleObject?.GetFromPoint(hitTestInfo);
 
                     case ComCtl32.MCHT.TODAYLINK:
                         return GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.TodayLink);
@@ -365,7 +365,8 @@ public string GetCalendarChildName(int calendarIndex, CalendarChildType calendar
 
             private CalendarCellAccessibleObject GetCalendarCell(int calendarIndex, AccessibleObject parentAccessibleObject, int columnIndex)
             {
-                if (columnIndex < 0 ||
+                if (parentAccessibleObject == null ||
+                    columnIndex < 0 ||
                     columnIndex >= MAX_DAYS ||
                     columnIndex >= ColumnCount)
                 {
@@ -418,7 +419,8 @@ private string GetCalendarCellName(DateTime endDate, DateTime startDate, string
 
             private CalendarRowAccessibleObject GetCalendarRow(int calendarIndex, AccessibleObject parentAccessibleObject, int rowIndex)
             {
-                if ((HasHeaderRow ? rowIndex < -1 : rowIndex < 0) ||
+                if (parentAccessibleObject == null ||
+                    (HasHeaderRow ? rowIndex < -1 : rowIndex < 0) ||
                     rowIndex >= RowCount)
                 {
                     return null;
@@ -443,7 +445,7 @@ private CalendarRowAccessibleObject GetCalendarRow(int calendarIndex, Accessible
 
                 SelectionRange cellsRange = _owner.GetDisplayRange(false);
 
-                if (cellsRange.Start > DateTimePicker.SysTimeToDateTime(endDate) || cellsRange.End < DateTimePicker.SysTimeToDateTime(startDate))
+                if (cellsRange == null || cellsRange.Start > DateTimePicker.SysTimeToDateTime(endDate) || cellsRange.End < DateTimePicker.SysTimeToDateTime(startDate))
                 {
                     // Do not create row if the row's first cell is out of the current calendar's view range.
                     return null;
@@ -588,7 +590,7 @@ public void RaiseMouseClick(int x, int y)
             {
                 POINT previousPosition = new POINT();
                 bool setOldCursorPos = UnsafeNativeMethods.GetPhysicalCursorPos(ref previousPosition);
- 
+
                 bool mouseSwapped = User32.GetSystemMetrics(User32.SystemMetric.SM_SWAPBUTTON) != 0;
 
                 SendMouseInput(x, y, User32.MOUSEEVENTF.MOVE | User32.MOUSEEVENTF.ABSOLUTE);
@@ -669,13 +671,23 @@ public void RaiseAutomationEventForChild(int automationEventId, DateTime selecti
 
             private AccessibleObject GetCalendarChildAccessibleObject(DateTime selectionStart, DateTime selectionEnd)
             {
-                int columnCount = ColumnCount;
-
                 AccessibleObject bodyAccessibleObject = GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarBody);
+
+                if (bodyAccessibleObject == null)
+                {
+                    return null;
+                }
+
                 for (int row = 0; row < RowCount; row++)
                 {
                     AccessibleObject rowAccessibleObject = GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarRow, bodyAccessibleObject, row);
-                    for (int column = 0; column < columnCount; column++)
+
+                    if (rowAccessibleObject == null)
+                    {
+                        continue;
+                    }
+
+                    for (int column = 0; column < ColumnCount; column++)
                     {
                         bool success = GetCalendarGridInfo(
                             ComCtl32.MCGIF.DATE,
@@ -724,6 +736,12 @@ internal override UnsafeNativeMethods.IRawElementProviderSimple[] GetColumnHeade
                 UnsafeNativeMethods.IRawElementProviderSimple[] headers =
                     new UnsafeNativeMethods.IRawElementProviderSimple[MonthCalendarAccessibleObject.MAX_DAYS];
                 AccessibleObject headerRowAccessibleObject = GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarRow, this, -1);
+
+                if (headerRowAccessibleObject == null)
+                {
+                    return null;
+                }
+
                 for (int columnIndex = 0; columnIndex < MonthCalendarAccessibleObject.MAX_DAYS; columnIndex++)
                 {
                     headers[columnIndex] = GetCalendarChildAccessibleObject(_calendarIndex, CalendarChildType.CalendarCell, headerRowAccessibleObject, columnIndex);

src/System.Windows.Forms/tests/UnitTests/AccessibleObjects/MonthCalendar.MonthCalendarAccessibleObjectTests.cs

@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Reflection;
+using Xunit;
+using static System.Windows.Forms.MonthCalendar;
+
+namespace System.Windows.Forms.Tests.AccessibleObjects
+{
+    public class MonthCalendarAccessibleObjectTests
+    {
+        [WinFormsFact]
+        public void MonthCalendarAccessibleObject_GetCalendarCell_DoesntThrowException_If_ParentAccessibleObject_IsNull()
+        {
+            using MonthCalendar monthCalendar = new MonthCalendar();
+            MonthCalendarAccessibleObject accessibleObject = (MonthCalendarAccessibleObject)monthCalendar.AccessibilityObject;
+            Type type = typeof(MonthCalendarAccessibleObject);
+            MethodInfo method = type.GetMethod("GetCalendarCell", BindingFlags.NonPublic | BindingFlags.Instance);
+            Assert.Null(method.Invoke(accessibleObject, new object[] { 0, /*parentAccessibleObject*/ null, 0 }));
+        }
+    }
+}

src/System.Windows.Forms/tests/UnitTests/DateTimePickerTests.cs

@@ -16,5 +16,15 @@ public void DateTimePicker_Constructor()
             Assert.NotNull(dtp);
             Assert.Equal(DateTimePickerFormat.Long, dtp.Format);
         }
+
+        [WinFormsFact]
+        public void DateTimePicker_SysTimeToDateTime_DoesnThrowException_If_SYSTEMTIME_IsIncorrect()
+        {
+            // An empty SYSTEMTIME has year, month and day as 0, but DateTime can't have these parameters.
+            // So an empty SYSTEMTIME is incorrect in this case.
+            Interop.Kernel32.SYSTEMTIME systemTime = new Interop.Kernel32.SYSTEMTIME();
+            DateTime dateTime = DateTimePicker.SysTimeToDateTime(systemTime);
+            Assert.Equal(DateTime.MinValue, dateTime);
+        }
     }
 }

src/System.Windows.Forms/tests/UnitTests/MonthCalendarTests.cs

@@ -2,7 +2,14 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Threading;
 using Xunit;
+using System.Runtime.InteropServices;
+using System.Collections.Generic;
+using System.Reflection;
+using static Interop.Kernel32;
+using static System.Windows.Forms.NativeMethods;
+using static Interop;
 
 namespace System.Windows.Forms.Tests
 {
@@ -16,5 +23,121 @@ public void MonthCalendar_Constructor()
             Assert.NotNull(mc);
             Assert.True(mc.TabStop);
         }
+
+        public static IEnumerable<object[]> MonthCalendar_SettingDate_DoesntCrashApplication_TestData()
+        {
+            yield return new object[] { new MonthCalendar { MinDate = new DateTime(2020, 4, 9) } };
+            yield return new object[] { new MonthCalendar { MaxDate = new DateTime(2020, 4, 22) } };
+            yield return new object[] { new MonthCalendar() };
+        }
+
+        /// <summary>
+        ///  We have to check that UseVisualStyles state isn't changed.
+        ///  This test name is used to make sure the test will be executed 
+        ///  BEFORE <see cref="MonthCalendar_SettingDate_DoesntCrashApplication(MonthCalendar)" /> test.
+        /// </summary>
+        [Fact]
+        public void Application_UseVisualStyles_IsNotSet()
+        {
+            Assert.False(Application.UseVisualStyles);
+        }
+
+        [Theory]
+        [MemberData(nameof(MonthCalendar_SettingDate_DoesntCrashApplication_TestData))]
+        public void MonthCalendar_SettingDate_DoesntCrashApplication(MonthCalendar calendar)
+        {
+            Assert.False(Application.UseVisualStyles);
+            Exception exception = null;
+            ThreadExceptionEventHandler getException =
+                    (object sender, ThreadExceptionEventArgs e) => exception = e.Exception;
+
+            try
+            {
+                /// EnableVisualStyles method changes UseVisualStyles state. 
+                /// It mustn't affect other tests, 
+                /// so we have to check it in the <see cref="MonthCalendar_SettingDate_UseVisualStyles_IsNotSet()" /> test.
+                Application.EnableVisualStyles();
+                Application.ThreadException += getException;
+
+                // We need to check if the calendar works well when selecting some date
+                // if the calendar has a min/max date or not.
+                // In this case, a min/max date is chosen so that the calendar grid doesn't have some dates.
+                // This could potentially lead to exceptions when creating the calendar AccessibleObject,
+                // so we have to check these cases.
+                // Application running and TestForm using are necessary
+                // because otherwise there is no way to get a correct result when getting MCGRIDINFO from the calendar.
+                Application.Run(new TestForm(calendar));
+            }
+            finally
+            {
+                Application.ExitThread();
+
+                // Return UseVisualStyles as it was before.
+                PropertyInfo prop = typeof(Application).GetProperty(nameof(Application.UseVisualStyles));
+                prop?.SetValue(null, false, BindingFlags.NonPublic | BindingFlags.Static, null, null, null);
+
+                // Verify the process is finished correctly
+                Assert.False(Application.UseVisualStyles);
+                Assert.Null(exception);
+                Assert.False(Application.MessageLoop);
+            }
+        }
+
+        class TestForm : Form
+        {
+            private readonly MonthCalendar _monthCalendar;
+
+            public TestForm(MonthCalendar calendar)
+            {
+                _monthCalendar = calendar;
+                Controls.Add(_monthCalendar);
+                Load += Form_Load;
+            }
+
+            private void Form_Load(object sender, EventArgs e)
+            {
+                // Simulate the Windows notification sending about changing a selected date.
+                try
+                {
+                    DateTime selectedDate = new DateTime(2020, 4, 10);
+
+                    SYSTEMTIME date = new SYSTEMTIME
+                    {
+                        wYear = (short)selectedDate.Year,
+                        wMonth = (short)selectedDate.Month,
+                        wDay = (short)selectedDate.Day
+                    };
+
+                    Assert.NotEqual(IntPtr.Zero, _monthCalendar.Handle);
+
+                    NMSELCHANGE lParam = new NMSELCHANGE
+                    {
+                        nmhdr = new NMHDR
+                        {
+                            code = MCN_SELCHANGE,
+                        },
+                        stSelStart = date,
+                        stSelEnd = date,
+                    };
+
+                    UnsafeNativeMethods.SendMessage(new HandleRef(_monthCalendar, _monthCalendar.Handle), WindowMessages.WM_REFLECT + WindowMessages.WM_NOTIFY, 0, lParam);
+                }
+                finally
+                {
+                    Close();
+                }
+            }
+        }
+
+        /// <summary>
+        ///  We have to check that UseVisualStyles state isn't changed.
+        ///  This test name is used to make sure the test will be executed 
+        ///  AFTER <see cref="MonthCalendar_SettingDate_DoesntCrashApplication(MonthCalendar)" /> test.
+        /// </summary>
+        [Fact]
+        public void MonthCalendar_SettingDate_UseVisualStyles_IsNotSet()
+        {
+            Assert.False(Application.UseVisualStyles);
+        }
     }
 }

src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ApplicationTests.cs

@@ -4,16 +4,30 @@
 
 using Xunit;
 using System.Runtime.InteropServices;
+using System.Reflection;
 
 namespace System.Windows.Forms.Tests
 {
     public class ApplicationTests
     {
+        private readonly object _locker = new object();
+
         [Fact]
         public void Application_EnableVisualStyles_GetUseVisualStyles_ReturnsTrue()
         {
-            Application.EnableVisualStyles();
-            Assert.True(Application.UseVisualStyles, "New Visual Styles will not be applied on Winforms app. This is a high priority bug and must be looked into");
+            lock (_locker)
+            {
+                Assert.False(Application.UseVisualStyles);
+
+                Application.EnableVisualStyles();
+                Assert.True(Application.UseVisualStyles, "New Visual Styles will not be applied on Winforms app. This is a high priority bug and must be looked into");
+
+                // Return UseVisualStyles as it was before.
+                PropertyInfo prop = typeof(Application).GetProperty(nameof(Application.UseVisualStyles));
+                prop?.SetValue(null, false, BindingFlags.NonPublic | BindingFlags.Static, null, null, null);
+            }
+
+            Assert.False(Application.UseVisualStyles);
         }
 
         [Fact]