Aspnetcore: VaryByQueryKeys should be implcit.

Created on 7 Dec 2018  路  1Comment  路  Source: dotnet/aspnetcore

The default behavior of a browser is to cache by query as well as url. Response caching should match this behavior so there is not confusion. In addition, according to the documentation, specifying VaryByQueryKeys will cause an exception if ResponseCaching is not enabled. This is problematic if you simply want to 'turn it off' temporarily in the Startup.

Major Problem

If you have a project that already has ResponseCache attributes applied for headers, enabling it breaks any existing routes that rely on query string values.

Solution

Please make VaryByQueryKeys = new []{"*"} by default so that it does not produce unexpected behavior when adding response caching to an existing project that is using RequestCache attributes for headers only.

Alternative

Some addition to ResponseCachingOptions to allow for this default.

affected-very-few area-middleware bug feature-ResponseCaching severity-nice-to-have

Most helpful comment

Yes this is awful and potentially very dangerous!

The sample code for response caching middleware has the following:

app.UseResponseCaching();

    app.Use(async (context, next) =>
    {
        context.Response.GetTypedHeaders().CacheControl = 
            new Microsoft.Net.Http.Headers.CacheControlHeaderValue()
            {
                Public = true,
                MaxAge = TimeSpan.FromSeconds(10)
            };
        context.Response.Headers[Microsoft.Net.Http.Headers.HeaderNames.Vary] = 
            new string[] { "Accept-Encoding" };

        await next();
    });

This will not very by query params, and given that browsers often send Max-Age: 0 when you just type in a URL it can be difficult to spot that your data is being cached (especially with a time of only 10 seconds).

Later in this same article it discusses the VaryByQueryKeys option - but doesn't combine the two approaches. This is what I did to achieve that:

        app.UseResponseCaching();

        app.Use(async (context, next) =>
        {
            // https://docs.microsoft.com/en-us/aspnet/core/performance/caching/middleware?view=aspnetcore-2.1
            context.Response.GetTypedHeaders().CacheControl =

                new CacheControlHeaderValue()
                {
                    Public = context.Request.Method == "GET",
                    MaxAge = TimeSpan.FromSeconds(12)
                };

            var responseCachingFeature = context.Features.Get<IResponseCachingFeature>();

            if (responseCachingFeature != null)
            {
                responseCachingFeature.VaryByQueryKeys = new[] { "*" };
            }

            context.Response.Headers[HeaderNames.Vary] = new string[] { "Accept-Encoding" };

            await next();
        });

>All comments

Yes this is awful and potentially very dangerous!

The sample code for response caching middleware has the following:

app.UseResponseCaching();

    app.Use(async (context, next) =>
    {
        context.Response.GetTypedHeaders().CacheControl = 
            new Microsoft.Net.Http.Headers.CacheControlHeaderValue()
            {
                Public = true,
                MaxAge = TimeSpan.FromSeconds(10)
            };
        context.Response.Headers[Microsoft.Net.Http.Headers.HeaderNames.Vary] = 
            new string[] { "Accept-Encoding" };

        await next();
    });

This will not very by query params, and given that browsers often send Max-Age: 0 when you just type in a URL it can be difficult to spot that your data is being cached (especially with a time of only 10 seconds).

Later in this same article it discusses the VaryByQueryKeys option - but doesn't combine the two approaches. This is what I did to achieve that:

        app.UseResponseCaching();

        app.Use(async (context, next) =>
        {
            // https://docs.microsoft.com/en-us/aspnet/core/performance/caching/middleware?view=aspnetcore-2.1
            context.Response.GetTypedHeaders().CacheControl =

                new CacheControlHeaderValue()
                {
                    Public = context.Request.Method == "GET",
                    MaxAge = TimeSpan.FromSeconds(12)
                };

            var responseCachingFeature = context.Features.Get<IResponseCachingFeature>();

            if (responseCachingFeature != null)
            {
                responseCachingFeature.VaryByQueryKeys = new[] { "*" };
            }

            context.Response.Headers[HeaderNames.Vary] = new string[] { "Accept-Encoding" };

            await next();
        });
Was this page helpful?
0 / 5 - 0 ratings

Related issues

guardrex picture guardrex  路  3Comments

farhadibehnam picture farhadibehnam  路  3Comments

markrendle picture markrendle  路  3Comments

rbanks54 picture rbanks54  路  3Comments

ermithun picture ermithun  路  3Comments