ALWAYS include the `Vary: Accept-Encoding` header when response compression is enabled

[aradalvand]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

Currently, the response compression middleware only adds a Vary: Accept-Encoding header to the response, if and only if the request contains an Accept-Encoding header.

This is wrong. The Vary: Accept-Encoding header should be included in the response regardless of whether or not Accept-Encoding was part of the original request. This is because otherwise, if the very first request is one without the Accept-Encoding header (and by extension there is no Vary: Accept-Encoding in the response), an HTTP cache would not know that if a subsequent request came in that did include an Accept-Encoding header, it should NOT use the previous cache entry to satisfy that request.

The value of the Vary header shouldn't be 'conditional' (for a request to the same URL). That's incorrect behavior and would cause issues especially with proxy caches (e.g. CDNs).

This principle is something that was explicitly pointed out in this article by Fastly — which is also mentioned in the MDN page for Vary.

Expected Behavior

The Vary: Accept-Encoding header should be included in every response whose Content-Type MIME type is supported by the response compression middleware, regardless of the presence of Accept-Encoding in the request.

This same principle also applies to any middleware that might add a value to the Vary header of the response, there could be some others in the ASP.NET Core codebase as well, although the response compression middleware is the one I stumbled into.

The relevant piece of code seems to be the following:

aspnetcore/src/Middleware/ResponseCompression/src/ResponseCompressionBody.cs

Lines 202 to 225 in 86817b0

	private ICompressionProvider? InitializeCompressionHeaders()
	{
	if (_provider.ShouldCompressResponse(_context))
	{
	var headers = _context.Response.Headers;
	// If the MIME type indicates that the response could be compressed, caches will need to vary by the Accept-Encoding header
	var varyValues = headers.GetCommaSeparatedValues(HeaderNames.Vary);
	var varyByAcceptEncoding = false;
	for (var i = 0; i < varyValues.Length; i++)
	{
	if (string.Equals(varyValues[i], HeaderNames.AcceptEncoding, StringComparison.OrdinalIgnoreCase))
	{
	varyByAcceptEncoding = true;
	break;
	}
	}
	if (!varyByAcceptEncoding)
	{
	// Can't use += as StringValues does not override operator+
	// and the implict conversions will cause an incorrect string concat https://github.com/dotnet/runtime/issues/52507
	headers.Vary = StringValues.Concat(headers.Vary, HeaderNames.AcceptEncoding);
	}

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

[aradalvand]

@Tratcher This is actually a bug report rather than a feature request. Is the "feature-middleware" label really appropriate?

[Tratcher]

That label just means it's part of a middleware feature area, response compression.

[adityamandaleeka]

Thanks for reporting this @aradalvand, I just looked at this and you're right that this is a bug.

It looks like we bail in invoking the middleware entirely if the Accept-Encoding header is not set in the request.

aspnetcore/src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs

Lines 37 to 45 in f56c242

	public Task Invoke(HttpContext context)
	{
	if (!_provider.CheckRequestAcceptsCompression(context))
	{
	return _next(context);
	}
	return InvokeCore(context);
	}

aspnetcore/src/Middleware/ResponseCompression/src/ResponseCompressionProvider.cs

Lines 219 to 229 in f56c242

	public bool CheckRequestAcceptsCompression(HttpContext context)
	{
	if (string.IsNullOrEmpty(context.Request.Headers.AcceptEncoding))
	{
	_logger.NoAcceptEncoding();
	return false;
	}
	_logger.RequestAcceptsCompression(); // Trace, there will be more logs
	return true;
	}

We'll need to fix this logic so that the Vary: Accept-Encoding header is included regardless.

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[aradalvand]

Hi @adityamandaleeka, thank you for looking into this.

Note that one important point that's easy to overlook (which I did hint at in my original comment but I think is worth reiterating), is that Vary: Accept-Encoding should be added only to responses with a Content-Type supported by the response compression middleware, and not to every response — but irrespective of whether or not the request includes Accept-Encoding, which is the original bug, of course.

I just wanted to point this out again because I think it's easy to miss and consequently introduce another bug.