Merge pull request #30 from messerli-informatik-ag/fix-cache-control-…

…header-clash Fix cache control header clash
messerli-informatik-ag · Mar 9, 2020 · 57b9751 · 57b9751
2 parents 35d2db3 + 1712aa4
commit 57b9751
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 30 deletions.
diff --git a/Messerli.Session.AspNetCore/Internal/Response.cs b/Messerli.Session.AspNetCore/Internal/Response.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using Messerli.Session.Configuration;
 using Messerli.Session.Http;
 using Microsoft.AspNetCore.Http;
@@ -40,35 +41,29 @@ public void SetCookie(Cookie cookie)
         }
 
         public void SetHeader(string name, string value)
-        {
-            _httpContext.Response.Headers.Append(name, value);
-        }
+            => _httpContext.Response.Headers.Append(name, value);
 
-        public bool HasHeader(string name)
-        {
-            return _httpContext.Response.Headers.ContainsKey(name);
-        }
+        public string? GetFirstHeaderValue(string name)
+            => _httpContext.Response.Headers.TryGetValue(name, out var value)
+                ? value.FirstOrDefault()
+                : null;
 
         private bool MapSecurePreferenceToBool(CookieSecurePreference securePreference)
-        {
-            return securePreference switch
+            => securePreference switch
             {
                 CookieSecurePreference.Always => true,
                 CookieSecurePreference.Never => false,
                 CookieSecurePreference.MatchingRequest => _httpContext.Request.IsHttps,
                 _ => throw new InvalidOperationException(),
             };
-        }
 
         private static SameSiteMode MapToAspNetCoreSameSiteMode(CookieSameSiteMode sameSiteMode)
-        {
-            return sameSiteMode switch
+            => sameSiteMode switch
             {
                 CookieSameSiteMode.None => SameSiteMode.None,
                 CookieSameSiteMode.Lax => SameSiteMode.Lax,
                 CookieSameSiteMode.Strict => SameSiteMode.Strict,
                 _ => throw new InvalidOperationException(),
             };
-        }
     }
 }
diff --git a/Messerli.Session.AspNetCore/Messerli.Session.AspNetCore.csproj b/Messerli.Session.AspNetCore/Messerli.Session.AspNetCore.csproj
@@ -5,7 +5,7 @@
         <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
         <RootNamespace>Messerli.Session.AspNetCore</RootNamespace>
         <AssemblyName>Messerli.Session.AspNetCore</AssemblyName>
-        <Version>0.2.0</Version>
+        <Version>0.3.0</Version>
         <RepositoryUrl>https://github.com/messerli-informatik-ag/session</RepositoryUrl>
         <RepositoryType>git</RepositoryType>
         <PackageId>Messerli.Session.AspNetCore</PackageId>

diff --git a/Messerli.Session.AspNetCore/changelog.md b/Messerli.Session.AspNetCore/changelog.md
@@ -12,3 +12,7 @@
 ## 0.2.0
 - Depend on latest version (3.1.x) of Microsoft.Extensions.Caching.Abstractions.
 - Depend on latest version (3.1.x) of Microsoft.Extensions.Logging.Abstractions.
+
+## 0.3.0
+- Do not throw when `Cache-Control` is already present and set to `no-cache`.
+  This was an issue when using the session middleware and ASP.NET Core's built-in anti forgery handling.
diff --git a/Messerli.Session.Test/Internal/CacheControlHeaderWriterTest.cs b/Messerli.Session.Test/Internal/CacheControlHeaderWriterTest.cs
@@ -8,6 +8,8 @@ namespace Messerli.Session.Test.Internal
 {
     public class CacheControlHeaderWriterTest
     {
+        private const string ExpectedCacheControlHeader = "must-revalidate, max-age=0, private";
+
         [Fact]
         public void DoesNothingWhenAutomaticCacheControlIsDisabledOnResponse()
         {
@@ -27,24 +29,24 @@ public void SetsHeaderWhenAutomaticCacheControlIsEnabled()
                 .Setup(r => r.AutomaticCacheControl)
                 .Returns(true);
             response
-                .Setup(r => r.HasHeader(It.IsAny<string>()))
-                .Returns(false);
+                .Setup(r => r.GetFirstHeaderValue(It.IsAny<string>()))
+                .Returns(() => null);
             response
-                .Setup(r => r.SetHeader(It.IsAny<string>(), It.IsAny<string>()));
+                .Setup(r => r.SetHeader(It.IsAny<string>(), ExpectedCacheControlHeader));
             var cacheControlHeaderWriter = CreateCacheControlHeaderWriter();
             cacheControlHeaderWriter.AddCacheControlHeaders(response.Object);
         }
 
         [Fact]
         public void ThrowsIfCacheControlHeaderIsAlreadyPresentInResponse()
         {
+            const string validCacheControlHeaderValue = "private, must-revalidate";
             var response = new Mock<IResponse>(MockBehavior.Strict);
             response
                 .Setup(r => r.AutomaticCacheControl)
                 .Returns(true);
-            response
-                .Setup(r => r.HasHeader(It.IsAny<string>()))
-                .Returns(true);
+            response.Setup(r => r.GetFirstHeaderValue(It.IsAny<string>()))
+                .Returns(validCacheControlHeaderValue);
             var cacheControlHeaderWriter = CreateCacheControlHeaderWriter();
 
             Assert.Throws<InvalidOperationException>(() =>
@@ -53,6 +55,20 @@ public void ThrowsIfCacheControlHeaderIsAlreadyPresentInResponse()
             });
         }
 
+        [Fact]
+        public void DoesNothingWhenCachingIsAlreadyDisabled()
+        {
+            const string cacheControlNoCacheValue = "no-cache";
+            var response = new Mock<IResponse>(MockBehavior.Strict);
+            response
+                .Setup(r => r.AutomaticCacheControl)
+                .Returns(true);
+            response.Setup(r => r.GetFirstHeaderValue(It.IsAny<string>()))
+                .Returns(cacheControlNoCacheValue);
+            var cacheControlHeaderWriter = CreateCacheControlHeaderWriter();
+            cacheControlHeaderWriter.AddCacheControlHeaders(response.Object);
+        }
+
         private static ICacheControlHeaderWriter CreateCacheControlHeaderWriter()
         {
             return new CacheControlHeaderWriter();

diff --git a/Messerli.Session/Http/IResponse.cs b/Messerli.Session/Http/IResponse.cs
@@ -8,6 +8,6 @@ public interface IResponse
 
         void SetHeader(string name, string value);
 
-        bool HasHeader(string name);
+        string? GetFirstHeaderValue(string name);
     }
 }
