From 3df9c753806607fb3ac9fbf6a410fccc9d572f5a Mon Sep 17 00:00:00 2001 From: Ruben Schmidmeister Date: Thu, 5 Mar 2020 13:29:00 +0100 Subject: [PATCH 1/4] Ignore existing cache-control header when it disables caching --- .../Internal/Response.cs | 5 ++- .../Internal/CacheControlHeaderWriterTest.cs | 28 +++++++++++--- Messerli.Session/Http/IResponse.cs | 2 +- .../Internal/CacheControlHeaderWriter.cs | 37 +++++++++++++++---- 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/Messerli.Session.AspNetCore/Internal/Response.cs b/Messerli.Session.AspNetCore/Internal/Response.cs index 92a5572..00e0a69 100644 --- 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; @@ -44,9 +45,9 @@ public void SetHeader(string name, string value) _httpContext.Response.Headers.Append(name, value); } - public bool HasHeader(string name) + public string? GetFirstHeaderValue(string name) { - return _httpContext.Response.Headers.ContainsKey(name); + return _httpContext.Response.Headers[name].FirstOrDefault(); } private bool MapSecurePreferenceToBool(CookieSecurePreference securePreference) diff --git a/Messerli.Session.Test/Internal/CacheControlHeaderWriterTest.cs b/Messerli.Session.Test/Internal/CacheControlHeaderWriterTest.cs index 42f9b1f..4dc1426 100644 --- 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,10 +29,10 @@ public void SetsHeaderWhenAutomaticCacheControlIsEnabled() .Setup(r => r.AutomaticCacheControl) .Returns(true); response - .Setup(r => r.HasHeader(It.IsAny())) - .Returns(false); + .Setup(r => r.GetFirstHeaderValue(It.IsAny())) + .Returns(() => null); response - .Setup(r => r.SetHeader(It.IsAny(), It.IsAny())); + .Setup(r => r.SetHeader(It.IsAny(), ExpectedCacheControlHeader)); var cacheControlHeaderWriter = CreateCacheControlHeaderWriter(); cacheControlHeaderWriter.AddCacheControlHeaders(response.Object); } @@ -38,13 +40,13 @@ public void SetsHeaderWhenAutomaticCacheControlIsEnabled() [Fact] public void ThrowsIfCacheControlHeaderIsAlreadyPresentInResponse() { + const string validCacheControlHeaderValue = "private, must-revalidate"; var response = new Mock(MockBehavior.Strict); response .Setup(r => r.AutomaticCacheControl) .Returns(true); - response - .Setup(r => r.HasHeader(It.IsAny())) - .Returns(true); + response.Setup(r => r.GetFirstHeaderValue(It.IsAny())) + .Returns(validCacheControlHeaderValue); var cacheControlHeaderWriter = CreateCacheControlHeaderWriter(); Assert.Throws(() => @@ -53,6 +55,20 @@ public void ThrowsIfCacheControlHeaderIsAlreadyPresentInResponse() }); } + [Fact] + public void DoesNothingWhenCachingIsAlreadyDisabled() + { + const string cacheControlNoCacheValue = "no-cache"; + var response = new Mock(MockBehavior.Strict); + response + .Setup(r => r.AutomaticCacheControl) + .Returns(true); + response.Setup(r => r.GetFirstHeaderValue(It.IsAny())) + .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 index 65b77ea..e8f94c5 100644 --- 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 index 3b1483a..28e92ec 100644 --- a/Messerli.Session/Internal/CacheControlHeaderWriter.cs +++ b/Messerli.Session/Internal/CacheControlHeaderWriter.cs @@ -1,4 +1,5 @@ using System; +using System.Net.Http.Headers; using Messerli.Session.Http; namespace Messerli.Session.Internal @@ -6,10 +7,6 @@ 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, + }; } } From a15a26ac0eb9f7e705a29f77891e3c0dfb5566ff Mon Sep 17 00:00:00 2001 From: Ruben Schmidmeister Date: Thu, 5 Mar 2020 13:29:50 +0100 Subject: [PATCH 2/4] Use expression body --- Messerli.Session.AspNetCore/Internal/Response.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/Messerli.Session.AspNetCore/Internal/Response.cs b/Messerli.Session.AspNetCore/Internal/Response.cs index 00e0a69..b5be5e0 100644 --- a/Messerli.Session.AspNetCore/Internal/Response.cs +++ b/Messerli.Session.AspNetCore/Internal/Response.cs @@ -41,35 +41,27 @@ 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 string? GetFirstHeaderValue(string name) - { - return _httpContext.Response.Headers[name].FirstOrDefault(); - } + => _httpContext.Response.Headers[name].FirstOrDefault(); 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(), }; - } } } From fbe967a679c169dcb8866741660fea341475e198 Mon Sep 17 00:00:00 2001 From: Ruben Schmidmeister Date: Fri, 6 Mar 2020 15:58:46 +0100 Subject: [PATCH 3/4] Bump version and update changelog --- .../Messerli.Session.AspNetCore.csproj | 2 +- Messerli.Session.AspNetCore/changelog.md | 4 ++++ Messerli.Session/Messerli.Session.csproj | 2 +- Messerli.Session/changelog.md | 3 +++ 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Messerli.Session.AspNetCore/Messerli.Session.AspNetCore.csproj b/Messerli.Session.AspNetCore/Messerli.Session.AspNetCore.csproj index ad72fef..84783be 100644 --- a/Messerli.Session.AspNetCore/Messerli.Session.AspNetCore.csproj +++ b/Messerli.Session.AspNetCore/Messerli.Session.AspNetCore.csproj @@ -5,7 +5,7 @@ true Messerli.Session.AspNetCore Messerli.Session.AspNetCore - 0.2.0 + 0.3.0 https://github.com/messerli-informatik-ag/session git Messerli.Session.AspNetCore diff --git a/Messerli.Session.AspNetCore/changelog.md b/Messerli.Session.AspNetCore/changelog.md index 50077a2..432f3fe 100644 --- 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/Messerli.Session.csproj b/Messerli.Session/Messerli.Session.csproj index 4b168ef..67a7a9c 100644 --- a/Messerli.Session/Messerli.Session.csproj +++ b/Messerli.Session/Messerli.Session.csproj @@ -5,7 +5,7 @@ true Messerli.Session Messerli.Session - 0.1.3 + 0.2.0 https://github.com/messerli-informatik-ag/session git Messerli.Session diff --git a/Messerli.Session/changelog.md b/Messerli.Session/changelog.md index a9c9f9a..73f6fde 100644 --- 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`. From 1712aa42773e9586e5ef22ceab3355182b6a7899 Mon Sep 17 00:00:00 2001 From: Ruben Schmidmeister Date: Fri, 6 Mar 2020 16:01:31 +0100 Subject: [PATCH 4/4] Use TryGetValue --- Messerli.Session.AspNetCore/Internal/Response.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Messerli.Session.AspNetCore/Internal/Response.cs b/Messerli.Session.AspNetCore/Internal/Response.cs index b5be5e0..cdfb0b6 100644 --- a/Messerli.Session.AspNetCore/Internal/Response.cs +++ b/Messerli.Session.AspNetCore/Internal/Response.cs @@ -44,7 +44,9 @@ public void SetHeader(string name, string value) => _httpContext.Response.Headers.Append(name, value); public string? GetFirstHeaderValue(string name) - => _httpContext.Response.Headers[name].FirstOrDefault(); + => _httpContext.Response.Headers.TryGetValue(name, out var value) + ? value.FirstOrDefault() + : null; private bool MapSecurePreferenceToBool(CookieSecurePreference securePreference) => securePreference switch