From f4094bfaaa7e2fa3fd5b765949adb464a9bd4d73 Mon Sep 17 00:00:00 2001
From: Taylor Southwick <tasou@microsoft.com>
Date: Tue, 29 Sep 2020 13:59:26 -0700
Subject: [PATCH] Check for null in OptionsFactory (#41047)

If an IValidateOptions<TOptions> returns null, then an error occurs that can be difficult to track down. This change checks for null before checking if it failed.
---
 .../src/OptionsFactory.cs                          |  2 +-
 .../OptionsFactoryTests.cs                         | 27 ++++++++++++++++++----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsFactory.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsFactory.cs
index dec163c..35c383d 100644
--- a/src/libraries/Microsoft.Extensions.Options/src/OptionsFactory.cs
+++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsFactory.cs
@@ -68,7 +68,7 @@ namespace Microsoft.Extensions.Options
                 foreach (IValidateOptions<TOptions> validate in _validations)
                 {
                     ValidateOptionsResult result = validate.Validate(name, options);
-                    if (result.Failed)
+                    if (result is not null && result.Failed)
                     {
                         failures.AddRange(result.Failures);
                     }
diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsFactoryTests.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsFactoryTests.cs
index e49c65e..5c09fe9 100644
--- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsFactoryTests.cs
+++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsFactoryTests.cs
@@ -33,8 +33,8 @@ namespace Microsoft.Extensions.Options.Tests
             var factory = sp.GetRequiredService<IOptionsFactory<FakeOptions>>();
             Assert.Equal("UP", factory.Create("UP").Message);
             Assert.Equal("up", factory.Create("up").Message);
-        }        
-        
+        }
+
         [Fact]
         public void CanConfigureAllOptions()
         {
@@ -199,6 +199,14 @@ namespace Microsoft.Extensions.Options.Tests
             }
         }
 
+        public class FakeOptionsValidationNull : IValidateOptions<FakeOptions>
+        {
+            public ValidateOptionsResult Validate(string name, FakeOptions options)
+            {
+                return null;
+            }
+        }
+
         [Fact]
         public void CanValidateOptionsWithConfigureOptions()
         {
@@ -212,6 +220,17 @@ namespace Microsoft.Extensions.Options.Tests
             Assert.Equal("Hello world", message);
         }
 
+        [Fact]
+        public void ValidateOptionsChecksNullWithConfigureOptions()
+        {
+            var factory = new ServiceCollection()
+                .ConfigureOptions<FakeOptionsValidationNull>()
+                .BuildServiceProvider()
+                .GetRequiredService<IOptionsFactory<FakeOptions>>();
+
+            Assert.NotNull(factory.Create(Options.DefaultName));
+        }
+
         public class UberSetup
             : IConfigureNamedOptions<FakeOptions>
             , IConfigureNamedOptions<FakeOptions2>
@@ -221,12 +240,12 @@ namespace Microsoft.Extensions.Options.Tests
             , IValidateOptions<FakeOptions2>
         {
             public void Configure(string name, FakeOptions options)
-                => options.Message += "["+name;
+                => options.Message += "[" + name;
 
             public void Configure(FakeOptions options) => Configure(Options.DefaultName, options);
 
             public void Configure(string name, FakeOptions2 options)
-                => options.Message += "[["+name;
+                => options.Message += "[[" + name;
 
             public void Configure(FakeOptions2 options) => Configure(Options.DefaultName, options);
 
-- 
2.7.4