Permalink
Browse files

fix issue with fetching and content types

  • Loading branch information...
jeffkaufman committed Mar 28, 2016
1 parent 7cccc26 commit 52e184babd3b4c54734e2a6e25a9b22bac4c56fb
@@ -1059,6 +1059,10 @@ TEST_F(CacheExtenderTest, NoPreserveUrlRelativity) {
"<img src=http://test.com/b.jpg.pagespeed.ce.0.jpg>");
}

TEST_F(CacheExtenderTest, ContentTypeValidation) {
ValidateFallbackHeaderSanitization("ce");
}

} // namespace

} // namespace net_instaweb
@@ -464,13 +464,16 @@ TEST_F(DistributedRewriteContextTest, ReconstructDistributedTwoFilterBlocks) {
new FakeFilter(RewriteOptions::kCssFilterId, rewrite_driver(),
semantic_type::kStylesheet);
fake_css_filter->set_exceed_deadline(true);
fake_css_filter->set_output_content_type(&kContentTypeCss);
FakeFilter* fake_css_combiner =
new FakeFilter(RewriteOptions::kCssCombinerId, rewrite_driver(),
semantic_type::kStylesheet);
fake_css_combiner->set_output_content_type(&kContentTypeCss);
FakeFilter* other_fake_css_filter =
new FakeFilter(RewriteOptions::kCssFilterId, other_rewrite_driver(),
semantic_type::kStylesheet);
other_fake_css_filter->set_exceed_deadline(true);
other_fake_css_filter->set_output_content_type(&kContentTypeCss);
rewrite_driver()->AppendRewriteFilter(fake_css_filter);
rewrite_driver()->AppendRewriteFilter(fake_css_combiner);
other_rewrite_driver()->AppendRewriteFilter(other_fake_css_filter);
@@ -643,6 +643,10 @@ TEST_F(CssImageMultiFilterTest, NoCombineAcrossFlattening) {
ValidateExpected("with_flattening", kBeforeHtml, kAfterHtml);
}

TEST_F(CssImageCombineTest, ContentTypeValidation) {
ValidateFallbackHeaderSanitization("is");
}

} // namespace

} // namespace net_instaweb
@@ -1404,6 +1404,10 @@ TEST_P(JavascriptFilterTest, ExternalAndNotInline) {
expected_rewritten_path_.c_str())));
}

TEST_P(JavascriptFilterTest, ContentTypeValidation) {
ValidateFallbackHeaderSanitization(kFilterId);
}

// We test with use_experimental_minifier == GetParam() as both true and false.
INSTANTIATE_TEST_CASE_P(JavascriptFilterTestInstance, JavascriptFilterTest,
::testing::Bool());
@@ -429,6 +429,10 @@ class RewriteTestBase : public RewriteOptionsTestBase {
const StringPiece& rewritten_name,
const StringPiece& rewritten_content);

// Check that when we have a cache miss for a pagespeed resource we set
// headers to reduce the chance of it being interpreted as html.
void ValidateFallbackHeaderSanitization(StringPiece filter_id);

TestRewriteDriverFactory* factory() { return factory_.get(); }
TestRewriteDriverFactory* other_factory() { return other_factory_.get(); }

@@ -826,6 +830,10 @@ class RewriteTestBase : public RewriteOptionsTestBase {
uint64 expected_nonce_;

GoogleString debug_message_; // Message used by DebugMessage

private:
void ValidateFallbackHeaderSanitizationHelper(
StringPiece filter_id, StringPiece origin_content_type, bool expect_load);
};

} // namespace net_instaweb
@@ -3181,6 +3181,10 @@ void RewriteContext::FixFetchFallbackHeaders(
ttl_ms = std::min(ttl_ms, headers->implicit_cache_ttl_ms());
}
headers->SetDateAndCaching(date_ms, ttl_ms, cache_control_suffix);
// Replace, as in "add if not already present". The only valid value for this
// header is "nosniff", so we don't have to worry about clobbering existing
// usage.
headers->Replace("X-Content-Type-Options", "nosniff");