diff --git a/Messerli.Session/Internal/CacheControlHeaderWriter.cs b/Messerli.Session/Internal/CacheControlHeaderWriter.cs
@@ -1,15 +1,12 @@
 using System;
+using System.Net.Http.Headers;
 using Messerli.Session.Http;
 
 namespace Messerli.Session.Internal
 {
     internal class CacheControlHeaderWriter : ICacheControlHeaderWriter
     {
         private const string HeaderName = "Cache-Control";
-        private const string Cacheability = "private";
-        private const string Expiration = "max-age=0";
-        private const string Validation = "must-revalidate";
-        private static readonly string CacheControlValue = $"{Cacheability}, {Expiration}, {Validation}";
 
         public void AddCacheControlHeaders(IResponse response)
         {
@@ -18,13 +15,37 @@ public void AddCacheControlHeaders(IResponse response)
                 return;
             }
 
-            if (response.HasHeader(HeaderName))
+            if (response.GetFirstHeaderValue(HeaderName) is { } headerValue)
             {
-                throw new InvalidOperationException(
-                    $"The {HeaderName} is already present in the response. If you need to set the header to a custom value, disable automatic cache control.");
+                ValidateExistingHeader(headerValue);
+            }
+            else
+            {
+                SetCacheControlHeader(response);
             }
+        }
+
+        private static void ValidateExistingHeader(string headerValue)
+        {
+            var cacheControlHeader = CacheControlHeaderValue.Parse(headerValue);
 
-            response.SetHeader(HeaderName, CacheControlValue);
+            if (!cacheControlHeader.NoCache)
+            {
+                throw new InvalidOperationException(
+                    $"The {HeaderName} header is already present in the response and does not disable caching completely. " +
+                    $"If you need to set the header to a custom value, disable automatic cache control.");
+            }
         }
+
+        private static void SetCacheControlHeader(IResponse response)
+            => response.SetHeader(HeaderName, CreateCacheControlHeader().ToString());
+
+        private static CacheControlHeaderValue CreateCacheControlHeader()
+            => new CacheControlHeaderValue
+            {
+                Private = true,
+                MaxAge = TimeSpan.Zero,
+                MustRevalidate = true,
+            };
     }
 }
diff --git a/Messerli.Session/Messerli.Session.csproj b/Messerli.Session/Messerli.Session.csproj
@@ -5,7 +5,7 @@
         <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
         <RootNamespace>Messerli.Session</RootNamespace>
         <AssemblyName>Messerli.Session</AssemblyName>
-        <Version>0.1.3</Version>
+        <Version>0.2.0</Version>
         <RepositoryUrl>https://github.com/messerli-informatik-ag/session</RepositoryUrl>
         <RepositoryType>git</RepositoryType>
         <PackageId>Messerli.Session</PackageId>

diff --git a/Messerli.Session/changelog.md b/Messerli.Session/changelog.md
@@ -14,3 +14,6 @@
 ## 0.1.3
 - Added support for the `SameSite` cookie attribute is now supported.
   The attribute is set to `Lax` by default.
+
+## 0.2.0
+- Do not throw when `Cache-Control` is already present and set to `no-cache`.