Fix NullReferenceExceptions in expressions and connection strings

src/libraries/Common/src/System/Data/Common/DbConnectionOptions.Common.cs

@@ -114,21 +114,20 @@ private string UsersConnectionString(bool hidePassword, bool forceHidePassword)
         public bool ConvertValueToBoolean(string keyName, bool defaultValue)
         {
             string? value;
-            // TODO: Is it possible for _parsetable to contain a null value here? If so there's a bug here, investigate.
             return _parsetable.TryGetValue(keyName, out value) ?
-                ConvertValueToBooleanInternal(keyName, value!) :
+                ConvertValueToBooleanInternal(keyName, value) :
                 defaultValue;
         }
 
-        internal static bool ConvertValueToBooleanInternal(string keyName, string stringValue)
+        internal static bool ConvertValueToBooleanInternal(string keyName, string? stringValue)
         {
             if (CompareInsensitiveInvariant(stringValue, "true") || CompareInsensitiveInvariant(stringValue, "yes"))
                 return true;
             else if (CompareInsensitiveInvariant(stringValue, "false") || CompareInsensitiveInvariant(stringValue, "no"))
                 return false;
             else
             {
-                string tmp = stringValue.Trim();  // Remove leading & trailing whitespace.
+                string? tmp = stringValue?.Trim();  // Remove leading & trailing whitespace.
                 if (CompareInsensitiveInvariant(tmp, "true") || CompareInsensitiveInvariant(tmp, "yes"))
                     return true;
                 else if (CompareInsensitiveInvariant(tmp, "false") || CompareInsensitiveInvariant(tmp, "no"))
@@ -140,7 +139,7 @@ internal static bool ConvertValueToBooleanInternal(string keyName, string string
             }
         }
 
-        private static bool CompareInsensitiveInvariant(string strvalue, string strconst) =>
+        private static bool CompareInsensitiveInvariant(string? strvalue, string strconst) =>
             (0 == StringComparer.OrdinalIgnoreCase.Compare(strvalue, strconst));
 
         [System.Diagnostics.Conditional("DEBUG")]

src/libraries/System.Data.Common/src/System/Data/DataTable.cs

@@ -7202,8 +7202,13 @@ internal void EvaluateDependentExpressions(List<DataColumn>? columns, DataRow ro
             for (int i = 0; i < count; i++)
             {
                 DataColumn dc = columns[i];
+                if (dc.DataExpression == null)
+                {
+                    continue;
+                }
+
                 // if this column is NOT in my table or it is in the table and is not a local aggregate (self refs)
-                if (dc.Table != this || (dc.DataExpression != null && !dc.DataExpression.HasLocalAggregate()))
+                if (dc.Table != this || !dc.DataExpression.HasLocalAggregate())
                 {
                     DataRowVersion foreignVer = (version == DataRowVersion.Proposed) ? DataRowVersion.Default : version;
 
@@ -7225,9 +7230,7 @@ internal void EvaluateDependentExpressions(List<DataColumn>? columns, DataRow ro
 
                             if (cachedRow != null && ((cachedRow.RowState != DataRowState.Deleted) && (version != DataRowVersion.Original || cachedRow._oldRecord != -1)))
                             {
-                                // if deleted GetRecordFromVersion will throw
-                                // TODO: Possible bug, dc.DataExpression may be null
-                                object newValue = dc.DataExpression!.Evaluate(cachedRow, foreignVer);
+                                object newValue = dc.DataExpression.Evaluate(cachedRow, foreignVer);
                                 SilentlySetValue(cachedRow, dc, foreignVer, newValue);
                             }
                         }
@@ -7257,9 +7260,7 @@ internal void EvaluateDependentExpressions(List<DataColumn>? columns, DataRow ro
 
                             if (parentRow != null && ((parentRow.RowState != DataRowState.Deleted) && (version != DataRowVersion.Original || parentRow._oldRecord != -1)))
                             {
-                                // if deleted GetRecordFromVersion will throw
-                                // TODO: Possible bug, dc.DataExpression may be null
-                                object newValue = dc.DataExpression!.Evaluate(parentRow, foreignVer);
+                                object newValue = dc.DataExpression.Evaluate(parentRow, foreignVer);
                                 SilentlySetValue(parentRow, dc, foreignVer, newValue);
                             }
                         }
@@ -7289,9 +7290,7 @@ internal void EvaluateDependentExpressions(List<DataColumn>? columns, DataRow ro
 
                             if (childRow != null && ((childRow.RowState != DataRowState.Deleted) && (version != DataRowVersion.Original || childRow._oldRecord != -1)))
                             {
-                                // if deleted GetRecordFromVersion will throw
-                                // TODO: Possible bug, dc.DataExpression may be null
-                                object newValue = dc.DataExpression!.Evaluate(childRow, foreignVer);
+                                object newValue = dc.DataExpression.Evaluate(childRow, foreignVer);
                                 SilentlySetValue(childRow, dc, foreignVer, newValue);
                             }
                         }

src/libraries/System.Data.Common/src/System/Data/Filter/DataExpression.cs

@@ -138,10 +138,9 @@ internal object Evaluate(DataRow? row, DataRowVersion version)
                     // we need to convert the return value to the column.Type;
                     try
                     {
-                        if (StorageType.Object != _storageType)
+                        if (_dataType != null && StorageType.Object != _storageType)
                         {
-                            // TODO: _dataType can be null, probably a bug
-                            result = SqlConvert.ChangeType2(result, _storageType, _dataType!, _table!.FormatProvider);
+                            result = SqlConvert.ChangeType2(result, _storageType, _dataType, _table == null ? System.Globalization.CultureInfo.CurrentCulture : _table.FormatProvider);
                         }
                     }
                     catch (Exception e) when (ADP.IsCatchableExceptionType(e))

src/libraries/System.Data.Common/tests/System/Data/Common/DbConnectionStringBuilderTest.cs

@@ -2281,5 +2281,19 @@ private void AssertValueTest(string[][] tests1, string[][] tests2, bool useOdbc)
                 }
             }
         }
+
+        [Fact]
+        public void DbConnectionOptions_EmptyBooleanValue_ThrowsArgumentException()
+        {
+            Type type = typeof(System.Data.Common.DbConnectionStringBuilder).Assembly.GetType("System.Data.Common.DbConnectionOptions");
+            Assert.NotNull(type);
+
+            object options = Activator.CreateInstance(type, new object[] { "persist security info=;", null, false });
+            System.Reflection.MethodInfo convertMethod = type.GetMethod("ConvertValueToBoolean", new Type[] { typeof(string), typeof(bool) });
+            Assert.NotNull(convertMethod);
+
+            var ex = Assert.Throws<System.Reflection.TargetInvocationException>(() => convertMethod.Invoke(options, new object[] { "persist security info", false }));
+            Assert.IsType<ArgumentException>(ex.InnerException);
+        }
     }
 }

