From e8cf690f992d0bcd61c80bea95d7aa5e990b23b6 Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Tue, 27 Aug 2024 08:15:08 +0200 Subject: [PATCH 1/5] Add parameter "setTotalEntitySizeLimit" to XmlDecoder (#554) - add tests --- .../java/org/metafacture/xml/XmlDecoder.java | 24 +++++-- .../org/metafacture/xml/XmlDecoderTest.java | 69 +++++++++++++++++++ 2 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 metafacture-xml/src/test/java/org/metafacture/xml/XmlDecoderTest.java diff --git a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java index 1712c277..2e432ce2 100644 --- a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java +++ b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java @@ -48,18 +48,34 @@ public final class XmlDecoder extends DefaultObjectPipe { private static final String SAX_PROPERTY_LEXICAL_HANDLER = "http://xml.org/sax/properties/lexical-handler"; - - private final XMLReader saxReader; + private XMLReader saxReader; + private final SAXParserFactory parserFactory = SAXParserFactory.newInstance(); /** - * Constructs an XmlDecoder by obtaining a new instance of an + * Creates an instance of {@link XmlDecoder} by obtaining a new instance of an * {@link org.xml.sax.XMLReader}. */ public XmlDecoder() { try { - final SAXParserFactory parserFactory = SAXParserFactory.newInstance(); parserFactory.setNamespaceAware(true); + saxReader = parserFactory.newSAXParser().getXMLReader(); + } + catch (final ParserConfigurationException | SAXException e) { + throw new MetafactureException(e); + } + } + /** + * Sets the total entity size limit for the XML parser. + * See java-api-xml-processing-jaxp-security-guide.html + * + * Defaults to "50,000,000". Set to "0" to allow unlimited entities. + * + * @param size the size of the allowed entities. Set to "0" if entities should be unlimited. + */ + public void setTotalEntitySizeLimit(final String size) { + try { + System.setProperty("jdk.xml.totalEntitySizeLimit", size); saxReader = parserFactory.newSAXParser().getXMLReader(); } catch (final ParserConfigurationException | SAXException e) { diff --git a/metafacture-xml/src/test/java/org/metafacture/xml/XmlDecoderTest.java b/metafacture-xml/src/test/java/org/metafacture/xml/XmlDecoderTest.java new file mode 100644 index 00000000..7414aece --- /dev/null +++ b/metafacture-xml/src/test/java/org/metafacture/xml/XmlDecoderTest.java @@ -0,0 +1,69 @@ +/* + * Copyright 2024 Pascal Christoph (hbz) + * + * Licensed under the Apache License, Version 2.0 the "License"; + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.metafacture.xml; + +import org.junit.Before; +import org.junit.Test; +import org.metafacture.framework.MetafactureException; + +import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; + +/** + * Tests for class {@link XmlDecoder}. + * + * @author Pascal Christoph (dr0i) + */ +public final class XmlDecoderTest { + + private final String TEST_XML_WITH_TWO_ENTITIES = ">>"; + private XmlDecoder xmlDecoder; + private final Reader reader = new StringReader(TEST_XML_WITH_TWO_ENTITIES); + + @Before + public void initSystemUnderTest() { + xmlDecoder = new XmlDecoder(); + } + + @Test + public void issue554_default() { + process(xmlDecoder); + } + + @Test(expected = MetafactureException.class) + public void issue554_shouldFail() { + xmlDecoder.setTotalEntitySizeLimit("1"); + process(xmlDecoder); + } + + @Test + public void issue554_unlimitedEntities() { + xmlDecoder.setTotalEntitySizeLimit("0"); + process(xmlDecoder); + } + + private void process(XmlDecoder xmlDecoder) { + try { + xmlDecoder.process(reader); + reader.close(); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } +} From e277f8ad3ad9f8478cb4e9d8e3f12883aedf731a Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Fri, 30 Aug 2024 14:39:36 +0200 Subject: [PATCH 2/5] Collaps "catch" blocks --- .../src/main/java/org/metafacture/xml/XmlDecoder.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java index 2e432ce2..18d36d33 100644 --- a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java +++ b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java @@ -88,10 +88,7 @@ public void process(final Reader reader) { try { saxReader.parse(new InputSource(reader)); } - catch (final IOException e) { - throw new MetafactureException(e); - } - catch (final SAXException e) { + catch (final IOException | SAXException e) { throw new MetafactureException(e); } } @@ -105,10 +102,7 @@ protected void onSetReceiver() { try { saxReader.setProperty(SAX_PROPERTY_LEXICAL_HANDLER, getReceiver()); } - catch (final SAXNotRecognizedException e) { - throw new MetafactureException(e); - } - catch (final SAXNotSupportedException e) { + catch (final SAXNotRecognizedException | SAXNotSupportedException e) { throw new MetafactureException(e); } } From ee179578e61179a0fdb7cedac632ad1b96825486 Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Fri, 30 Aug 2024 15:38:39 +0200 Subject: [PATCH 3/5] Use XML API instead of setting parameter globally (#554) This has lesser implications. Also, some more declarations can be made "final". Thx @blackwinter. --- .../main/java/org/metafacture/xml/XmlDecoder.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java index 18d36d33..8e4be050 100644 --- a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java +++ b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java @@ -48,8 +48,8 @@ public final class XmlDecoder extends DefaultObjectPipe { private static final String SAX_PROPERTY_LEXICAL_HANDLER = "http://xml.org/sax/properties/lexical-handler"; - private XMLReader saxReader; - private final SAXParserFactory parserFactory = SAXParserFactory.newInstance(); + private static final String TOTAL_ENTITY_SIZE_LIMIT = "http://www.oracle.com/xml/jaxp/properties/totalEntitySizeLimit"; + private final XMLReader saxReader; /** * Creates an instance of {@link XmlDecoder} by obtaining a new instance of an @@ -57,6 +57,7 @@ public final class XmlDecoder extends DefaultObjectPipe { */ public XmlDecoder() { try { + final SAXParserFactory parserFactory = SAXParserFactory.newInstance(); parserFactory.setNamespaceAware(true); saxReader = parserFactory.newSAXParser().getXMLReader(); } @@ -71,14 +72,13 @@ public XmlDecoder() { * * Defaults to "50,000,000". Set to "0" to allow unlimited entities. * - * @param size the size of the allowed entities. Set to "0" if entities should be unlimited. + * @param totalEntitySizeLimit the size of the allowed entities. Set to "0" if entities should be unlimited. */ - public void setTotalEntitySizeLimit(final String size) { + public void setTotalEntitySizeLimit(final String totalEntitySizeLimit) { try { - System.setProperty("jdk.xml.totalEntitySizeLimit", size); - saxReader = parserFactory.newSAXParser().getXMLReader(); + saxReader.setProperty(TOTAL_ENTITY_SIZE_LIMIT, totalEntitySizeLimit); } - catch (final ParserConfigurationException | SAXException e) { + catch (final SAXException e) { throw new MetafactureException(e); } } From c7f8e0facd4b0729d059cb4b05a58608df7d6844 Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Fri, 30 Aug 2024 15:43:48 +0200 Subject: [PATCH 4/5] Add comment into @Description (#554) This brings the comment of how to set the variable to allow unlimited entities into flux-commands.md. --- .../src/main/java/org/metafacture/xml/XmlDecoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java index 8e4be050..e9da2d27 100644 --- a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java +++ b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java @@ -41,7 +41,7 @@ * @author Christoph Böhme * */ -@Description("Reads an XML file and passes the XML events to a receiver.") +@Description("Reads an XML file and passes the XML events to a receiver. Set 'totalEntitySizeLimit=\"0\"' to allow unlimited XML entities.") @In(Reader.class) @Out(XmlReceiver.class) @FluxCommand("decode-xml") From f0a618a9a64f6dbd3646c54481c530cfc0ec8ab4 Mon Sep 17 00:00:00 2001 From: Pascal Christoph Date: Mon, 2 Sep 2024 10:08:50 +0200 Subject: [PATCH 5/5] Update metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java Co-authored-by: Jens Wille --- .../src/main/java/org/metafacture/xml/XmlDecoder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java index e9da2d27..2fc11380 100644 --- a/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java +++ b/metafacture-xml/src/main/java/org/metafacture/xml/XmlDecoder.java @@ -41,7 +41,7 @@ * @author Christoph Böhme * */ -@Description("Reads an XML file and passes the XML events to a receiver. Set 'totalEntitySizeLimit=\"0\"' to allow unlimited XML entities.") +@Description("Reads an XML file and passes the XML events to a receiver. Set `totalEntitySizeLimit=\"0\"` to allow unlimited XML entities.") @In(Reader.class) @Out(XmlReceiver.class) @FluxCommand("decode-xml")