Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
caching: don't respect vary when cache-extending either
Browse files Browse the repository at this point in the history
By default we don't respect Vary headers we find on resources.  People who do want us to respect them can set 'RespectVary on' in their configuration.  The cache-extender was not considering the value of the RespectVary option in deciding whether things were cachable, and was instead assuming RespectVary=true.

Fixes #1373
  • Loading branch information
jeffkaufman authored and crowell committed Aug 25, 2016
1 parent f63cc12 commit 98e17e8
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
10 changes: 8 additions & 2 deletions net/instaweb/rewriter/cache_extender.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,16 +303,22 @@ RewriteResult CacheExtender::RewriteLoadedResource(
// See if the resource is cacheable; and if so whether there is any need
// to cache extend it.
bool ok = false;
// Assume that it may have cookies; see comment in
// CacheableResourceBase::IsValidAndCacheableImpl.
RequestHeaders::Properties req_properties;
const ContentType* output_type = NULL;
if (!server_context()->http_cache()->force_caching() &&
!headers->IsProxyCacheable()) {
!headers->IsProxyCacheable(
req_properties,
ResponseHeaders::GetVaryOption(driver()->options()->respect_vary()),
ResponseHeaders::kNoValidator)) {
// Note: RewriteContextTest.PreserveNoCacheWithFailedRewrites
// relies on CacheExtender failing rewrites in this case.
// If you change this behavior that test MUST be updated as it covers
// security.
not_cacheable_count_->Add(1);
} else if (ShouldRewriteResource(
headers, now_ms, input_resource,url, result)) {
headers, now_ms, input_resource, url, result)) {
// We must be careful what Content-Types we allow to be cache extended.
// Specifically, we do not want to cache extend any Content-Types that
// could execute scripts when loaded in a browser because that could
Expand Down
16 changes: 16 additions & 0 deletions net/instaweb/rewriter/cache_extender_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,22 @@ TEST_F(CacheExtenderTest, SrcSet) {
a_url, " 1x, ", b_url, " 2x\">"));
}

TEST_F(CacheExtenderTest, VaryOrigin) {
options()->EnableExtendCacheFilters();
rewrite_driver()->AddFilters();

GoogleString url = AbsolutifyUrl("a.css");
ResponseHeaders response_headers;
DefaultResponseHeaders(kContentTypeCss, 100 /* ttl */, &response_headers);
response_headers.Add("Vary", "Origin");
SetFetchResponse(url, response_headers, "css file");

GoogleString cache_extended_css = Encode("", kFilterId, "0", "a.css", "css");
ValidateExpected("vary origin",
StringPrintf(kCssFormat, "a.css"),
StringPrintf(kCssFormat, cache_extended_css.c_str()));
}

} // namespace

} // namespace net_instaweb

0 comments on commit 98e17e8

Please sign in to comment.