Commit ff1c866d authored by Daniel Vogelheim's avatar Daniel Vogelheim Committed by Commit Bot

Tolerate multiple ParseIntegrityAttribute calls for HTMLPreloadScanner.

HTMLPreloadScanner is written to be simple and fast.
SubresourceIntegrity::ParseIntegrtiyAttribute assumes that the caller does
error checking, and won't call it twice for the same element. Reconcile the
two by adding a relaxed version, TryParseIntegrityAttribute.

Bug: 818385
Change-Id: I722fb80c283249941e8db14a8efc9af392bc1cbc
Reviewed-on: https://chromium-review.googlesource.com/948848
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: default avatarYoav Weiss <yoav@yoav.ws>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542891}
parent 50d1e0c1
...@@ -135,6 +135,7 @@ class TokenPreloadScanner::StartTagScanner { ...@@ -135,6 +135,7 @@ class TokenPreloadScanner::StartTagScanner {
media_values_(media_values), media_values_(media_values),
referrer_policy_set_(false), referrer_policy_set_(false),
referrer_policy_(kReferrerPolicyDefault), referrer_policy_(kReferrerPolicyDefault),
integrity_attr_set_(false),
integrity_features_(features), integrity_features_(features),
scanner_type_(scanner_type) { scanner_type_(scanner_type) {
if (Match(tag_impl_, imgTag) || Match(tag_impl_, sourceTag)) { if (Match(tag_impl_, imgTag) || Match(tag_impl_, sourceTag)) {
...@@ -291,7 +292,8 @@ class TokenPreloadScanner::StartTagScanner { ...@@ -291,7 +292,8 @@ class TokenPreloadScanner::StartTagScanner {
SetDefer(FetchParameters::kLazyLoad); SetDefer(FetchParameters::kLazyLoad);
} else if (Match(attribute_name, deferAttr)) { } else if (Match(attribute_name, deferAttr)) {
SetDefer(FetchParameters::kLazyLoad); SetDefer(FetchParameters::kLazyLoad);
} else if (Match(attribute_name, integrityAttr)) { } else if (!integrity_attr_set_ && Match(attribute_name, integrityAttr)) {
integrity_attr_set_ = true;
SubresourceIntegrity::ParseIntegrityAttribute( SubresourceIntegrity::ParseIntegrityAttribute(
attribute_value, integrity_features_, integrity_metadata_); attribute_value, integrity_features_, integrity_metadata_);
} else if (Match(attribute_name, typeAttr)) { } else if (Match(attribute_name, typeAttr)) {
...@@ -369,7 +371,8 @@ class TokenPreloadScanner::StartTagScanner { ...@@ -369,7 +371,8 @@ class TokenPreloadScanner::StartTagScanner {
SecurityPolicy::ReferrerPolicyFromString( SecurityPolicy::ReferrerPolicyFromString(
attribute_value, kDoNotSupportReferrerPolicyLegacyKeywords, attribute_value, kDoNotSupportReferrerPolicyLegacyKeywords,
&referrer_policy_); &referrer_policy_);
} else if (Match(attribute_name, integrityAttr)) { } else if (!integrity_attr_set_ && Match(attribute_name, integrityAttr)) {
integrity_attr_set_ = true;
SubresourceIntegrity::ParseIntegrityAttribute( SubresourceIntegrity::ParseIntegrityAttribute(
attribute_value, integrity_features_, integrity_metadata_); attribute_value, integrity_features_, integrity_metadata_);
} else if (Match(attribute_name, srcsetAttr) && } else if (Match(attribute_name, srcsetAttr) &&
...@@ -594,6 +597,7 @@ class TokenPreloadScanner::StartTagScanner { ...@@ -594,6 +597,7 @@ class TokenPreloadScanner::StartTagScanner {
Member<MediaValuesCached> media_values_; Member<MediaValuesCached> media_values_;
bool referrer_policy_set_; bool referrer_policy_set_;
ReferrerPolicy referrer_policy_; ReferrerPolicy referrer_policy_;
bool integrity_attr_set_;
IntegrityMetadataSet integrity_metadata_; IntegrityMetadataSet integrity_metadata_;
SubresourceIntegrity::IntegrityFeatures integrity_features_; SubresourceIntegrity::IntegrityFeatures integrity_features_;
TokenPreloadScanner::ScannerType scanner_type_; TokenPreloadScanner::ScannerType scanner_type_;
......
...@@ -74,6 +74,11 @@ struct ContextTestCase { ...@@ -74,6 +74,11 @@ struct ContextTestCase {
bool is_image_set; bool is_image_set;
}; };
struct IntegrityTestCase {
size_t number_of_integrity_metadata_found;
const char* input_html;
};
class HTMLMockHTMLResourcePreloader : public ResourcePreloader { class HTMLMockHTMLResourcePreloader : public ResourcePreloader {
public: public:
void PreloadRequestVerification(Resource::Type type, void PreloadRequestVerification(Resource::Type type,
...@@ -166,6 +171,14 @@ class HTMLMockHTMLResourcePreloader : public ResourcePreloader { ...@@ -166,6 +171,14 @@ class HTMLMockHTMLResourcePreloader : public ResourcePreloader {
EXPECT_EQ(preload_request_->IsImageSetForTestingOnly(), is_image_set); EXPECT_EQ(preload_request_->IsImageSetForTestingOnly(), is_image_set);
} }
void CheckNumberOfIntegrityConstraints(size_t expected) {
size_t actual = 0;
if (preload_request_) {
actual = preload_request_->IntegrityMetadataForTestingOnly().size();
EXPECT_EQ(expected, actual);
}
}
protected: protected:
void Preload(std::unique_ptr<PreloadRequest> preload_request, void Preload(std::unique_ptr<PreloadRequest> preload_request,
const NetworkHintsInterface&) override { const NetworkHintsInterface&) override {
...@@ -307,6 +320,18 @@ class HTMLPreloadScannerTest : public PageTestBase { ...@@ -307,6 +320,18 @@ class HTMLPreloadScannerTest : public PageTestBase {
preloader.ContextVerification(test_case.is_image_set); preloader.ContextVerification(test_case.is_image_set);
} }
void Test(IntegrityTestCase test_case) {
SCOPED_TRACE(test_case.input_html);
HTMLMockHTMLResourcePreloader preloader;
KURL base_url("http://example.test/");
scanner_->AppendToEnd(String(test_case.input_html));
PreloadRequestStream requests = scanner_->Scan(base_url, nullptr);
preloader.TakeAndPreload(requests);
preloader.CheckNumberOfIntegrityConstraints(
test_case.number_of_integrity_metadata_found);
}
private: private:
std::unique_ptr<HTMLPreloadScanner> scanner_; std::unique_ptr<HTMLPreloadScanner> scanner_;
}; };
...@@ -1076,4 +1101,28 @@ TEST_F(HTMLPreloadScannerTest, ReferrerHeader) { ...@@ -1076,4 +1101,28 @@ TEST_F(HTMLPreloadScannerTest, ReferrerHeader) {
Test(test_case); Test(test_case);
} }
TEST_F(HTMLPreloadScannerTest, Integrity) {
IntegrityTestCase test_cases[] = {
{0, "<script src=bla.js>"},
{1,
"<script src=bla.js "
"integrity=sha256-qznLcsROx4GACP2dm0UCKCzCG+HiZ1guq6ZZDob/Tng=>"},
{0, "<script src=bla.js integrity=sha257-XXX>"},
{2,
"<script src=bla.js "
"integrity=sha256-qznLcsROx4GACP2dm0UCKCzCG+HiZ1guq6ZZDob/Tng= "
"sha256-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxng=>"},
{1,
"<script src=bla.js "
"integrity=sha256-qznLcsROx4GACP2dm0UCKCzCG+HiZ1guq6ZZDob/Tng= "
"integrity=sha257-XXXX>"},
{0,
"<script src=bla.js integrity=sha257-XXX "
"integrity=sha256-qznLcsROx4GACP2dm0UCKCzCG+HiZ1guq6ZZDob/Tng=>"},
};
for (const auto& test_case : test_cases)
Test(test_case);
}
} // namespace blink } // namespace blink
...@@ -106,6 +106,9 @@ class CORE_EXPORT PreloadRequest { ...@@ -106,6 +106,9 @@ class CORE_EXPORT PreloadRequest {
void SetIntegrityMetadata(const IntegrityMetadataSet& metadata_set) { void SetIntegrityMetadata(const IntegrityMetadataSet& metadata_set) {
integrity_metadata_ = metadata_set; integrity_metadata_ = metadata_set;
} }
const IntegrityMetadataSet& IntegrityMetadataForTestingOnly() const {
return integrity_metadata_;
}
void SetFromInsertionScanner(const bool from_insertion_scanner) { void SetFromInsertionScanner(const bool from_insertion_scanner) {
from_insertion_scanner_ = from_insertion_scanner; from_insertion_scanner_ = from_insertion_scanner;
} }
......
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