From febb7448ddfc77e6817cf4da9cdfd64a03b12f20 Mon Sep 17 00:00:00 2001 From: cketti Date: Sat, 29 Apr 2017 01:45:28 +0200 Subject: [PATCH 1/5] Introduce HtmlProcessor --- .../mailstore/MessageViewInfoExtractor.java | 21 ++++---- .../fsck/k9/message/html/HtmlProcessor.java | 21 ++++++++ .../MessageViewInfoExtractorTest.java | 49 ++++++++++--------- .../k9/message/html/HtmlSanitizerHelper.java | 13 ----- 4 files changed, 57 insertions(+), 47 deletions(-) create mode 100644 k9mail/src/main/java/com/fsck/k9/message/html/HtmlProcessor.java delete mode 100644 k9mail/src/test/java/com/fsck/k9/message/html/HtmlSanitizerHelper.java diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java index d2df2a3fb..a54aaa5e1 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java @@ -10,14 +10,9 @@ import android.content.Context; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import android.support.annotation.WorkerThread; -import timber.log.Timber; import com.fsck.k9.Globals; -import com.fsck.k9.K9; import com.fsck.k9.R; -import com.fsck.k9.mail.internet.MimeUtility; -import com.fsck.k9.message.html.HtmlConverter; -import com.fsck.k9.message.html.HtmlSanitizer; import com.fsck.k9.mail.Address; import com.fsck.k9.mail.Flag; import com.fsck.k9.mail.Message; @@ -28,9 +23,12 @@ import com.fsck.k9.mail.internet.Viewable; import com.fsck.k9.mail.internet.Viewable.Flowed; import com.fsck.k9.mailstore.util.FlowedMessageUtils; import com.fsck.k9.message.extractors.AttachmentInfoExtractor; +import com.fsck.k9.message.html.HtmlConverter; +import com.fsck.k9.message.html.HtmlProcessor; import com.fsck.k9.ui.crypto.MessageCryptoAnnotations; import com.fsck.k9.ui.crypto.MessageCryptoSplitter; import com.fsck.k9.ui.crypto.MessageCryptoSplitter.CryptoMessageParts; +import timber.log.Timber; import static com.fsck.k9.mail.internet.MimeUtility.getHeaderParameter; import static com.fsck.k9.mail.internet.Viewable.Alternative; @@ -51,22 +49,22 @@ public class MessageViewInfoExtractor { private final Context context; private final AttachmentInfoExtractor attachmentInfoExtractor; - private final HtmlSanitizer htmlSanitizer; + private final HtmlProcessor htmlProcessor; public static MessageViewInfoExtractor getInstance() { Context context = Globals.getContext(); AttachmentInfoExtractor attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); - HtmlSanitizer htmlSanitizer = HtmlSanitizer.getInstance(); - return new MessageViewInfoExtractor(context, attachmentInfoExtractor, htmlSanitizer); + HtmlProcessor htmlProcessor = HtmlProcessor.newInstance(); + return new MessageViewInfoExtractor(context, attachmentInfoExtractor, htmlProcessor); } @VisibleForTesting MessageViewInfoExtractor(Context context, AttachmentInfoExtractor attachmentInfoExtractor, - HtmlSanitizer htmlSanitizer) { + HtmlProcessor htmlProcessor) { this.context = context; this.attachmentInfoExtractor = attachmentInfoExtractor; - this.htmlSanitizer = htmlSanitizer; + this.htmlProcessor = htmlProcessor; } @WorkerThread @@ -199,8 +197,7 @@ public class MessageViewInfoExtractor { } } - String content = HtmlConverter.wrapMessageContent(html); - String sanitizedHtml = htmlSanitizer.sanitize(content); + String sanitizedHtml = htmlProcessor.processForDisplay(html.toString()); return new ViewableExtractedText(text.toString(), sanitizedHtml); } catch (Exception e) { diff --git a/k9mail/src/main/java/com/fsck/k9/message/html/HtmlProcessor.java b/k9mail/src/main/java/com/fsck/k9/message/html/HtmlProcessor.java new file mode 100644 index 000000000..ab96a3bb5 --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/message/html/HtmlProcessor.java @@ -0,0 +1,21 @@ +package com.fsck.k9.message.html; + + +public class HtmlProcessor { + private final HtmlSanitizer htmlSanitizer; + + + public static HtmlProcessor newInstance() { + HtmlSanitizer htmlSanitizer = HtmlSanitizer.getInstance(); + return new HtmlProcessor(htmlSanitizer); + } + + private HtmlProcessor(HtmlSanitizer htmlSanitizer) { + this.htmlSanitizer = htmlSanitizer; + } + + public String processForDisplay(String html) { + String wrappedHtml = HtmlConverter.wrapMessageContent(html); + return htmlSanitizer.sanitize(wrappedHtml); + } +} diff --git a/k9mail/src/test/java/com/fsck/k9/mailstore/MessageViewInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/mailstore/MessageViewInfoExtractorTest.java index fd9fee679..cb85c1f4c 100644 --- a/k9mail/src/test/java/com/fsck/k9/mailstore/MessageViewInfoExtractorTest.java +++ b/k9mail/src/test/java/com/fsck/k9/mailstore/MessageViewInfoExtractorTest.java @@ -12,8 +12,6 @@ import android.app.Application; import com.fsck.k9.GlobalsHelper; import com.fsck.k9.K9RobolectricTestRunner; -import com.fsck.k9.message.html.HtmlSanitizer; -import com.fsck.k9.message.html.HtmlSanitizerHelper; import com.fsck.k9.mail.Address; import com.fsck.k9.mail.Message.RecipientType; import com.fsck.k9.mail.MessagingException; @@ -28,14 +26,17 @@ import com.fsck.k9.mail.internet.TextBody; import com.fsck.k9.mail.internet.Viewable; import com.fsck.k9.mail.internet.Viewable.MessageHeader; import com.fsck.k9.mailstore.MessageViewInfoExtractor.ViewableExtractedText; +import com.fsck.k9.message.html.HtmlProcessor; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import org.robolectric.RuntimeEnvironment; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertSame; -import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -57,10 +58,8 @@ public class MessageViewInfoExtractorTest { GlobalsHelper.setContext(context); - HtmlSanitizer dummyHtmlSanitizer = HtmlSanitizerHelper.getDummyHtmlSanitizer(); - - messageViewInfoExtractor = new MessageViewInfoExtractor(context, - null, dummyHtmlSanitizer); + HtmlProcessor htmlProcessor = createFakeHtmlProcessor(); + messageViewInfoExtractor = new MessageViewInfoExtractor(context,null, htmlProcessor); } @Test @@ -74,11 +73,11 @@ public class MessageViewInfoExtractorTest { message.setHeader(MimeHeader.HEADER_CONTENT_TYPE, "text/plain; format=flowed"); // Prepare fixture - HtmlSanitizer htmlSanitizer = mock(HtmlSanitizer.class); + HtmlProcessor htmlProcessor = mock(HtmlProcessor.class); MessageViewInfoExtractor messageViewInfoExtractor = - new MessageViewInfoExtractor(context, null, htmlSanitizer); + new MessageViewInfoExtractor(context, null, htmlProcessor); String value = "--sanitized html--"; - when(htmlSanitizer.sanitize(any(String.class))).thenReturn(value); + when(htmlProcessor.processForDisplay(anyString())).thenReturn(value); // Extract text List outputNonViewableParts = new ArrayList<>(); @@ -113,7 +112,7 @@ public class MessageViewInfoExtractorTest { ""; assertEquals(expectedText, container.text); - assertEquals(expectedHtml, getHtmlBodyText(container.html)); + assertEquals(expectedHtml, container.html); } @Test @@ -140,7 +139,7 @@ public class MessageViewInfoExtractorTest { ""; assertEquals(expectedText, container.text); - assertEquals(expectedHtml, getHtmlBodyText(container.html)); + assertEquals(expectedHtml, container.html); } @Test @@ -166,7 +165,7 @@ public class MessageViewInfoExtractorTest { bodyText; assertEquals(expectedText, container.text); - assertEquals(expectedHtml, getHtmlBodyText(container.html)); + assertEquals(expectedHtml, container.html); } @Test @@ -211,7 +210,7 @@ public class MessageViewInfoExtractorTest { assertEquals(expectedText, container.text); - assertEquals(expectedHtml, getHtmlBodyText(container.html)); + assertEquals(expectedHtml, container.html); } @Test @@ -229,7 +228,7 @@ public class MessageViewInfoExtractorTest { // Create message/rfc822 body MimeMessage innerMessage = new MimeMessage(); - innerMessage.addSentDate(new Date(112, 02, 17), false); + innerMessage.addSentDate(new Date(112, 2, 17), false); innerMessage.setRecipients(RecipientType.TO, new Address[] { new Address("to@example.com") }); innerMessage.setSubject("Subject"); innerMessage.setFrom(new Address("from@example.com")); @@ -290,7 +289,7 @@ public class MessageViewInfoExtractorTest { ""; assertEquals(expectedText, container.text); - assertEquals(expectedHtml, getHtmlBodyText(container.html)); + assertEquals(expectedHtml, container.html); } @Test @@ -355,13 +354,19 @@ public class MessageViewInfoExtractorTest { ViewableExtractedText firstMessageExtractedText = messageViewInfoExtractor.extractTextFromViewables(outputViewableParts); assertEquals(expectedExtractedText, firstMessageExtractedText.text); - assertEquals(expectedHtmlText, getHtmlBodyText(firstMessageExtractedText.html)); + assertEquals(expectedHtmlText, firstMessageExtractedText.html); } - private static String getHtmlBodyText(String htmlText) { - htmlText = htmlText.substring(htmlText.indexOf("") +6); - htmlText = htmlText.substring(0, htmlText.indexOf("")); - return htmlText; - } + HtmlProcessor createFakeHtmlProcessor() { + HtmlProcessor htmlProcessor = mock(HtmlProcessor.class); + when(htmlProcessor.processForDisplay(anyString())).thenAnswer(new Answer() { + @Override + public String answer(InvocationOnMock invocation) throws Throwable { + return (String) invocation.getArguments()[0]; + } + }); + + return htmlProcessor; + } } diff --git a/k9mail/src/test/java/com/fsck/k9/message/html/HtmlSanitizerHelper.java b/k9mail/src/test/java/com/fsck/k9/message/html/HtmlSanitizerHelper.java deleted file mode 100644 index 3ad0445a4..000000000 --- a/k9mail/src/test/java/com/fsck/k9/message/html/HtmlSanitizerHelper.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.fsck.k9.message.html; - - -public class HtmlSanitizerHelper { - public static HtmlSanitizer getDummyHtmlSanitizer() { - return new HtmlSanitizer() { - @Override - public String sanitize(String html) { - return html; - } - }; - } -} From 268189c1b097a058cf8e5da621f760871fa58d58 Mon Sep 17 00:00:00 2001 From: cketti Date: Sat, 29 Apr 2017 02:28:33 +0200 Subject: [PATCH 2/5] Use jsoup in HtmlProcessor and HtmlSanitizer --- k9mail/build.gradle | 3 +- .../com/fsck/k9/message/html/HeadCleaner.java | 100 ++++++++++++++++++ .../fsck/k9/message/html/HtmlConverter.java | 4 +- .../fsck/k9/message/html/HtmlProcessor.java | 25 ++++- .../fsck/k9/message/html/HtmlSanitizer.java | 72 ++++--------- .../k9/message/html/HtmlSanitizerTest.java | 61 +++++------ 6 files changed, 175 insertions(+), 90 deletions(-) create mode 100644 k9mail/src/main/java/com/fsck/k9/message/html/HeadCleaner.java diff --git a/k9mail/build.gradle b/k9mail/build.gradle index 1d238b6c9..ac1eb9a55 100644 --- a/k9mail/build.gradle +++ b/k9mail/build.gradle @@ -28,6 +28,7 @@ dependencies { compile 'commons-io:commons-io:2.4' compile "com.android.support:support-v4:${androidSupportLibraryVersion}" compile 'net.sourceforge.htmlcleaner:htmlcleaner:2.18' + compile 'org.jsoup:jsoup:1.10.2' compile 'de.cketti.library.changelog:ckchangelog:1.2.1' compile 'com.github.bumptech.glide:glide:3.6.1' compile 'com.splitwise:tokenautocomplete:2.0.7' @@ -41,7 +42,6 @@ dependencies { testCompile "org.robolectric:robolectric:${robolectricVersion}" testCompile "junit:junit:${junitVersion}" testCompile "org.mockito:mockito-core:${mockitoVersion}" - testCompile 'org.jsoup:jsoup:1.10.2' } android { @@ -96,6 +96,7 @@ android { exclude 'META-INF/LICENSE.txt' exclude 'META-INF/NOTICE' exclude 'META-INF/NOTICE.txt' + exclude 'META-INF/README' exclude 'LICENSE.txt' } diff --git a/k9mail/src/main/java/com/fsck/k9/message/html/HeadCleaner.java b/k9mail/src/main/java/com/fsck/k9/message/html/HeadCleaner.java new file mode 100644 index 000000000..75ce95964 --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/message/html/HeadCleaner.java @@ -0,0 +1,100 @@ +package com.fsck.k9.message.html; + + +import java.util.List; +import java.util.Locale; + +import org.jsoup.nodes.Attributes; +import org.jsoup.nodes.DataNode; +import org.jsoup.nodes.Document; +import org.jsoup.nodes.Element; +import org.jsoup.nodes.Node; +import org.jsoup.nodes.TextNode; +import org.jsoup.parser.Tag; +import org.jsoup.select.NodeTraversor; +import org.jsoup.select.NodeVisitor; + +import static java.util.Arrays.asList; + + +class HeadCleaner { + private static final List ALLOWED_TAGS = asList("style", "meta"); + + + public void clean(Document dirtyDocument, Document cleanedDocument) { + copySafeNodes(dirtyDocument.head(), cleanedDocument.head()); + } + + private void copySafeNodes(Element source, Element destination) { + CleaningVisitor cleaningVisitor = new CleaningVisitor(source, destination); + NodeTraversor traversor = new NodeTraversor(cleaningVisitor); + traversor.traverse(source); + } + + + static class CleaningVisitor implements NodeVisitor { + private final Element root; + private Element destination; + private boolean skipChildren = false; + + + CleaningVisitor(Element root, Element destination) { + this.root = root; + this.destination = destination; + } + + public void head(Node source, int depth) { + if (skipChildren) { + return; + } + + if (source instanceof Element) { + Element sourceElement = (Element) source; + + if (isSafeTag(sourceElement)) { + String sourceTag = sourceElement.tagName(); + Attributes destinationAttributes = sourceElement.attributes().clone(); + Element destinationChild = new Element(Tag.valueOf(sourceTag), sourceElement.baseUri(), destinationAttributes); + + destination.appendChild(destinationChild); + destination = destinationChild; + } else if (source != root) { + skipChildren = true; + } + } else if (source instanceof TextNode) { + TextNode sourceText = (TextNode) source; + TextNode destinationText = new TextNode(sourceText.getWholeText(), source.baseUri()); + destination.appendChild(destinationText); + } else if (source instanceof DataNode && isSafeTag(source.parent())) { + DataNode sourceData = (DataNode) source; + DataNode destinationData = new DataNode(sourceData.getWholeData(), source.baseUri()); + destination.appendChild(destinationData); + } + } + + public void tail(Node source, int depth) { + if (source == destination) { + destination = destination.parent(); + skipChildren = false; + } + } + + private boolean isSafeTag(Node node) { + if (isMetaRefresh(node)) { + return false; + } + + String tag = node.nodeName().toLowerCase(Locale.ROOT); + return ALLOWED_TAGS.contains(tag); + } + + private boolean isMetaRefresh(Node node) { + if (!"meta".equalsIgnoreCase(node.nodeName())) { + return false; + } + + String attributeValue = node.attributes().getIgnoreCase("http-equiv"); + return "refresh".equalsIgnoreCase(attributeValue.trim()); + } + } +} diff --git a/k9mail/src/main/java/com/fsck/k9/message/html/HtmlConverter.java b/k9mail/src/main/java/com/fsck/k9/message/html/HtmlConverter.java index 9b0e5d9ab..0dabd4558 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/html/HtmlConverter.java +++ b/k9mail/src/main/java/com/fsck/k9/message/html/HtmlConverter.java @@ -1260,7 +1260,7 @@ public class HtmlConverter { ""; } - private static String cssStyleTheme() { + static String cssStyleTheme() { if (K9.getK9MessageViewTheme() == K9.Theme.DARK) { return "