From 2e006697e0aac3594de2d0df618d0db0e2c5534e Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 9 Jul 2020 10:56:44 +0200 Subject: [PATCH] [core] Refactor XMLRenderer to use XMLStreamWriter In order to properly support different encodings, a OutputStream is needed. Then Java will take care of unmappaple characters and encode them as entities for XML. For backwards compatibility, a writer is still created and exposed. --- .../pmd/renderers/XMLRenderer.java | 312 ++++++++++-------- .../pmd/renderers/XSLTRenderer.java | 40 +-- .../pmd/renderers/XMLRendererTest.java | 8 +- .../net/sourceforge/pmd/ant/PMDTaskTest.java | 13 +- 4 files changed, 200 insertions(+), 173 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java index 2f7c74d7da..23ba8fd334 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java @@ -6,16 +6,23 @@ package net.sourceforge.pmd.renderers; import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.io.OutputStreamWriter; -import java.nio.charset.Charset; -import java.nio.charset.UnsupportedCharsetException; +import java.io.UnsupportedEncodingException; +import java.io.Writer; import java.nio.file.Files; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Iterator; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.xml.XMLConstants; +import javax.xml.stream.XMLOutputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamWriter; +import org.apache.commons.io.output.WriterOutputStream; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.text.StringEscapeUtils; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.PMDVersion; @@ -34,6 +41,13 @@ public class XMLRenderer extends AbstractIncrementingRenderer { public static final StringProperty ENCODING = new StringProperty("encoding", "XML encoding format, defaults to UTF-8.", "UTF-8", 0); + private static final String PMD_REPORT_NS_URI = "http://pmd.sourceforge.net/report/2.0.0"; + private static final String PMD_REPORT_NS_LOCATION = "http://pmd.sourceforge.net/report_2_0_0.xsd"; + private static final String XSI_NS_PREFIX = "xsi"; + + private XMLStreamWriter xmlWriter; + private OutputStream stream; + public XMLRenderer() { super(NAME, "XML format."); definePropertyDescriptor(ENCODING); @@ -53,170 +67,204 @@ public class XMLRenderer extends AbstractIncrementingRenderer { public void start() throws IOException { String encoding = getProperty(ENCODING); - StringBuilder buf = new StringBuilder(500); - buf.append("").append(PMD.EOL); - createVersionAttr(buf); - createTimestampAttr(buf); - // FIXME: elapsed time not available until the end of the processing - // buf.append(createTimeElapsedAttr(report)); - buf.append('>').append(PMD.EOL); - writer.write(buf.toString()); + try { + xmlWriter.writeStartDocument(encoding, "1.0"); + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.setDefaultNamespace(PMD_REPORT_NS_URI); + xmlWriter.writeStartElement(PMD_REPORT_NS_URI, "pmd"); + xmlWriter.writeDefaultNamespace(PMD_REPORT_NS_URI); + xmlWriter.writeNamespace(XSI_NS_PREFIX, XMLConstants.W3C_XML_SCHEMA_INSTANCE_NS_URI); + xmlWriter.writeAttribute(XSI_NS_PREFIX, XMLConstants.W3C_XML_SCHEMA_INSTANCE_NS_URI, "schemaLocation", + PMD_REPORT_NS_URI + " " + PMD_REPORT_NS_LOCATION); + xmlWriter.writeAttribute("version", PMDVersion.VERSION); + xmlWriter.writeAttribute("timestamp", new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS").format(new Date())); + // FIXME: elapsed time not available until the end of the processing + // xmlWriter.writeAttribute("time_elapsed", ...); + } catch (XMLStreamException e) { + throw new IOException(e); + } } @Override public void renderFileViolations(Iterator violations) throws IOException { - StringBuilder buf = new StringBuilder(500); String filename = null; - // rule violations - while (violations.hasNext()) { - buf.setLength(0); - RuleViolation rv = violations.next(); - String nextFilename = determineFileName(rv.getFilename()); - if (!nextFilename.equals(filename)) { - // New File - if (filename != null) { - // Not first file ? - buf.append("").append(PMD.EOL); + try { + // rule violations + while (violations.hasNext()) { + RuleViolation rv = violations.next(); + String nextFilename = determineFileName(rv.getFilename()); + if (!nextFilename.equals(filename)) { + // New File + if (filename != null) { + // Not first file ? + xmlWriter.writeEndElement(); + } + filename = nextFilename; + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeStartElement("file"); + xmlWriter.writeAttribute("name", filename); + xmlWriter.writeCharacters(PMD.EOL); } - filename = nextFilename; - buf.append("").append(PMD.EOL); + + xmlWriter.writeStartElement("violation"); + xmlWriter.writeAttribute("beginline", String.valueOf(rv.getBeginLine())); + xmlWriter.writeAttribute("endline", String.valueOf(rv.getEndLine())); + xmlWriter.writeAttribute("begincolumn", String.valueOf(rv.getBeginColumn())); + xmlWriter.writeAttribute("endcolumn", String.valueOf(rv.getEndColumn())); + xmlWriter.writeAttribute("rule", rv.getRule().getName()); + xmlWriter.writeAttribute("ruleset", rv.getRule().getRuleSetName()); + maybeAdd("package", rv.getPackageName()); + maybeAdd("class", rv.getClassName()); + maybeAdd("method", rv.getMethodName()); + maybeAdd("variable", rv.getVariableName()); + maybeAdd("externalInfoUrl", rv.getRule().getExternalInfoUrl()); + xmlWriter.writeAttribute("priority", String.valueOf(rv.getRule().getPriority().getPriority())); + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeCharacters(removeInvalidCharacters(rv.getDescription())); + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeEndElement(); + xmlWriter.writeCharacters(PMD.EOL); } - - buf.append("").append(PMD.EOL); - buf.append(escape(rv.getDescription())); - - buf.append(PMD.EOL); - buf.append(""); - buf.append(PMD.EOL); - writer.write(buf.toString()); - } - if (filename != null) { // Not first file ? - writer.write(""); - writer.write(PMD.EOL); + if (filename != null) { // Not first file ? + xmlWriter.writeEndElement(); + } + } catch (XMLStreamException e) { + throw new IOException(e); } } @Override public void end() throws IOException { - StringBuilder buf = new StringBuilder(500); - // errors - for (Report.ProcessingError pe : errors) { - buf.setLength(0); - buf.append("").append(PMD.EOL); - buf.append("").append(PMD.EOL); - buf.append("").append(PMD.EOL); - writer.write(buf.toString()); - } - - // suppressed violations - if (showSuppressedViolations) { - for (Report.SuppressedViolation s : suppressed) { - buf.setLength(0); - buf.append("").append(PMD.EOL); - writer.write(buf.toString()); + try { + // errors + for (Report.ProcessingError pe : errors) { + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeStartElement("error"); + xmlWriter.writeAttribute("filename", determineFileName(pe.getFile())); + xmlWriter.writeAttribute("msg", pe.getMsg()); + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeCData(pe.getDetail()); + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeEndElement(); } - } - // config errors - for (final Report.ConfigurationError ce : configErrors) { - buf.setLength(0); - buf.append("").append(PMD.EOL); - writer.write(buf.toString()); - } + // suppressed violations + if (showSuppressedViolations) { + for (Report.SuppressedViolation s : suppressed) { + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeStartElement("suppressedviolation"); + xmlWriter.writeAttribute("filename", determineFileName(s.getRuleViolation().getFilename())); + xmlWriter.writeAttribute("suppressiontype", s.suppressedByNOPMD() ? "nopmd" : "annotation"); + xmlWriter.writeAttribute("msg", s.getRuleViolation().getDescription()); + xmlWriter.writeAttribute("usermsg", s.getUserMessage() == null ? "" : s.getUserMessage()); + xmlWriter.writeEndElement(); + } + } - writer.write("" + PMD.EOL); + // config errors + for (final Report.ConfigurationError ce : configErrors) { + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeEmptyElement("configerror"); + xmlWriter.writeAttribute("rule", ce.rule().getName()); + xmlWriter.writeAttribute("msg", ce.issue()); + } + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.writeEndElement(); // + xmlWriter.writeCharacters(PMD.EOL); + xmlWriter.flush(); + } catch (XMLStreamException e) { + throw new IOException(e); + } } - private void maybeAdd(String attr, String value, StringBuilder buf) { + private void maybeAdd(String attr, String value) throws XMLStreamException { if (value != null && value.length() > 0) { - buf.append(' ').append(attr).append("=\""); - buf.append(escape(value)); - buf.append('"'); + xmlWriter.writeAttribute(attr, value); } } - private void createVersionAttr(StringBuilder buffer) { - buffer.append("Allowed characters are: + *
+ * Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] + * // any Unicode character, excluding the surrogate blocks, FFFE, and FFFF. + *
+ * (see Extensible Markup Language (XML) 1.0 (Fifth Edition)). */ - private String escape(String text) { - String result = StringEscapeUtils.escapeXml10(text); + private String removeInvalidCharacters(String text) { + Pattern pattern = Pattern.compile( + "\\x00|\\x01|\\x02|\\x03|\\x04|\\x05|\\x06|\\x07|\\x08|" + + "\\x0b|\\x0c|\\x0e|\\x0f|" + + "\\x10|\\x11|\\x12|\\x13|\\x14|\\x15|\\x16|\\x17|\\x18|" + + "\\x19|\\x1a|\\x1b|\\x1c|\\x1d|\\x1e|\\x1f"); + Matcher matcher = pattern.matcher(text); + return matcher.replaceAll(""); + } + + @Override + public void setWriter(final Writer writer) { String encoding = getProperty(ENCODING); - if (!"UTF-8".equalsIgnoreCase(encoding)) { - StringBuilder sb = new StringBuilder(result); - for (int i = 0; i < sb.length(); i++) { - char c = sb.charAt(i); - // surrogate characters are not allowed in XML - if (Character.isHighSurrogate(c)) { - char low = sb.charAt(i + 1); - int codepoint = Character.toCodePoint(c, low); - sb.replace(i, i + 2, "&#x" + Integer.toHexString(codepoint) + ";"); - } else if (c > 0xff) { - sb.replace(i, i + 1, "&#x" + Integer.toHexString((int) c) + ";"); - } - } - result = sb.toString(); + // for backwards compatibility, create a OutputStream that writes to the writer. + this.stream = new WriterOutputStream(writer, encoding); + + XMLOutputFactory outputFactory = XMLOutputFactory.newFactory(); + try { + this.xmlWriter = outputFactory.createXMLStreamWriter(this.stream, encoding); + // for backwards compatibility, also provide a writer. + // Note: both XMLStreamWriter and this writer will write to this.stream + this.writer = new WrappedOutputStreamWriter(xmlWriter, stream, encoding); + } catch (XMLStreamException | UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } + + private static class WrappedOutputStreamWriter extends OutputStreamWriter { + private final XMLStreamWriter xmlWriter; + + WrappedOutputStreamWriter(XMLStreamWriter xmlWriter, OutputStream out, String charset) throws UnsupportedEncodingException { + super(out, charset); + this.xmlWriter = xmlWriter; + } + + @Override + public void flush() throws IOException { + try { + xmlWriter.flush(); + } catch (XMLStreamException e) { + throw new IOException(e); + } + super.flush(); + } + + @Override + public void close() throws IOException { + try { + xmlWriter.close(); + } catch (XMLStreamException e) { + throw new IOException(e); + } + super.close(); } - return result; } // FIXME: elapsed time not available until the end of the processing diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XSLTRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XSLTRenderer.java index 28a3bd2132..b9f6ce6791 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XSLTRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XSLTRenderer.java @@ -45,6 +45,7 @@ public class XSLTRenderer extends XMLRenderer { private Transformer transformer; private String xsltFilename = "/pmd-nicerhtml.xsl"; private Writer outputWriter; + private StringWriter stringWriter; public XSLTRenderer() { super(); @@ -71,8 +72,8 @@ public class XSLTRenderer extends XMLRenderer { // We keep the inital writer to put the final html output this.outputWriter = getWriter(); // We use a new one to store the XML... - Writer w = new StringWriter(); - setWriter(w); + this.stringWriter = new StringWriter(); + setWriter(stringWriter); // If don't find the xsl no need to bother doing the all report, // so we check this here... InputStream xslt = null; @@ -100,16 +101,14 @@ public class XSLTRenderer extends XMLRenderer { * The stylesheet provided as an InputStream */ private void prepareTransformer(InputStream xslt) { - if (xslt != null) { - try { - // Get a TransformerFactory object - TransformerFactory factory = TransformerFactory.newInstance(); - StreamSource src = new StreamSource(xslt); - // Get an XSL Transformer object - this.transformer = factory.newTransformer(src); - } catch (TransformerConfigurationException e) { - e.printStackTrace(); - } + try { + // Get a TransformerFactory object + TransformerFactory factory = TransformerFactory.newInstance(); + StreamSource src = new StreamSource(xslt); + // Get an XSL Transformer object + this.transformer = factory.newTransformer(src); + } catch (TransformerConfigurationException e) { + throw new RuntimeException(e); } } @@ -118,25 +117,17 @@ public class XSLTRenderer extends XMLRenderer { // First we finish the XML report super.end(); // Now we transform it using XSLT - if (writer instanceof StringWriter) { - StringWriter w = (StringWriter) writer; - Document doc = this.getDocument(w.toString()); - this.transform(doc); - } else { - // Should not happen ! - throw new RuntimeException("Wrong writer"); - } - + Document doc = this.getDocument(stringWriter.toString()); + this.transform(doc); } private void transform(Document doc) { DOMSource source = new DOMSource(doc); - this.setWriter(new StringWriter()); StreamResult result = new StreamResult(this.outputWriter); try { transformer.transform(source, result); } catch (TransformerException e) { - e.printStackTrace(); + throw new RuntimeException(e); } } @@ -145,8 +136,7 @@ public class XSLTRenderer extends XMLRenderer { DocumentBuilder parser = DocumentBuilderFactory.newInstance().newDocumentBuilder(); return parser.parse(new InputSource(new StringReader(xml))); } catch (ParserConfigurationException | SAXException | IOException e) { - e.printStackTrace(); + throw new RuntimeException(e); } - return null; } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java index e3f38e8b8c..390d3a812e 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java @@ -125,10 +125,10 @@ public class XMLRendererTest extends AbstractRendererTest { public String getHeader() { return "" + PMD.EOL - + "" + PMD.EOL; + + "" + PMD.EOL; } @Test diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/ant/PMDTaskTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/ant/PMDTaskTest.java index 3250e895eb..82802e409c 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/ant/PMDTaskTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/ant/PMDTaskTest.java @@ -8,10 +8,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.UnsupportedEncodingException; -import java.lang.reflect.Field; import java.nio.charset.Charset; import java.util.Locale; -import java.util.Objects; import org.apache.commons.io.FileUtils; import org.junit.Rule; @@ -112,17 +110,8 @@ public class PMDTaskTest extends AbstractAntTestHelper { } }; - // See http://stackoverflow.com/questions/361975/setting-the-default-java-character-encoding and http://stackoverflow.com/a/14987992/1169968 private static void setDefaultCharset(String charsetName) { - try { - System.setProperty("file.encoding", charsetName); - Field charset = Charset.class.getDeclaredField("defaultCharset"); - charset.setAccessible(true); - charset.set(null, null); - Objects.requireNonNull(Charset.defaultCharset()); - } catch (Exception e) { - throw new RuntimeException(e); - } + System.setProperty("file.encoding", charsetName); } @Rule