Should HTTP Vary header always be added to every compressible request passing through the Response Compression middleware?

[tiakun]

I recently ran into a "problem" when I put Response Caching middleware and Response Compression middleware together. The problem is that ASP.NET Response Caching middleware return uncompressed cached response despite of Accept-Encoding request header. While this does not violate HTTP protocol, as uncompressed response is valid for any Accept-Endoing values, it is sub-optimal.

My setup is that, in order to get my response compressed before getting cached, I register Response Compression middleware behind Response Caching middleware. This works fine when Accept-Encoding presents for all requests.

However, when I make a request without Accept-Encoding header, my compressed cached response gets overridden by uncompressed one. All the subsequent requests will get uncompressed response no matter what.

I am not very sure whether this is a bug or not, but I think all compressible responses passing through Response Compression also warrants the Vary header even it is uncompressed. "Compressible" here means there might be some chance that the response might be compressed by some request.

Here is the documentation

ASP.NET Core 2.0 or later, the middleware adds the Vary header automatically when the response is compressed.

While the middleware works as documented, I don't think the Vary header should be omitted for uncompressed response.

My workaround right now is that I put Vary: Accept-Encoding to every cachable response.

[Tratcher]

That would make sense if all the compression criteria were met except for the Accept-Encoding. You wouldn't want to add Vary to a response that wouldn't otherwise be compressible.

With the current abreactions it might be tricky to make that granular distinction. Are you willing to do some experimenting?

[Tratcher]

Yes, this was an oversight on my part.

[tiakun]

I checked the code and tried to think how to checked whether response is compressible or not, but all we have is URL and the metadata explicitly defined by the developer and we cannot rely on generated back-end response to mark any URL compressible because we need to maintain consistency among the requests of each specific URL. I don't think checking if the response is compressible is a way to go then.

Anyway, I think one thing we can fix to make things work in the scenario I described is that we just add Vary: Accept-Encoding for the requests that doesn't pass IResponseCompressionProvider.CheckRequestAcceptsCompression check. This way we can keep all abstractions intact and introduce only minimal changes to both the code base and the functionality. Do you think it's a good solution?

[Tratcher]

Oh I had meant that as experimenting inside the middleware, not your code. The middleware knows what is compressible. Adding the vary header for non compressible responses would be inappropriate.

Looking at the code, the first issue is that the middleware completely no-ops if there's no Accept-Encoding header. This is for performance reasons, substituting the response body structures would cause non trivial overhead. But without that the middleware doesn't get to participate in the response at all.

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

Line 48 in f490dd2

if (!_provider.CheckRequestAcceptsCompression(context))

We'd need to find a lightweight way to participate in responses that fail CheckRequestAcceptsCompression so we can still call ShouldCompressResponse and add the vary header.

[tiakun]

Please correct me if I'm wrong, but I mean we can just add Vary header right after there, something like

if (!_provider.CheckRequestAcceptsCompression(context))
{
    await _next(context);
    context.Response.Headers[HeaderNames.Vary] = "Accept-Encoding"; // needs more sophisticated logic here though
    return;
}

I'm not sure if this is possible or not.

[Tratcher]

That's too soon, that header shouldn't be added without first checking if the response is compressible, and that check doesn't happen until much later.

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

Lines 234 to 257 in f490dd2

	private void OnWrite()
	{
	if (!_compressionChecked)
	{
	_compressionChecked = true;
	if (_provider.ShouldCompressResponse(_context))
	{
	// If the MIME type indicates that the response could be compressed, caches will need to vary by the Accept-Encoding header
	var varyValues = _context.Response.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)
	{
	_context.Response.Headers.Append(HeaderNames.Vary, HeaderNames.AcceptEncoding);
	}

Plugging in a custom Stream just to set the vary header is a lot of overhead for responses where we know compression won't be used.