Skip to content
This repository was archived by the owner on Dec 20, 2018. It is now read-only.

Add Vary: Accept-Encoding if response was compressed #199

Merged
merged 1 commit into from
Jan 20, 2017
Merged

Add Vary: Accept-Encoding if response was compressed #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/Microsoft.AspNetCore.ResponseCompression/BodyWrapperStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,24 @@ private void OnWrite()
_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);
}

var compressionProvider = ResolveCompressionProvider();
if (compressionProvider != null)
{
Expand Down
20 changes: 20 additions & 0 deletions test/Microsoft.AspNetCore.ResponseCompression.Tests/BodyWrapperStreamTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,33 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.Net.Http.Headers;
using Moq;
using Xunit;

namespace Microsoft.AspNetCore.ResponseCompression.Tests
{
public class BodyWrapperStreamTests
{
[Theory]
[InlineData(null, "Accept-Encoding")]
[InlineData("", "Accept-Encoding")]
[InlineData("AnotherHeader", "AnotherHeader,Accept-Encoding")]
[InlineData("Accept-Encoding", "Accept-Encoding")]
[InlineData("accepT-encodinG", "accepT-encodinG")]
[InlineData("accept-encoding,AnotherHeader", "accept-encoding,AnotherHeader")]
public void OnWrite_AppendsAcceptEncodingToVaryHeader_IfNotPresent(string providedVaryHeader, string expectedVaryHeader)
{
var httpContext = new DefaultHttpContext();
httpContext.Response.Headers[HeaderNames.Vary] = providedVaryHeader;
var stream = new BodyWrapperStream(httpContext, new MemoryStream(), new MockResponseCompressionProvider(flushable: true), null, null);

stream.Write(new byte[] { }, 0, 0);


Assert.Equal(expectedVaryHeader, httpContext.Response.Headers[HeaderNames.Vary]);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down
49 changes: 38 additions & 11 deletions test/Microsoft.AspNetCore.ResponseCompression.Tests/ResponseCompressionMiddlewareTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public async Task Request_NoAcceptEncoding_Uncompressed()
{
var response = await InvokeMiddleware(100, requestAcceptEncodings: null, responseType: TextPlain);

CheckResponseNotCompressed(response, expectedBodyLength: 100);
CheckResponseNotCompressed(response, expectedBodyLength: 100, sendVaryHeader: false);
}

[Fact]
Expand All @@ -52,7 +52,7 @@ public async Task Request_AcceptUnknown_NotCompressed()
{
var response = await InvokeMiddleware(100, requestAcceptEncodings: new string[] { "unknown" }, responseType: TextPlain);

CheckResponseNotCompressed(response, expectedBodyLength: 100);
CheckResponseNotCompressed(response, expectedBodyLength: 100, sendVaryHeader: true);
}

[Theory]
Expand Down Expand Up @@ -149,7 +149,7 @@ public async Task MimeTypes_OtherContentTypes_NoMatch(string contentType)

var response = await client.SendAsync(request);

CheckResponseNotCompressed(response, expectedBodyLength: 100);
CheckResponseNotCompressed(response, expectedBodyLength: 100, sendVaryHeader: false);
}

[Theory]
Expand Down Expand Up @@ -185,7 +185,7 @@ public async Task NoBody_NotCompressed(string contentType)

var response = await client.SendAsync(request);

CheckResponseNotCompressed(response, expectedBodyLength: 0);
CheckResponseNotCompressed(response, expectedBodyLength: 0, sendVaryHeader: false);
}

[Fact]
Expand All @@ -201,7 +201,7 @@ public async Task Request_AcceptIdentity_NotCompressed()
{
var response = await InvokeMiddleware(100, requestAcceptEncodings: new string[] { "identity" }, responseType: TextPlain);

CheckResponseNotCompressed(response, expectedBodyLength: 100);
CheckResponseNotCompressed(response, expectedBodyLength: 100, sendVaryHeader: true);
}

[Theory]
Expand All @@ -221,15 +221,15 @@ public async Task Request_AcceptWithhigherIdentityQuality_NotCompressed(string[]
{
var response = await InvokeMiddleware(100, requestAcceptEncodings: acceptEncodings, responseType: TextPlain);

CheckResponseNotCompressed(response, expectedBodyLength: expectedBodyLength);
CheckResponseNotCompressed(response, expectedBodyLength: expectedBodyLength, sendVaryHeader: true);
}

[Fact]
public async Task Response_UnknownMimeType_NotCompressed()
{
var response = await InvokeMiddleware(100, requestAcceptEncodings: new string[] { "gzip" }, responseType: "text/custom");

CheckResponseNotCompressed(response, expectedBodyLength: 100);
CheckResponseNotCompressed(response, expectedBodyLength: 100, sendVaryHeader: false);
}

[Fact]
Expand All @@ -240,7 +240,7 @@ public async Task Response_WithContentRange_NotCompressed()
r.Headers[HeaderNames.ContentRange] = "1-2/*";
});

CheckResponseNotCompressed(response, expectedBodyLength: 50);
CheckResponseNotCompressed(response, expectedBodyLength: 50, sendVaryHeader: false);
}