// TODO(jmarantz): Use the actual content-hash to replace the W/"0" etag
// rather than removing the etag altogether. This requires adding code to
@@ -3198,8 +3202,29 @@ bool RewriteContext::SendFallbackResponse(StringPiece output_url_base,
StringPiece contents,
AsyncFetch* async_fetch,
MessageHandler* handler) {
const ContentType* content_type =
async_fetch->response_headers()->DetermineContentType();
if (content_type == NULL ||
!(content_type->IsJs() ||
content_type->IsCss() ||
content_type->IsImage() ||
content_type == &kContentTypePdf)) {
// If the content type header isn't one that we would generate a pagespeed
// resource for, fail the request. This is a security measure that limits
// people's ability to get us to pass html.

handler->Message(
kInfo, "Dropping response for %s for disallowed origin content type %s",
output_url_base.as_string().c_str(),
(content_type == NULL ? "[missing or unrecognized]"
: content_type->mime_type()));

return false;
}

async_fetch->set_content_length(contents.size());
async_fetch->HeadersComplete();

return async_fetch->Write(contents, handler);
}

@@ -1459,6 +1459,7 @@ TEST_F(RewriteContextTest, TestRewritesOnEmptyPrivateResources) {
no_store_css_header.set_minor_version(1);
no_store_css_header.SetStatusAndReason(HttpStatus::kOK);
no_store_css_header.SetDateAndCaching(now_ms, 0, ",no-store");
no_store_css_header.Add(HttpAttributes::kContentType, "text/css");
no_store_css_header.ComputeCaching();

SetFetchResponse(AbsolutifyUrl(kPath), no_store_css_header, "");
@@ -374,6 +374,7 @@ void RewriteContextTestBase::InitResourcesToDomain(const char* domain) {
SetDefaultLongCacheHeaders(&kContentTypeCss, &low_ttl_css_header);
low_ttl_css_header.SetDateAndCaching(now_ms, kLowOriginTtlMs);
low_ttl_css_header.ComputeCaching();
low_ttl_css_header.Add(HttpAttributes::kContentType, "text/css");
SetFetchResponse(StrCat(domain, "d.css"), low_ttl_css_header, "d");

// trimmable, low ttl.
@@ -392,6 +393,7 @@ void RewriteContextTestBase::InitResourcesToDomain(const char* domain) {
private_css_header.set_minor_version(1);
private_css_header.SetStatusAndReason(HttpStatus::kOK);
private_css_header.SetDateAndCaching(now_ms, kOriginTtlMs, ",private");
private_css_header.Add(HttpAttributes::kContentType, "text/css");
private_css_header.ComputeCaching();

SetFetchResponse(StrCat(domain, "a_private.css"),
@@ -404,6 +406,7 @@ void RewriteContextTestBase::InitResourcesToDomain(const char* domain) {
no_cache_css_header.set_minor_version(1);
no_cache_css_header.SetStatusAndReason(HttpStatus::kOK);
no_cache_css_header.SetDateAndCaching(now_ms, 0, ",no-cache");
no_cache_css_header.Add(HttpAttributes::kContentType, "text/css");
no_cache_css_header.ComputeCaching();

SetFetchResponse(StrCat(domain, "a_no_cache.css"),
@@ -417,6 +420,7 @@ void RewriteContextTestBase::InitResourcesToDomain(const char* domain) {
no_transform_css_header.SetStatusAndReason(HttpStatus::kOK);
no_transform_css_header.SetDateAndCaching(now_ms, kOriginTtlMs,
",no-transform");
no_transform_css_header.Add(HttpAttributes::kContentType, "text/css");
no_transform_css_header.ComputeCaching();

SetFetchResponse(StrCat(domain, "a_no_transform.css"),
@@ -429,6 +433,7 @@ void RewriteContextTestBase::InitResourcesToDomain(const char* domain) {
no_store_css_header.set_minor_version(1);
no_store_css_header.SetStatusAndReason(HttpStatus::kOK);
no_store_css_header.SetDateAndCaching(now_ms, 0, ",no-cache,no-store");
no_store_css_header.Add(HttpAttributes::kContentType, "text/css");
no_store_css_header.ComputeCaching();

SetFetchResponse(StrCat(domain, "a_no_store.css"),
@@ -235,7 +235,7 @@ class RewriteSingleResourceFilterTest
options()->ComputeSignature();

MockResource("a.tst", "good", TtlSec());
MockResource("bad.tst", "bad", TtlSec());
SetResponseWithDefaultHeaders("bad.tst", kContentTypeCss, "bad", TtlSec());
MockResource("busy.tst", "$", TtlSec());
MockMissingResource("404.tst");

@@ -634,6 +634,109 @@ void RewriteTestBase::TestServeFiles(
}
}

void RewriteTestBase::ValidateFallbackHeaderSanitizationHelper(
StringPiece filter_id, StringPiece origin_content_type, bool expect_load) {

// Mangle the content type to make a url name by removing '/'s.
GoogleString leafable(origin_content_type.as_string());
GlobalReplaceSubstring("/", "-", &leafable);

GoogleString leaf = StrCat("leaf-", leafable);
GoogleString origin_contents = "this isn't a real file";

ResponseHeaders origin_response_headers;
origin_response_headers.set_major_version(1);
origin_response_headers.set_minor_version(1);
origin_response_headers.SetStatusAndReason(HttpStatus::kOK);
origin_response_headers.Add(HttpAttributes::kContentType,
origin_content_type);

int64 now_ms = timer()->NowMs();
// This is a case where we do need to make some changes for security and we
// want to be sure we make them even if no-transform is set.
origin_response_headers.SetDateAndCaching(now_ms, 0 /* ttl */,
"; no-transform");
origin_response_headers.ComputeCaching();

SetFetchResponse(
AbsolutifyUrl(leaf), origin_response_headers, origin_contents);

GoogleString resource = AbsolutifyUrl(Encode(
"", filter_id, "0", leaf, "ignored"));

GoogleString response_content;
ResponseHeaders response_headers;

if (expect_load) {
ASSERT_TRUE(FetchResourceUrl(resource,
NULL /* use default request headers */,
&response_content,
&response_headers));
EXPECT_EQ(origin_contents, response_content);
const ContentType* content_type = response_headers.DetermineContentType();
ASSERT_TRUE(NULL != content_type);
EXPECT_EQ(origin_content_type, content_type->mime_type());

const char* nosniff = response_headers.Lookup1("X-Content-Type-Options");
ASSERT_TRUE(NULL != nosniff);
EXPECT_EQ("nosniff", GoogleString(nosniff));
} else {
ASSERT_FALSE(FetchResourceUrl(resource,
NULL /* use default request headers */,
&response_content,
&response_headers));
}
}


void RewriteTestBase::ValidateFallbackHeaderSanitization(
StringPiece filter_id) {
// Freeze our options.
server_context()->ComputeSignature(options());

// These content types will all be preserved.
ValidateFallbackHeaderSanitizationHelper(
filter_id, "text/css", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "text/javascript", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "application/javascript", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/jpg", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/jpeg", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/png", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/gif", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/webp", true);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "application/pdf", true);

// All other content types will be stripped.
ValidateFallbackHeaderSanitizationHelper(
filter_id, "text/html", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "text/plain", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "text/xml", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "application/xml", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/svg", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "image/svg+xml", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "audio/mp3", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "video/mp4", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "", false);
ValidateFallbackHeaderSanitizationHelper(
filter_id, "invalid", false);
}

// Just check if we can fetch a resource successfully, ignore response.
bool RewriteTestBase::TryFetchResource(const StringPiece& url) {
GoogleString contents;
@@ -608,6 +608,7 @@ TEST_F(ProxyInterfaceTest, HeadResourceRequest) {
GoogleString expected_response_headers_string = "HTTP/1.1 200 OK\r\n"
"Content-Type: text/css\r\n"
"X-Background-Fetch: 0\r\n"
"X-Content-Type-Options: nosniff\r\n"
"Date: Tue, 02 Feb 2010 18:51:26 GMT\r\n"
"Expires: Tue, 02 Feb 2010 18:56:26 GMT\r\n"
"Cache-Control: max-age=300,private\r\n"
@@ -650,6 +651,7 @@ TEST_F(ProxyInterfaceTest, HeadResourceRequest) {
expected_response_headers_string = "HTTP/1.1 200 OK\r\n"
"Content-Type: text/css\r\n"
"X-Background-Fetch: 0\r\n"
"X-Content-Type-Options: nosniff\r\n"
"Date: Tue, 02 Feb 2010 18:51:26 GMT\r\n"
"Expires: Tue, 02 Feb 2010 18:56:26 GMT\r\n"
"Cache-Control: max-age=300,private\r\n"
@@ -91,6 +91,7 @@ fi
run_test content_length
run_test keep_data_urls
run_test rel_canonical
run_test resource_content_type_html

wait_for_async_tests

@@ -0,0 +1,67 @@
function verify_nosniff {
leaf="$1"
content_type="$2"
URL=$REWRITTEN_ROOT/$leaf
OUT=$($CURL -D- -o/dev/null -sS "$URL")
check_from "$OUT" grep '^HTTP.* 200 OK'
check_from "$OUT" grep '^Content-Type: '"$content_type"''
check_from "$OUT" grep '^X-Content-Type-Options: nosniff'
}

function verify_404 {
leaf="$1"
URL=$REWRITTEN_ROOT/$leaf
OUT=$($CURL -D- -o/dev/null -sS "$URL")
check_from "$OUT" grep '^HTTP.* 404 Not Found'
}

# test that all the filters do fine with one of our content types
start_test js minification css
verify_nosniff styles/big.css.pagespeed.jm.0.foo text/css

start_test image spriting css
verify_nosniff styles/big.css.pagespeed.is.0.foo text/css

start_test image compression css
verify_nosniff styles/xbig.css.pagespeed.ic.0.foo text/css

start_test cache extension css
verify_nosniff styles/big.css.pagespeed.ce.0.foo text/css


# test that we also do fine with the other content types we generate
start_test js minification js
verify_nosniff rewrite_javascript.js.pagespeed.jm.0.foo \
"application/\(x-\)\?javascript"

start_test js minification png
verify_nosniff images/Cuppa.png.pagespeed.jm.0.foo image/png

start_test js minification gif
verify_nosniff images/IronChef2.gif.pagespeed.jm.0.foo image/gif

start_test js minification jpg
verify_nosniff images/Puzzle.jpg.pagespeed.jm.0.foo image/jpeg

start_test js minification pdf
verify_nosniff example.pdf.pagespeed.jm.0.foo application/pdf


# test that we 404 html
start_test js minification html
verify_404 index.html.pagespeed.jm.0.foo

start_test image spriting html
verify_404 index.html.pagespeed.is.0.foo

start_test image compression html
verify_404 xindex.html.pagespeed.ic.0.foo

start_test cache extension html
verify_404 index.html.pagespeed.ce.0.foo


# test that we 404 svgs too
start_test js minification svg
verify_404 images/schedule_event.svg.pagespeed.jm.0.foo

@@ -52,7 +52,9 @@ const ContentType kTypes[] = {
{"application/javascript", ".map", ContentType::kSourceMap},
{"application/pdf", ".pdf", ContentType::kPdf}, // RFC 3778
{"application/octet-stream", ".bin", ContentType::kOctetStream },
// SVG is XML.
// SVG is XML. If at some point we start optimizing svg as images, we need to
// be very careful not to just lump them in with images for all purposes, to
// avoid creating security vulnerabilities.
{"image/svg+xml", ".svg", ContentType::kXml},

// Synonyms; Note that the canonical types above are referenced by index
Oops, something went wrong.

0 comments on commit 52e184b

Please sign in to comment.