From 6d1fb3e4cd17fc6940d618b9c18c346279e51bb6 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 12 Sep 2024 09:44:54 +0200 Subject: [PATCH] [core] Fix PMD's XMLRenderer to escape CDATA Processing errors might contain inside their details message a CDATA section. This is output itself as a CDATA section, but XMLStreamWriter#writeCData doesn't escape it automatically - it just outputs the string as is. This results in invalid XML. Fixes #5059 --- docs/pages/release_notes.md | 2 ++ .../pmd/renderers/XMLRenderer.java | 9 ++++++- .../sourceforge/pmd/cpd/XMLRendererTest.java | 25 +++++++++++++++++++ .../pmd/renderers/XMLRendererTest.java | 16 ++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 53b4529c76..693284fc86 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,8 @@ This is a {{ site.pmd.release_type }} release. (ApexCRUDViolation, CognitiveComplexity, OperationWithLimitsInLoop) * [#5163](https://github.com/pmd/pmd/issues/5163): \[apex] Parser error when using toLabel in SOSL query * [#5182](https://github.com/pmd/pmd/issues/5182): \[apex] Parser error when using GROUPING in a SOQL query +* core + * [#5059](https://github.com/pmd/pmd/issues/5059): \[core] xml output doesn't escape CDATA inside its own CDATA * java * [#5190](https://github.com/pmd/pmd/issues/5190): \[java] NPE in type inference 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 b18c2ff0d0..3f60009838 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 @@ -196,7 +196,14 @@ public class XMLRenderer extends AbstractIncrementingRenderer { xmlWriter.writeAttribute("filename", determineFileName(pe.getFileId())); xmlWriter.writeAttribute("msg", pe.getMsg()); writeNewLine(); - xmlWriter.writeCData(pe.getDetail()); + + // in case the message contains itself some CDATA sections, they need to be handled + // in order to not produce invalid XML... + String detail = pe.getDetail(); + // split "]]>" into "]]" and ">" into two cdata sections + detail = detail.replace("]]>", "]]]]>"); + + xmlWriter.writeCData(detail); writeNewLine(); xmlWriter.writeEndElement(); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java index b22dc67307..05b511d0c0 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.cpd; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.ByteArrayInputStream; @@ -15,6 +16,7 @@ import java.io.StringReader; import java.io.StringWriter; import java.util.Collections; import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; @@ -316,6 +318,29 @@ class XMLRendererTest { assertEquals(processingError.getDetail(), textContent); } + /** + * Note, that CPD's processing error isn't output as a CDATA section at the moment. + * This test just makes sure, the XML is valid, in case {@code } is changed into CDATA. + * Currently, {@code >>]} is automatically escaped into {@code ]]>}. + * + * @see [core] xml output doesn't escape CDATA inside its own CDATA + */ + @Test + void cdataSectionInError() throws Exception { + FileId fileId = FileId.fromPathLikeString("file1.txt"); + Report.ProcessingError processingError = new Report.ProcessingError( + new LexException(2, 1, fileId, "test exception", new RuntimeException("Invalid source: ' ...'")), + fileId); + CPDReportRenderer renderer = new XMLRenderer(); + StringWriter sw = new StringWriter(); + renderer.render(CpdTestUtils.makeReport(Collections.emptyList(), Collections.emptyMap(), Collections.singletonList(processingError)), sw); + String report = sw.toString(); + assertReportIsValidSchema(report); + + DocumentBuilder documentBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + assertDoesNotThrow(() -> documentBuilder.parse(new InputSource(new StringReader(report)))); + } + private static void assertReportIsValidSchema(String report) throws SAXException, ParserConfigurationException, IOException { SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); Schema schema = schemaFactory.newSchema(new StreamSource(XMLRenderer.class.getResourceAsStream("/cpd-report_1_0_0.xsd"))); 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 d322d3885b..287e2bbaa6 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 @@ -4,6 +4,7 @@ package net.sourceforge.pmd.renderers; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -15,6 +16,7 @@ import java.io.StringReader; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Path; +import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import org.junit.jupiter.api.Test; @@ -26,6 +28,7 @@ import org.xml.sax.InputSource; import net.sourceforge.pmd.FooRule; import net.sourceforge.pmd.PMDVersion; import net.sourceforge.pmd.internal.util.IOUtil; +import net.sourceforge.pmd.lang.ast.ParseException; import net.sourceforge.pmd.lang.document.FileId; import net.sourceforge.pmd.lang.document.FileLocation; import net.sourceforge.pmd.lang.document.TextRange2d; @@ -161,6 +164,19 @@ class XMLRendererTest extends AbstractRendererTest { }); } + /** + * @see [core] xml output doesn't escape CDATA inside its own CDATA + */ + @Test + void cdataSectionInError() throws Exception { + ProcessingError processingError = new ProcessingError(new ParseException("Invalid source: ' ...'"), + FileId.fromPathLikeString("dummy.txt")); + String result = renderReport(getRenderer(), it -> it.onError(processingError), StandardCharsets.UTF_8); + + DocumentBuilder documentBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + assertDoesNotThrow(() -> documentBuilder.parse(new InputSource(new StringReader(result)))); + } + private String renderTempFile(Renderer renderer, Report report, Charset expectedCharset) throws IOException { File reportFile = folder.resolve("report.out").toFile();