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..9df50e5c6b1ec8e1ba560cc4894956e395396f4e 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,45 @@ 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());
+    // Boundaries are normally randomly generated but this one is predefined for
+    // simplicity and as good as any other. Plus it gets used in almost all the
+    // examples in the MHTML spec - RFC 2557.
+    const 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()) {

[jianli, 2017/04/26 00:54:45]

Do we have plan to validate the number of resources?

[carlosk, 2017/04/26 01:07:51]

For MHTML well-formed-ness checks -- which is the only thing I wanted to check
for -- we don't need to. Any MHTML validation error will cause the output to be
fully empty.

+        ADD_FAILURE() << "Invalid MHTML";
+        mhtml_string = String();
+      }
+    }
+    return mhtml_string;
   }
 
   void SetRemovePopupOverlay(bool remove_popup_overlay) {
@@ -240,7 +271,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 +293,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 +303,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 +338,12 @@ 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");
+  // For this specific test we only care that the result of the body parts
+  // generation is empty so it is simpler to check it by only generating that
+  // part of the MHTML output by setting |only_body_parts| to true.
+  String mhtml = GenerateMHTML("http://www.test.com", "broken-image.png",
+                               "image/png", true);

[jianli, 2017/04/26 00:54:45]

I don't think you need to pass another parameter since you can simply check mime
type. If it is not text/html, there is no need to generate other parts.

[carlosk, 2017/04/26 01:07:50]

I would prefer to keep it explicit because handling mime types differently seems
counter intuitive. This is difference in behavior is specific to this test to
make it easier to test its results and not because the different mime type would
change MHTML generation.

WDYT?

[jianli, 2017/04/26 01:12:27]

I prefer not to adding too many arguments to complicate things. Since only one
test is involved, adding one more custom argument seems to be too much. Also I
think it makes sense not to generate header and foot for any non-html
serialization.

[carlosk, 2017/04/28 01:16:19]

Changing the returned contents solely based on mime type still feels too
surprising to me. But I added two helper methods that keep calls from tests
simple, with only two parameters.
WDYT?

+  ASSERT_FALSE(HasFailure());
   EXPECT_TRUE(mhtml.IsEmpty());
 }
 
@@ -321,7 +358,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 +381,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 +395,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 +404,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"));