[Fixes #3068] TempData fails silently without sessions middleware

aspnet · Sep 30, 2015 · 94c87c1 · 94c87c1
1 parent eefa58e
commit 94c87c1
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 38 deletions.
diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SessionStateTempDataProvider.cs
@@ -59,16 +59,10 @@ public virtual IDictionary<string, object> LoadTempData(HttpContext context)
                 throw new ArgumentNullException(nameof(context));
             }
 
-            if (!IsSessionEnabled(context))
-            {
-                // Session middleware is not enabled. No-op
-                return null;
-            }
-
             var session = context.Session;
             if (session == null)
             {
-                return null;
+                throw new InvalidOperationException("Session cannot be null.");
             }
 
             var tempDataDictionary = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
@@ -162,6 +156,9 @@ public virtual void SaveTempData(HttpContext context, IDictionary<string, object
                 throw new ArgumentNullException(nameof(context));
             }
 
+            // Accessing Session property will throw if the session middleware is not enabled.
+            var session = context.Session;
+
             var hasValues = (values != null && values.Count > 0);
             if (hasValues)
             {
@@ -171,9 +168,6 @@ public virtual void SaveTempData(HttpContext context, IDictionary<string, object
                     EnsureObjectCanBeSerialized(item);
                 }
 
-                // Accessing Session property will throw if the session middleware is not enabled.
-                var session = context.Session;
-
                 using (var memoryStream = new MemoryStream())
                 {
                     using (var writer = new BsonWriter(memoryStream))
@@ -183,18 +177,12 @@ public virtual void SaveTempData(HttpContext context, IDictionary<string, object
                     }
                 }
             }
-            else if (IsSessionEnabled(context))
+            else
             {
-                var session = context.Session;
                 session.Remove(TempDataSessionStateKey);
             }
         }
 
-        private static bool IsSessionEnabled(HttpContext context)
-        {
-            return context.Features.Get<ISessionFeature>() != null;
-        }
-
         internal void EnsureObjectCanBeSerialized(object item)
         {
             var itemType = item.GetType();

diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/TempDataDictionary.cs
@@ -101,6 +101,13 @@ public object this[string key]
         /// <inheritdoc />
         public void Keep()
         {
+            // if the data is not loaded, we can assume none of it has been read
+            // and so silently return.
+            if (!_loaded)
+            {
+                return;
+            }
+
             Load();
             _retainedKeys.Clear();
             _retainedKeys.UnionWith(_data.Keys);

diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/SessionStateTempDataProviderTest.cs
@@ -16,52 +16,54 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
     public class SessionStateTempDataProviderTest
     {
         [Fact]
-        public void Load_NullSession_ReturnsEmptyDictionary()
+        public void Load_ThrowsException_WhenSessionIsNotEnabled()
         {
             // Arrange
             var testProvider = new SessionStateTempDataProvider();
 
-            // Act
-            var tempDataDictionary = testProvider.LoadTempData(
-                GetHttpContext(session: null, sessionEnabled: true));
-
-            // Assert
-            Assert.Null(tempDataDictionary);
+            // Act & Assert
+            var exception = Assert.Throws<InvalidOperationException>(() => testProvider.LoadTempData(
+                GetHttpContext(session: null, sessionEnabled: false)));
         }
 
         [Fact]
-        public void Load_NonNullSession_NoSessionData_ReturnsEmptyDictionary()
+        public void Save_ThrowsException_WhenSessionIsNotEnabled()
         {
             // Arrange
             var testProvider = new SessionStateTempDataProvider();
+            var values = new Dictionary<string, object>();
+            values.Add("key1", "value1");
 
-            // Act
-            var tempDataDictionary = testProvider.LoadTempData(
-                GetHttpContext(Mock.Of<ISession>()));
-
-            // Assert
-            Assert.Empty(tempDataDictionary);
+            // Act & Assert
+            var exception = Assert.Throws<InvalidOperationException>(() => testProvider.SaveTempData(
+                GetHttpContext(session: null, sessionEnabled: false), values));
         }
 
         [Fact]
-        public void Save_NullSession_NullDictionary_DoesNotThrow()
+        public void Load_NullSession_ReturnsEmptyDictionary()
         {
             // Arrange
             var testProvider = new SessionStateTempDataProvider();
 
-            // Act & Assert (does not throw)
-            testProvider.SaveTempData(GetHttpContext(session: null, sessionEnabled: false), null);
+            // Act & Assert
+            var exception = Assert.Throws<InvalidOperationException>(() => testProvider.LoadTempData(
+                GetHttpContext(session: null, sessionEnabled: true)));
+
+            Assert.Equal("Session cannot be null.", exception.Message);
         }
 
         [Fact]
-        public void Save_NullSession_EmptyDictionary_DoesNotThrow()
+        public void Load_NonNullSession_NoSessionData_ReturnsEmptyDictionary()
         {
             // Arrange
             var testProvider = new SessionStateTempDataProvider();
 
-            // Act & Assert (does not throw)
-            testProvider.SaveTempData(
-                GetHttpContext(session: null, sessionEnabled: false), new Dictionary<string, object>());
+            // Act
+            var tempDataDictionary = testProvider.LoadTempData(
+                GetHttpContext(Mock.Of<ISession>()));
+
+            // Assert
+            Assert.Empty(tempDataDictionary);
         }
 
         [Fact]

diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/TempDataDictionaryTest.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using Microsoft.AspNet.Http;
 using Moq;
@@ -10,6 +11,26 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
 {
     public class TempDataDictionaryTest
     {
+        [Fact]
+        public void ThrowsException_OnSettingValue_AndWhenSessionIsNotEnabled()
+        {
+            // Arrange
+            var tempData = new TempDataDictionary(GetHttpContextAccessor(), new SessionStateTempDataProvider());
+
+            // Act & Assert
+            Assert.Throws<InvalidOperationException>(() => tempData["key1"] = "value1");
+        }
+
+        [Fact]
+        public void Keep_DoesNotThrowException_WhenDataIsNotLoaded()
+        {
+            // Arrange
+            var tempData = new TempDataDictionary(GetHttpContextAccessor(), new SessionStateTempDataProvider());
+
+            // Act & Assert
+            tempData.Keep();
+        }
+
         [Fact]
         public void TempData_Load_CreatesEmptyDictionaryIfProviderReturnsNull()
         {