src/libraries/System.Data.Common/tests/System/Data/DataTableTest.cs

@@ -3720,5 +3720,44 @@ private static void Select(DataTable tbl)
             DataRow[] rows = tbl.Select(filter);
             Assert.Equal(1, rows.Length);
         }
+
+        [Fact]
+        public void EvaluateDependentExpressions_NullDataExpression_DoesNotThrow()
+        {
+            DataTable table = new DataTable();
+            table.Columns.Add("Id", typeof(int));
+            table.Columns.Add("Value", typeof(int));
+            DataColumn computedCol = table.Columns.Add("Computed", typeof(int), "Id + Value");
+
+            table.Rows.Add(1, 10);
+            table.Rows.Add(2, 20);
+
+            // Nulling out the expression
+            computedCol.Expression = null;
+
+            // Triggering data change. 
+            // The underlying EvaluateDependentExpressions method now gracefully handles null expressions.
+            var exception = Record.Exception(() => table.Rows[0]["Id"] = 5);
+            Assert.Null(exception);
+        }
+
+        [Fact]
+        public void DataExpression_Evaluate_NullDataType_DoesNotThrowArgumentNullException()
+        {
+            DataTable table = new DataTable();
+            table.Columns.Add("A", typeof(int));
+            table.Rows.Add(5);
+
+            Type expressionType = typeof(DataTable).Assembly.GetType("System.Data.DataExpression");
+            Assert.NotNull(expressionType);
+
+            object expr = Activator.CreateInstance(expressionType!, System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Public, null, new object[] { table, "A + 5", null }, null);
+
+            System.Reflection.MethodInfo evaluateMethod = expressionType.GetMethod("Evaluate", new Type[] { typeof(DataRow), typeof(DataRowVersion) });
+            Assert.NotNull(evaluateMethod);
+            object result = evaluateMethod!.Invoke(expr, new object[] { table.Rows[0], DataRowVersion.Default });
+
+            Assert.Equal(10, Convert.ToInt32(result));
+        }
     }
 }