escape link, image and table cell text in markdown handler

Author: rootvector2

tika-core/src/main/java/org/apache/tika/sax/ToMarkdownContentHandler.java

@@ -150,7 +150,8 @@ public void startElement(String uri, String localName, String qName, Attributes
             case "img":
                 String alt = atts.getValue("alt");
                 String src = atts.getValue("src");
-                write("![" + (alt != null ? alt : "") + "](" + (src != null ? src : "") + ")");
+                write("![" + escapeMarkdown(alt != null ? alt : "") + "](" +
+                        formatLinkDestination(src) + ")");
                 break;
             case "ul":
             case "ol":
@@ -282,9 +283,8 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
                 break;
             case "a":
                 if (linkText != null) {
-                    String text = linkText.toString();
-                    String href = linkHref != null ? linkHref : "";
-                    write("[" + text + "](" + href + ")");
+                    write("[" + escapeMarkdown(linkText.toString()) + "](" +
+                            formatLinkDestination(linkHref) + ")");
                     linkText = null;
                     linkHref = null;
                 }
@@ -343,7 +343,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
             case "th":
             case "td":
                 if (tableDepth == 1 && currentRow != null && currentCell != null) {
-                    currentRow.add(currentCell.toString().trim());
+                    currentRow.add(escapeTableCell(currentCell.toString()));
                     currentCell = null;
                 }
                 break;
@@ -525,6 +525,28 @@ private static String escapeMarkdown(String text) {
         return sb.toString();
     }
 
+    private static String escapeTableCell(String text) {
+        // a newline ends a GFM table row and a pipe ends a cell; fold the former and
+        // escape the latter (with the other markers) so cell content can't break out
+        // of its column or row.
+        return escapeMarkdown(text.replaceAll("[\\r\\n]+", " ").trim());
+    }
+
+    private static String formatLinkDestination(String url) {
+        if (url == null || url.isEmpty()) {
+            return "";
+        }
+        // angle brackets and line breaks can't appear in any markdown destination
+        String cleaned = url.replaceAll("[\\u0000-\\u001f<>]", "");
+        // a space or paren would otherwise close the bare (url) form early, so wrap
+        // those in <>, where spaces and parens are allowed
+        if (cleaned.indexOf(' ') >= 0 || cleaned.indexOf('(') >= 0
+                || cleaned.indexOf(')') >= 0) {
+            return "<" + cleaned + ">";
+        }
+        return cleaned;
+    }
+
     private static String repeatChar(char c, int count) {
         StringBuilder sb = new StringBuilder(count);
         for (int i = 0; i < count; i++) {

tika-core/src/test/java/org/apache/tika/sax/ToMarkdownContentHandlerTest.java

@@ -697,6 +697,108 @@ private static void assertContains(String needle, String haystack) {
                 "Expected to find '" + needle + "' in: " + haystack);
     }
 
+    @Test
+    public void testLinkTextEscaped() throws Exception {
+        ToMarkdownContentHandler handler = new ToMarkdownContentHandler();
+        handler.startDocument();
+
+        startElement(handler, "p");
+        startElement(handler, "a", "href", "https://good.example");
+        chars(handler, "x](https://evil.example)");
+        endElement(handler, "a");
+        endElement(handler, "p");
+
+        handler.endDocument();
+
+        String result = handler.toString();
+        // the ] in the link text must be escaped so it cannot close the link early
+        assertTrue(result.contains("x\\]"), result);
+        assertFalse(result.contains("[x](https://evil.example)"), result);
+    }
+
+    @Test
+    public void testLinkHrefNoBreakout() throws Exception {
+        ToMarkdownContentHandler handler = new ToMarkdownContentHandler();
+        handler.startDocument();
+
+        startElement(handler, "p");
+        startElement(handler, "a", "href", "https://evil.example) text");
+        chars(handler, "click");
+        endElement(handler, "a");
+        endElement(handler, "p");
+
+        handler.endDocument();
+
+        String result = handler.toString();
+        // a destination with spaces/parens must be wrapped so it cannot terminate (..) early
+        assertTrue(result.contains("](<https://evil.example"), result);
+        assertFalse(result.contains("](https://evil.example)"), result);
+    }
+
+    @Test
+    public void testImageAltAndSrcEscaped() throws Exception {
+        ToMarkdownContentHandler handler = new ToMarkdownContentHandler();
+        handler.startDocument();
+
+        startElement(handler, "p");
+        AttributesImpl atts = new AttributesImpl();
+        atts.addAttribute("", "alt", "alt", "CDATA", "a]b");
+        atts.addAttribute("", "src", "src", "CDATA", "https://evil.example) x");
+        startElement(handler, "img", atts);
+        endElement(handler, "img");
+        endElement(handler, "p");
+
+        handler.endDocument();
+
+        String result = handler.toString();
+        assertTrue(result.contains("a\\]b"), result);
+        assertTrue(result.contains("](<https://evil.example"), result);
+        assertFalse(result.contains("](https://evil.example)"), result);
+    }
+
+    @Test
+    public void testTableCellPipeEscaped() throws Exception {
+        ToMarkdownContentHandler handler = new ToMarkdownContentHandler();
+        handler.startDocument();
+
+        startElement(handler, "table");
+        startElement(handler, "tr");
+        startElement(handler, "td");
+        chars(handler, "a|b");
+        endElement(handler, "td");
+        startElement(handler, "td");
+        chars(handler, "c");
+        endElement(handler, "td");
+        endElement(handler, "tr");
+        endElement(handler, "table");
+
+        handler.endDocument();
+
+        String result = handler.toString();
+        // a pipe inside a cell must be escaped so it does not inject an extra column
+        assertTrue(result.contains("| a\\|b | c |"), result);
+    }
+
+    @Test
+    public void testTableCellNewlineFolded() throws Exception {
+        ToMarkdownContentHandler handler = new ToMarkdownContentHandler();
+        handler.startDocument();
+
+        startElement(handler, "table");
+        startElement(handler, "tr");
+        startElement(handler, "td");
+        chars(handler, "a\nb");
+        endElement(handler, "td");
+        endElement(handler, "tr");
+        endElement(handler, "table");
+
+        handler.endDocument();
+
+        String result = handler.toString();
+        // a newline inside a cell must not terminate the table row
+        assertTrue(result.contains("| a b |"), result);
+    }
+
     @Test
     public void testHandlerTypeParsingMarkdown() {
         assertEquals(BasicContentHandlerFactory.HANDLER_TYPE.MARKDOWN,