[Fact]
Expand Down Expand Up @@ -446,7 +446,7 @@ public async Task FlushAsyncBody_CompressesAndFlushes()
request.Headers.AcceptEncoding.ParseAdd("gzip");

var response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);

IEnumerable<string> contentMD5 = null;
Assert.False(response.Headers.TryGetValues(HeaderNames.ContentMD5, out contentMD5));
Assert.Single(response.Content.Headers.ContentEncoding, "gzip");
Expand Down Expand Up @@ -662,7 +662,7 @@ public async Task SendFileAsync_DifferentContentType_NotBypassed()

var response = await client.SendAsync(request);

CheckResponseNotCompressed(response, expectedBodyLength: 1024);
CheckResponseNotCompressed(response, expectedBodyLength: 1024, sendVaryHeader: false);

Assert.True(fakeSendFile.Invoked);
}
Expand Down Expand Up @@ -793,13 +793,40 @@ private void CheckResponseCompressed(HttpResponseMessage response, int expectedB
{
IEnumerable<string> contentMD5 = null;

var containsVaryAcceptEncoding = false;
foreach (var value in response.Headers.GetValues(HeaderNames.Vary))
{
if (value.Contains(HeaderNames.AcceptEncoding))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a Contains vs. an exact match?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or are you trying to avoid parsing the header into multiple segments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case either would work since the tests only set one value to the Vary header but in general when we are dealing with headers, we can't just assume each string in StringValues contains one value. We ran into this problem when we received Header["Cache-Control"] = new[] {"no-cache,no-store"} from MVC for example. So it's best to check using contains rather than exact match.

In the same regard, we also shouldn't be assuming the format of the headers. For example, the test could break with a bad Vary header like Headers["Vary"] = new [] {"badValue=Accept-Encoding"} but I ignored this corner case in the tests.

{
containsVaryAcceptEncoding = true;
break;
}
}
Assert.True(containsVaryAcceptEncoding);
Assert.False(response.Headers.TryGetValues(HeaderNames.ContentMD5, out contentMD5));
Assert.Single(response.Content.Headers.ContentEncoding, "gzip");
Assert.Equal(expectedBodyLength, response.Content.Headers.ContentLength);
}

private void CheckResponseNotCompressed(HttpResponseMessage response, int expectedBodyLength)
private void CheckResponseNotCompressed(HttpResponseMessage response, int expectedBodyLength, bool sendVaryHeader)
{
if (sendVaryHeader)
{
var containsVaryAcceptEncoding = false;
foreach (var value in response.Headers.GetValues(HeaderNames.Vary))
{
if (value.Contains(HeaderNames.AcceptEncoding))
{
containsVaryAcceptEncoding = true;
break;
}
}
Assert.True(containsVaryAcceptEncoding);
}
else
{
Assert.False(response.Headers.Contains(HeaderNames.Vary));
}
Assert.NotNull(response.Headers.GetValues(HeaderNames.ContentMD5));
Assert.Empty(response.Content.Headers.ContentEncoding);
Assert.Equal(expectedBodyLength, response.Content.Headers.ContentLength);
Expand Down