diff --git a/backend/src/Squidex.Web/Pipeline/CachingFilter.cs b/backend/src/Squidex.Web/Pipeline/CachingFilter.cs index 77b1779d5..981ee5259 100644 --- a/backend/src/Squidex.Web/Pipeline/CachingFilter.cs +++ b/backend/src/Squidex.Web/Pipeline/CachingFilter.cs @@ -33,42 +33,60 @@ namespace Squidex.Web.Pipeline public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) { - var httpContext = context.HttpContext; + cachingManager.Start(context.HttpContext); - cachingManager.Start(httpContext); - - cachingManager.AddHeader("Auth-State"); - - if (!string.IsNullOrWhiteSpace(httpContext.User.OpenIdSubject())) - { - cachingManager.AddHeader(HeaderNames.Authorization); - } - else if (!string.IsNullOrWhiteSpace(httpContext.User.OpenIdClientId())) - { - cachingManager.AddHeader("Auth-ClientId"); - } + AppendAuthHeaders(context.HttpContext); var resultContext = await next(); - if (httpContext.Response.Headers.TryGetString(HeaderNames.ETag, out var etag)) + if (resultContext.HttpContext.Response.Headers.TryGetString(HeaderNames.ETag, out var etag)) { - if (!cachingOptions.StrongETag && !etag.StartsWith("W/", StringComparison.OrdinalIgnoreCase)) + if (!cachingOptions.StrongETag && IsWeakEtag(etag)) { - etag = $"W/{etag}"; + etag = ToWeakEtag(etag); - httpContext.Response.Headers[HeaderNames.ETag] = etag; + resultContext.HttpContext.Response.Headers[HeaderNames.ETag] = etag; } - if (HttpMethods.IsGet(httpContext.Request.Method) && - httpContext.Response.StatusCode == 200 && - httpContext.Request.Headers.TryGetString(HeaderNames.IfNoneMatch, out var noneMatch) && - string.Equals(etag, noneMatch, StringComparison.Ordinal)) + if (IsCacheable(resultContext.HttpContext, etag)) { resultContext.Result = new StatusCodeResult(304); } } - cachingManager.Finish(httpContext); + cachingManager.Finish(resultContext.HttpContext); + } + + private static bool IsCacheable(HttpContext httpContext, string etag) + { + return HttpMethods.IsGet(httpContext.Request.Method) && + httpContext.Response.StatusCode == 200 && + httpContext.Request.Headers.TryGetString(HeaderNames.IfNoneMatch, out var noneMatch) && + string.Equals(etag, noneMatch, StringComparison.Ordinal); + } + + private void AppendAuthHeaders(HttpContext httpContext) + { + cachingManager.AddHeader("Auth-State"); + + if (!string.IsNullOrWhiteSpace(httpContext.User.OpenIdSubject())) + { + cachingManager.AddHeader(HeaderNames.Authorization); + } + else if (!string.IsNullOrWhiteSpace(httpContext.User.OpenIdClientId())) + { + cachingManager.AddHeader("Auth-ClientId"); + } + } + + private static string ToWeakEtag(string? etag) + { + return $"W/{etag}"; + } + + private static bool IsWeakEtag(string etag) + { + return !etag.StartsWith("W/", StringComparison.OrdinalIgnoreCase); } } } diff --git a/backend/src/Squidex.Web/Pipeline/CachingManager.cs b/backend/src/Squidex.Web/Pipeline/CachingManager.cs index 95a3f2c60..f6eeec9b3 100644 --- a/backend/src/Squidex.Web/Pipeline/CachingManager.cs +++ b/backend/src/Squidex.Web/Pipeline/CachingManager.cs @@ -24,6 +24,8 @@ namespace Squidex.Web.Pipeline { public sealed class CachingManager : IRequestCache { + public const string SurrogateKeySizeHeader = "X-SurrogateKeys"; + private const int MaxAllowedKeysSize = 20000; private readonly ObjectPool stringBuilderPool; private readonly CachingOptions cachingOptions; private readonly IHttpContextAccessor httpContextAccessor; @@ -34,8 +36,14 @@ namespace Squidex.Web.Pipeline private readonly HashSet keys = new HashSet(); private readonly HashSet headers = new HashSet(); private readonly ReaderWriterLockSlim slimLock = new ReaderWriterLockSlim(); + private readonly int maxKeysSize; private bool hasDependency; + public CacheContext(int maxKeysSize) + { + this.maxKeysSize = maxKeysSize; + } + public void Dispose() { hasher.Dispose(); @@ -87,7 +95,7 @@ namespace Squidex.Web.Pipeline } } - public void Finish(HttpResponse response, int maxSurrogateKeySize, ObjectPool stringBuilderPool) + public void Finish(HttpResponse response, ObjectPool stringBuilderPool) { if (hasDependency && !response.Headers.ContainsKey(HeaderNames.ETag)) { @@ -100,10 +108,8 @@ namespace Squidex.Web.Pipeline } } - if (keys.Count > 0 && maxSurrogateKeySize > 0) + if (keys.Count > 0 && maxKeysSize > 0) { - const int GuidLength = 36; - var stringBuilder = stringBuilderPool.Get(); try { @@ -111,14 +117,14 @@ namespace Squidex.Web.Pipeline { if (stringBuilder.Length == 0) { - if (stringBuilder.Length + GuidLength > maxSurrogateKeySize) + if (stringBuilder.Length + key.Length > maxKeysSize) { break; } } else { - if (stringBuilder.Length + GuidLength + 1 > maxSurrogateKeySize) + if (stringBuilder.Length + key.Length + 1 > maxKeysSize) { break; } @@ -183,7 +189,21 @@ namespace Squidex.Web.Pipeline { Guard.NotNull(httpContext); - httpContext.Features.Set(new CacheContext()); + int maxKeysSize = GetKeysSize(httpContext); + + httpContext.Features.Set(new CacheContext(maxKeysSize)); + } + + private int GetKeysSize(HttpContext httpContext) + { + var headers = httpContext.Request.Headers; + + if (!headers.TryGetValue(SurrogateKeySizeHeader, out var header) || !int.TryParse(header, out int size)) + { + size = cachingOptions.MaxSurrogateKeysSize; + } + + return Math.Min(MaxAllowedKeysSize, size); } public void AddDependency(Guid key, long version) @@ -233,7 +253,7 @@ namespace Squidex.Web.Pipeline if (cacheContext != null) { - cacheContext.Finish(httpContext.Response, cachingOptions.MaxSurrogateKeysSize, stringBuilderPool); + cacheContext.Finish(httpContext.Response, stringBuilderPool); } } } diff --git a/backend/src/Squidex.Web/Pipeline/CachingOptions.cs b/backend/src/Squidex.Web/Pipeline/CachingOptions.cs index 21d68dbcb..780b8d58b 100644 --- a/backend/src/Squidex.Web/Pipeline/CachingOptions.cs +++ b/backend/src/Squidex.Web/Pipeline/CachingOptions.cs @@ -11,6 +11,6 @@ namespace Squidex.Web.Pipeline { public bool StrongETag { get; set; } - public int MaxSurrogateKeysSize { get; set; } = 17000; + public int MaxSurrogateKeysSize { get; set; } = 8000; } } diff --git a/backend/src/Squidex/appsettings.json b/backend/src/Squidex/appsettings.json index 8ae206792..f74ed02af 100644 --- a/backend/src/Squidex/appsettings.json +++ b/backend/src/Squidex/appsettings.json @@ -39,7 +39,7 @@ /* * Restrict the surrogate keys to 17KB. */ - "maxSurrogateKeysSize": 17000 + "maxSurrogateKeysSize": 8000 }, "languages": { diff --git a/backend/tests/Squidex.Web.Tests/Pipeline/CachingFilterTests.cs b/backend/tests/Squidex.Web.Tests/Pipeline/CachingFilterTests.cs index 23a5ce9b0..e44b0e2df 100644 --- a/backend/tests/Squidex.Web.Tests/Pipeline/CachingFilterTests.cs +++ b/backend/tests/Squidex.Web.Tests/Pipeline/CachingFilterTests.cs @@ -266,6 +266,25 @@ namespace Squidex.Web.Pipeline Assert.Equal(StringValues.Empty, httpContext.Response.Headers["Surrogate-Key"]); } + [Fact] + public async Task Should_not_append_surrogate_keys_if_maximum_is_overriden() + { + var id1 = Guid.NewGuid(); + var id2 = Guid.NewGuid(); + + httpContext.Request.Headers[CachingManager.SurrogateKeySizeHeader] = "20"; + + await sut.OnActionExecutionAsync(executingContext, () => + { + cachingManager.AddDependency(id1, 12); + cachingManager.AddDependency(id2, 12); + + return Task.FromResult(executedContext); + }); + + Assert.Equal(StringValues.Empty, httpContext.Response.Headers["Surrogate-Key"]); + } + [Fact] public async Task Should_generate_etag_from_ids_and_versions() {