fix(ZNTA-544): escape and santize instead

zanata · May 16, 2018 · 9c4a53c · 9c4a53c
1 parent 6c2a900
commit 9c4a53c
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 22 deletions.
diff --git a/server/services/src/main/java/org/zanata/email/ContactAdminAnonymousEmailStrategy.java b/server/services/src/main/java/org/zanata/email/ContactAdminAnonymousEmailStrategy.java
@@ -52,9 +52,8 @@ public Map<String, Object> makeContext(Map<String, Object> genericContext,
             InternetAddress[] toAddresses) {
         Map<String, Object> context =
                 super.makeContext(genericContext, toAddresses);
-        String plainText = HtmlUtil.htmlToText(
-                HtmlUtil.SANITIZER.sanitize(htmlMessage));
-        return context.put("ipAddress", ipAddress).put("htmlMessage", plainText);
+        return context.put("ipAddress", ipAddress).put("htmlMessage",
+                HtmlUtil.escapeAndSanitizeHtml(htmlMessage));
     }
 
     @java.beans.ConstructorProperties({ "ipAddress", "userSubject",

diff --git a/server/services/src/main/java/org/zanata/email/ContactAdminEmailStrategy.java b/server/services/src/main/java/org/zanata/email/ContactAdminEmailStrategy.java
@@ -59,11 +59,9 @@ public Map<String, Object> makeContext(Map<String, Object> genericContext,
             InternetAddress[] toAddresses) {
         Map<String, Object> context =
                 super.makeContext(genericContext, toAddresses);
-        String plainText = HtmlUtil.htmlToText(
-                HtmlUtil.SANITIZER.sanitize(htmlMessage));
         return context.put("fromLoginName", fromLoginName)
                 .put("fromName", fromName).put("replyEmail", replyEmail)
-                .put("htmlMessage", plainText);
+                .put("htmlMessage", HtmlUtil.escapeAndSanitizeHtml(htmlMessage));
     }
 
     @java.beans.ConstructorProperties({ "fromLoginName", "fromName",

diff --git a/server/services/src/main/java/org/zanata/email/ContactLanguageCoordinatorEmailStrategy.java b/server/services/src/main/java/org/zanata/email/ContactLanguageCoordinatorEmailStrategy.java
@@ -62,14 +62,12 @@ public Map<String, Object> makeContext(Map<String, Object> genericContext,
             InternetAddress[] toAddresses) {
         Map<String, Object> context =
                 super.makeContext(genericContext, toAddresses);
-        String plainText = HtmlUtil.htmlToText(
-                HtmlUtil.SANITIZER.sanitize(htmlMessage));
         return context.put("receiver", receiver)
                 .put("fromLoginName", fromLoginName)
                 .put("fromName", fromName).put("replyEmail", replyEmail)
                 .put("localeId", localeId)
                 .put("localeNativeName", localeNativeName)
-                .put("htmlMessage", plainText);
+                .put("htmlMessage", HtmlUtil.escapeAndSanitizeHtml(htmlMessage));
     }
 
     @java.beans.ConstructorProperties({ "receiver", "fromLoginName", "fromName",

diff --git a/server/services/src/main/java/org/zanata/email/RequestToJoinLanguageEmailStrategy.java b/server/services/src/main/java/org/zanata/email/RequestToJoinLanguageEmailStrategy.java
@@ -64,13 +64,11 @@ public Map<String, Object> makeContext(Map<String, Object> genericContext,
             InternetAddress[] toAddresses) {
         Map<String, Object> context =
                 super.makeContext(genericContext, toAddresses);
-        String plainText = HtmlUtil.htmlToText(
-                HtmlUtil.SANITIZER.sanitize(htmlMessage));
         return context.put("fromLoginName", fromLoginName)
                 .put("fromName", fromName).put("replyEmail", replyEmail)
                 .put("localeId", localeId)
                 .put("localeNativeName", localeNativeName)
-                .put("htmlMessage", plainText)
+                .put("htmlMessage", HtmlUtil.escapeAndSanitizeHtml(htmlMessage))
                 .put("requestAsTranslator", requestAsTranslator)
                 .put("requestAsReviewer", requestAsReviewer)
                 .put("requestAsCoordinator", requestAsCoordinator);

diff --git a/server/services/src/main/java/org/zanata/email/RequestToJoinVersionGroupEmailStrategy.java b/server/services/src/main/java/org/zanata/email/RequestToJoinVersionGroupEmailStrategy.java
@@ -62,13 +62,11 @@ public Map<String, Object> makeContext(Map<String, Object> genericContext,
             InternetAddress[] toAddresses) {
         Map<String, Object> context =
                 super.makeContext(genericContext, toAddresses);
-        String plainText = HtmlUtil.htmlToText(
-                HtmlUtil.SANITIZER.sanitize(htmlMessage));
         return context.put("fromLoginName", fromLoginName)
                 .put("fromName", fromName).put("replyEmail", replyEmail)
                 .put("groupName", groupName).put("versionGroupSlug", groupSlug)
                 .put("projectIterationIds", projectIterationIds)
-                .put("htmlMessage", plainText);
+                .put("htmlMessage", HtmlUtil.escapeAndSanitizeHtml(htmlMessage));
     }
 
     @java.beans.ConstructorProperties({ "fromLoginName", "fromName",

diff --git a/server/services/src/main/java/org/zanata/util/HtmlUtil.java b/server/services/src/main/java/org/zanata/util/HtmlUtil.java
@@ -21,6 +21,7 @@
 package org.zanata.util;
 
 import net.htmlparser.jericho.*;
+import org.apache.commons.lang.StringEscapeUtils;
 import org.owasp.html.HtmlPolicyBuilder;
 import org.owasp.html.PolicyFactory;
 
@@ -61,4 +62,8 @@ public static String htmlToText(String html) {
         Renderer htmlRend = new Renderer(htmlSeg);
         return htmlRend.toString();
     }
+
+    public static String escapeAndSanitizeHtml(String html) {
+        return SANITIZER.sanitize(StringEscapeUtils.escapeHtml(html));
+    }
 }
diff --git a/server/services/src/test/java/org/zanata/email/EmailStrategyTest.java b/server/services/src/test/java/org/zanata/email/EmailStrategyTest.java
@@ -76,6 +76,7 @@ public String formatWithAnyArgs(String key, Object... args) {
     String testServerPath = "https://zanata.example.com";
     InternetAddress toAddr;
     InternetAddress[] toAddresses;
+    String expectedUserMessage = "some &lt;b&gt;HTML&lt;/b&gt;";
 
     private MessagesFactory msgsFactory = new MessagesFactory() {
         private static final long serialVersionUID = 1L;
@@ -142,9 +143,7 @@ private String extractHtmlPart(MimeMessage message)
         BodyPart htmlPart = multipart.getBodyPart(1);
         assertThat(htmlPart.getDataHandler().getContentType()).isEqualTo(
                 "text/html; charset=UTF-8");
-        String htmlContent = (String) htmlPart.getContent();
-
-        return htmlContent;
+        return (String) htmlPart.getContent();
     }
 
     private void checkFromAndTo(MimeMessage message) throws MessagingException {
@@ -225,7 +224,7 @@ public void contactAdmin() throws Exception {
 
         assertThat(html).contains(msgs.format(
             "jsf.email.admin.UserMessageIntro", fromName, fromLoginName));
-        assertThat(html).contains(HtmlUtil.htmlToText(htmlMessage));
+        assertThat(html).contains(expectedUserMessage);
     }
 
     @Test
@@ -297,7 +296,7 @@ public void contactLanguageCoordinator() throws Exception {
         assertThat(html).contains(msgs.format(
                 "jsf.email.coordinator.UserMessageIntro",
                 fromName, fromLoginName, localeId, localeNativeName));
-        assertThat(html).contains(HtmlUtil.htmlToText(htmlMessage));
+        assertThat(html).contains(expectedUserMessage);
         assertThat(html).contains(
                 testServerPath + "/language/view/" + localeId);
     }
@@ -366,7 +365,7 @@ public void requestToJoinLanguage() throws Exception {
         assertThat(html).contains(msgs.format(
                 "jsf.email.joinrequest.UserRequestingToJoin",
                 fromName, fromLoginName, localeId, localeNativeName));
-        assertThat(html).contains(HtmlUtil.htmlToText(htmlMessage));
+        assertThat(html).contains(expectedUserMessage);
         assertThat(html).contains(
                 testServerPath + "/language/view/" + localeId);
     }
@@ -422,7 +421,7 @@ public void requestToJoinVersionGroup() throws Exception {
         assertThat(html).contains(msgs.format(
                 "jsf.email.joingrouprequest.RequestingToJoinGroup",
                 fromName, fromLoginName, versionGroupName));
-        assertThat(html).contains(HtmlUtil.htmlToText(htmlMessage));
+        assertThat(html).contains("some &lt;b&gt;HTML&lt;/b&gt;");
         assertThat(html).contains(
                 testServerPath + "/version-group/view/" + versionGroupSlug);
     }