Commit e0911d27 authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

Do not re-check subresource integrity on successful revalidation

In successful revalidation,
- The SRI check result before the revalidation can be reused
- Resource::Data() can be already null and thus we shouldn't check
  SRI again.

This CL
- Skips SRI checks if |integrity_disposition_| is already set
  (i.e. set before revalidation) to avoid crashing, and
- Clears |integrity_disposition_| on failed revalidation to ensure
  correctness.

Bug: 752830
Change-Id: I482cad4e6e0daa0c078f8fc5fe9f6c30526105dc
Reviewed-on: https://chromium-review.googlesource.com/656297Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarDaniel Vogelheim <vogelheim@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501573}
parent 0ff18909
<?
// Returns response headers that cause revalidation, and for revalidating
// requests returns 200 and a body different from the original one.
header('ETag: foo');
header('Cache-control: max-age=0');
if ($_SERVER['HTTP_IF_NONE_MATCH'] == 'foo') {
// The body after revalidation.
echo "/* after revalidation */";
exit;
}
// The body before revalidation.
echo "/* comment */";
<!DOCTYPE html>
<html>
<head>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
</head>
<script>
var t_css = async_test(
"SRI with CSS with non-empty body after revalidation");
function revalidate_css() {
var link = document.createElement('link');
link.setAttribute('rel', 'stylesheet');
link.integrity="sha256-YsI40D9FX0QghiYVdxQyySP2TOmARkLC5uPRO8RL2dE=";
link.onload = t_css.unreached_func('Second request should fail');
link.onerror = t_css.step_func_done();
link.href =
"../../cache/resources/etag-200-different.php?empty-after-revalidate-css";
document.head.appendChild(link);
}
</script>
<link
href="../../cache/resources/etag-200-different.php?empty-after-revalidate-css"
rel="stylesheet"
integrity="sha256-YsI40D9FX0QghiYVdxQyySP2TOmARkLC5uPRO8RL2dE="
onload="t_css.step_timeout(revalidate_css, 0)"
onerror="t_css.unreached_func('First request should pass')()"
>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
</head>
<script>
var t_script = async_test(
"SRI with script with non-empty body after failed revalidation");
function revalidate_script() {
var script = document.createElement('script');
script.integrity = "sha256-YsI40D9FX0QghiYVdxQyySP2TOmARkLC5uPRO8RL2dE=";
script.onload = t_script.unreached_func('Second request should fail');
script.onerror = t_script.step_func_done();
script.src =
"../../cache/resources/etag-200-different.php?revalidation-failed-script";
document.head.appendChild(script);
}
</script>
<script
src="../../cache/resources/etag-200-different.php?revalidation-failed-script"
type="text/javascript"
integrity="sha256-YsI40D9FX0QghiYVdxQyySP2TOmARkLC5uPRO8RL2dE="
onload="t_script.step_timeout(revalidate_script, 0)"
onerror="t_script.unreached_func('First request should pass')()"
></script>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
</head>
<script>
var t_css = async_test(
"SRI with CSS with successful revalidation shouldn't crash");
function revalidate_css() {
var link = document.createElement('link');
link.setAttribute('rel', 'stylesheet');
link.integrity="sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw=";
link.onload = t_css.step_func_done();
link.onerror = t_css.unreached_func('Second request should pass');
link.href =
"../../cache/resources/etag.php?empty-after-revalidate-css";
document.head.appendChild(link);
}
</script>
<link
href="../../cache/resources/etag.php?empty-after-revalidate-css"
rel="stylesheet"
integrity="sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw="
onload="t_css.step_timeout(revalidate_css, 0)"
onerror="t_css.unreached_func('First request should pass')()"
>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
</head>
<script>
var foo;
var t_script = async_test(
"SRI with script with successful revalidation shouldn't crash");
function revalidate_script() {
var script = document.createElement('script');
script.integrity = "sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw=";
script.onload = t_script.step_func_done();
script.onerror = t_script.unreached_func('Second request should pass');
script.src =
"../../cache/resources/etag.php?revalidation-failed-script";
document.head.appendChild(script);
}
</script>
<script
src="../../cache/resources/etag.php?revalidation-failed-script"
type="text/javascript"
integrity="sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw="
onload="t_script.step_timeout(revalidate_script, 0)"
onerror="t_script.unreached_func('First request should pass')()"
></script>
</html>
......@@ -325,6 +325,11 @@ void Resource::SetLoader(ResourceLoader* loader) {
}
void Resource::CheckResourceIntegrity() {
// Skip the check and reuse the previous check result, especially on
// successful revalidation.
if (IntegrityDisposition() != ResourceIntegrityDisposition::kNotChecked)
return;
// Loading error occurred? Then result is uncheckable.
integrity_report_info_.Clear();
if (ErrorOccurred()) {
......@@ -1070,6 +1075,8 @@ void Resource::RevalidationFailed() {
SECURITY_CHECK(redirect_chain_.IsEmpty());
ClearData();
cache_handler_.Clear();
integrity_disposition_ = ResourceIntegrityDisposition::kNotChecked;
integrity_report_info_.Clear();
DestroyDecodedDataForFailedRevalidation();
is_revalidating_ = false;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment