WebFrameSerializerSanitizationTest validates generated MHTML.

third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp

Index: third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp
diff --git a/third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp b/third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp
index d3b2e8b98290a748a90a9695287cc6f69efbafca..1f5591ba08f4565c2f292a102e529b1d48859942 100644
--- a/third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp
+++ b/third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp
@@ -30,6 +30,8 @@
 
 #include "public/web/WebFrameSerializer.h"
 
+#include "platform/mhtml/MHTMLArchive.h"
+#include "platform/mhtml/MHTMLParser.h"
 #include "platform/testing/URLTestHelpers.h"
 #include "platform/testing/UnitTestHelpers.h"
 #include "platform/weborigin/KURL.h"
@@ -218,16 +220,44 @@ class WebFrameSerializerSanitizationTest : public WebFrameSerializerTest {
 
   ~WebFrameSerializerSanitizationTest() override {}
 
-  String GenerateMHTMLParts(const String& url,
-                            const String& file_name,
-                            const String& mime_type = "text/html") {
+  String GenerateMHTML(const String& url,
+                       const String& file_name,
+                       const String& mime_type = "text/html",
+                       const bool only_body_parts = false) {
     KURL parsed_url(kParsedURLString, url);
     String file_path("frameserialization/" + file_name);
     RegisterMockedFileURLLoad(parsed_url, file_path, mime_type);
     FrameTestHelpers::LoadFrame(MainFrameImpl(), url.Utf8().data());
-    WebThreadSafeData result = WebFrameSerializer::GenerateMHTMLParts(
-        WebString("boundary"), MainFrameImpl(), &mhtml_delegate_);
-    return String(result.Data(), result.size());
+    // This boundary is as good as any other.  Plus it gets used in almost

[dewittj, 2017/04/25 22:32:33]

I'd rewrite this comment, since it's not obvious why you are justifying this
exactly.  Is it important that it is constant rather than randomly generated as
in real files?  Is its length important? etc.

[carlosk, 2017/04/26 00:16:49]

On 2017/04/25 22:32:33, dewittj wrote:
Context: this is a copy/paste from MHTMLTest::GenerateMHTMLData where I found
this comment. As I would have to promote the unnamed parameter into a named
value here to be used for all calls I thought it was better to get the
"justified" value and its explanation.
Rephrased. LMKWDYT.

+    // all the examples in the MHTML spec - RFC 2557.
+    WebString boundary("boundary-example");
+    StringBuilder mhtml;
+    if (!only_body_parts) {
+      WebThreadSafeData header_result = WebFrameSerializer::GenerateMHTMLHeader(
+          boundary, MainFrameImpl(), &mhtml_delegate_);
+      mhtml.Append(header_result.Data(), header_result.size());
+    }
+    WebThreadSafeData body_result = WebFrameSerializer::GenerateMHTMLParts(
+        boundary, MainFrameImpl(), &mhtml_delegate_);
+    mhtml.Append(body_result.Data(), body_result.size());
+    if (!only_body_parts) {
+      RefPtr<RawData> footer_data = RawData::Create();
+      MHTMLArchive::GenerateMHTMLFooterForTesting(boundary,
+                                                  *footer_data->MutableData());
+      mhtml.Append(footer_data->data(), footer_data->length());
+    }
+    String mhtml_string = mhtml.ToString();
+
+    if (!only_body_parts) {
+      // Validate the generated MHTML.
+      MHTMLParser parser(SharedBuffer::Create(mhtml_string.Characters8(),
+                                              size_t(mhtml_string.length())));
+      if (parser.ParseArchive().IsEmpty()) {
+        ADD_FAILURE() << "Invalid MHTML";
+        mhtml_string = String();
+      }
+    }
+    return mhtml_string;
   }
 
   void SetRemovePopupOverlay(bool remove_popup_overlay) {
@@ -240,7 +270,8 @@ class WebFrameSerializerSanitizationTest : public WebFrameSerializerTest {
 
 TEST_F(WebFrameSerializerSanitizationTest, RemoveInlineScriptInAttributes) {
   String mhtml =
-      GenerateMHTMLParts("http://www.test.com", "script_in_attributes.html");
+      GenerateMHTML("http://www.test.com", "script_in_attributes.html");
+  ASSERT_FALSE(HasFailure());
 
   // These scripting attributes should be removed.
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("onload="));
@@ -261,7 +292,8 @@ TEST_F(WebFrameSerializerSanitizationTest, RemoveInlineScriptInAttributes) {
 }
 
 TEST_F(WebFrameSerializerSanitizationTest, DisableFormElements) {
-  String mhtml = GenerateMHTMLParts("http://www.test.com", "form.html");
+  String mhtml = GenerateMHTML("http://www.test.com", "form.html");
+  ASSERT_FALSE(HasFailure());
 
   const char kDisabledAttr[] = "disabled=3D\"\"";
   int matches =
@@ -270,8 +302,8 @@ TEST_F(WebFrameSerializerSanitizationTest, DisableFormElements) {
 }
 
 TEST_F(WebFrameSerializerSanitizationTest, RemoveHiddenElements) {
-  String mhtml =
-      GenerateMHTMLParts("http://www.test.com", "hidden_elements.html");
+  String mhtml = GenerateMHTML("http://www.test.com", "hidden_elements.html");
+  ASSERT_FALSE(HasFailure());
 
   // The element with hidden attribute should be removed.
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("<p id=3D\"hidden_id\""));
@@ -305,8 +337,9 @@ TEST_F(WebFrameSerializerSanitizationTest, RemoveHiddenElements) {
 // Regression test for crbug.com/678893, where in some cases serializing an
 // image document could cause code to pick an element from an empty container.
 TEST_F(WebFrameSerializerSanitizationTest, FromBrokenImageDocument) {
-  String mhtml = GenerateMHTMLParts("http://www.test.com", "broken-image.png",
-                                    "image/png");
+  String mhtml = GenerateMHTML("http://www.test.com", "broken-image.png",
+                               "image/png", true);

[jianli, 2017/04/25 21:41:34]

Please comment why we only need to generate body parts for this test.

[carlosk, 2017/04/26 00:16:49]

Done. I left the (currently unnecessary) check below in place in case extra
validation is eventually added that would run even when generating only body
parts.

+  ASSERT_FALSE(HasFailure());
   EXPECT_TRUE(mhtml.IsEmpty());
 }
 
@@ -321,7 +354,8 @@ TEST_F(WebFrameSerializerSanitizationTest, ImageLoadedFromSrcsetForHiDPI) {
   // Set high DPR in order to load image from srcset, instead of src.
   WebView()->SetDeviceScaleFactor(2.0f);
 
-  String mhtml = GenerateMHTMLParts("http://www.test.com", "img_srcset.html");
+  String mhtml = GenerateMHTML("http://www.test.com", "img_srcset.html");
+  ASSERT_FALSE(HasFailure());
 
   // srcset attribute should be skipped.
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("srcset="));
@@ -343,7 +377,8 @@ TEST_F(WebFrameSerializerSanitizationTest, ImageLoadedFromSrcForNormalDPI) {
       KURL(kParsedURLString, "http://www.test.com/2x.png"),
       "frameserialization/2x.png");
 
-  String mhtml = GenerateMHTMLParts("http://www.test.com", "img_srcset.html");
+  String mhtml = GenerateMHTML("http://www.test.com", "img_srcset.html");
+  ASSERT_FALSE(HasFailure());
 
   // srcset attribute should be skipped.
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("srcset="));
@@ -356,7 +391,8 @@ TEST_F(WebFrameSerializerSanitizationTest, ImageLoadedFromSrcForNormalDPI) {
 TEST_F(WebFrameSerializerSanitizationTest, RemovePopupOverlayIfRequested) {
   WebView()->Resize(WebSize(500, 500));
   SetRemovePopupOverlay(true);
-  String mhtml = GenerateMHTMLParts("http://www.test.com", "popup.html");
+  String mhtml = GenerateMHTML("http://www.test.com", "popup.html");
+  ASSERT_FALSE(HasFailure());
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("class=3D\"overlay"));
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("class=3D\"modal"));
 }
@@ -364,15 +400,15 @@ TEST_F(WebFrameSerializerSanitizationTest, RemovePopupOverlayIfRequested) {
 TEST_F(WebFrameSerializerSanitizationTest, KeepPopupOverlayIfNotRequested) {
   WebView()->Resize(WebSize(500, 500));
   SetRemovePopupOverlay(false);
-  String mhtml = GenerateMHTMLParts("http://www.test.com", "popup.html");
+  String mhtml = GenerateMHTML("http://www.test.com", "popup.html");
+  ASSERT_FALSE(HasFailure());
   EXPECT_NE(WTF::kNotFound, mhtml.Find("class=3D\"overlay"));
   EXPECT_NE(WTF::kNotFound, mhtml.Find("class=3D\"modal"));
 }
 
 TEST_F(WebFrameSerializerSanitizationTest, RemoveElements) {
-  String mhtml =
-      GenerateMHTMLParts("http://www.test.com", "remove_elements.html");
-  LOG(ERROR) << mhtml;
+  String mhtml = GenerateMHTML("http://www.test.com", "remove_elements.html");
+  ASSERT_FALSE(HasFailure());
 
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("<script"));
   EXPECT_EQ(WTF::kNotFound, mhtml.Find("<noscript"));