From 7bf99fb496db5f6e17817568cf305c55b77c7ddb Mon Sep 17 00:00:00 2001 From: Ashish Chopra Date: Tue, 25 Feb 2020 11:26:16 +0530 Subject: [PATCH 1/4] #526 Preventing SSRF due to external resource refs in SVGs --- imageio/imageio-batik/pom.xml | 2 +- .../imageio/plugins/svg/SVGImageReader.java | 13 ++++++++++ .../imageio/plugins/svg/SVGReadParam.java | 14 ++++++++++ .../plugins/svg/SVGImageReaderTest.java | 26 +++++++++++++++++-- 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/imageio/imageio-batik/pom.xml b/imageio/imageio-batik/pom.xml index 385f6660..8e162958 100644 --- a/imageio/imageio-batik/pom.xml +++ b/imageio/imageio-batik/pom.xml @@ -88,6 +88,6 @@ - 1.9 + 1.12 diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java index c55012de..5b776f14 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java @@ -80,6 +80,7 @@ import java.util.Map; */ public class SVGImageReader extends ImageReaderBase { private Rasterizer rasterizer; + private boolean allowExternalResources; /** * Creates an {@code SVGImageReader}. @@ -88,6 +89,7 @@ public class SVGImageReader extends ImageReaderBase { */ public SVGImageReader(final ImageReaderSpi pProvider) { super(pProvider); + allowExternalResources = true; } protected void resetMembers() { @@ -116,6 +118,9 @@ public class SVGImageReader extends ImageReaderBase { if (pParam instanceof SVGReadParam) { SVGReadParam svgParam = (SVGReadParam) pParam; + // set the external-resource-resolution preference + allowExternalResources = svgParam.shouldAllowExternalResources(); + // Get the base URI // This must be done before converting the params to hints String baseURI = svgParam.getBaseURI(); @@ -641,6 +646,14 @@ public class SVGImageReader extends ImageReaderBase { public void displayMessage(String message) { processWarningOccurred(message.replaceAll("[\\r\\n]+", " ")); } + + @Override + public ExternalResourceSecurity getExternalResourceSecurity(ParsedURL resourceURL, ParsedURL docURL) { + if (allowExternalResources) { + return super.getExternalResourceSecurity(resourceURL, docURL); + } + return new NoLoadExternalResourceSecurity(); + } } } } diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java index 2d5ad4dd..12ec7899 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java @@ -41,6 +41,12 @@ import java.awt.*; public class SVGReadParam extends ImageReadParam { private Paint background; private String baseURI; + private boolean allowExternalResources; + + public SVGReadParam() { + super(); + allowExternalResources = true; + } public Paint getBackgroundColor() { return background; @@ -58,6 +64,14 @@ public class SVGReadParam extends ImageReadParam { baseURI = pBaseURI; } + public void allowExternalResources(boolean bAllow) { + allowExternalResources = bAllow; + } + + public boolean shouldAllowExternalResources() { + return allowExternalResources; + } + @Override public boolean canSetSourceRenderSize() { return true; diff --git a/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java b/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java index 21c30f3b..9a2c8863 100755 --- a/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java +++ b/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java @@ -49,7 +49,6 @@ import java.io.IOException; import java.io.InputStream; import java.net.URISyntaxException; import java.net.URL; -import java.nio.Buffer; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -67,6 +66,7 @@ import static org.mockito.Mockito.*; * @version $Id: SVGImageReaderTest.java,v 1.0 Apr 1, 2008 10:39:17 PM haraldk Exp$ */ public class SVGImageReaderTest extends ImageReaderAbstractTest { + private SVGImageReaderSpi provider = new SVGImageReaderSpi(); protected List getTestData() { @@ -306,4 +306,26 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest reader.dispose(); } } -} \ No newline at end of file + + @Test(expected = SecurityException.class) + public void testDisallowedExternalResources() throws URISyntaxException, IOException { + URL resource = getClassLoaderResource("/svg/barChart.svg"); + + SVGImageReader reader = createReader(); + + TestData data = new TestData(resource, (Dimension) null); + try (ImageInputStream stream = data.getInputStream()) { + reader.setInput(stream); + + SVGReadParam param = reader.getDefaultReadParam(); + param.setBaseURI(resource.toURI().toASCIIString()); + param.allowExternalResources(false); + // `reader.read` for `/svg/barChart.svg` should raise + // a SecurityException when External Resources are blocked + reader.read(0, param); + } + finally { + reader.dispose(); + } + } +} From 872523b0f03b3c9ac66286afbbf385a7a4e0be5a Mon Sep 17 00:00:00 2001 From: Ashish Chopra Date: Tue, 24 Mar 2020 18:40:01 +0530 Subject: [PATCH 2/4] #526 Incorporating review comments * renaming accessors * changing the default to disallow external resources * introduced system-property for backwards compatibility * honor system-property (if present) and SVGReadParams.isAllowExternalResources hasn't been called (as against ignoring system-property and reverting to 'block-by-default' if SVGReadParams.isAllowExternalResources invoked) * added + updated test cases --- .../imageio/plugins/svg/SVGImageReader.java | 9 +- .../imageio/plugins/svg/SVGReadParam.java | 18 ++- .../plugins/svg/SVGImageReaderTest.java | 119 ++++++++++++------ 3 files changed, 97 insertions(+), 49 deletions(-) diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java index 5b776f14..951b4dd6 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java @@ -79,8 +79,12 @@ import java.util.Map; * @see batik-dev */ public class SVGImageReader extends ImageReaderBase { + + static String ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP = "com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"; + private Rasterizer rasterizer; - private boolean allowExternalResources; + private boolean allowExternalResources = + "true".equalsIgnoreCase(System.getProperty(ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP)); /** * Creates an {@code SVGImageReader}. @@ -89,7 +93,6 @@ public class SVGImageReader extends ImageReaderBase { */ public SVGImageReader(final ImageReaderSpi pProvider) { super(pProvider); - allowExternalResources = true; } protected void resetMembers() { @@ -119,7 +122,7 @@ public class SVGImageReader extends ImageReaderBase { SVGReadParam svgParam = (SVGReadParam) pParam; // set the external-resource-resolution preference - allowExternalResources = svgParam.shouldAllowExternalResources(); + allowExternalResources = svgParam.isAllowExternalResources(); // Get the base URI // This must be done before converting the params to hints diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java index 12ec7899..78bbba71 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java @@ -41,11 +41,11 @@ import java.awt.*; public class SVGReadParam extends ImageReadParam { private Paint background; private String baseURI; - private boolean allowExternalResources; + private boolean allowExternalResources = false; + private boolean isAllowExternalResourcesSetExplicitly = false; public SVGReadParam() { super(); - allowExternalResources = true; } public Paint getBackgroundColor() { @@ -64,12 +64,18 @@ public class SVGReadParam extends ImageReadParam { baseURI = pBaseURI; } - public void allowExternalResources(boolean bAllow) { - allowExternalResources = bAllow; + public void setAllowExternalResources(boolean allow) { + allowExternalResources = allow; + isAllowExternalResourcesSetExplicitly = true; } - public boolean shouldAllowExternalResources() { - return allowExternalResources; + public boolean isAllowExternalResources() { + if (isAllowExternalResourcesSetExplicitly) { + return allowExternalResources; + } else { + // prefer the explicitly set value if invoked, read the system prop as a fallback if it wasn't + return "true".equals(System.getProperty(SVGImageReader.ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP)); + } } @Override diff --git a/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java b/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java index 9a2c8863..7785492a 100755 --- a/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java +++ b/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java @@ -228,6 +228,7 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest reader.addIIOReadWarningListener(listener); SVGReadParam param = reader.getDefaultReadParam(); + param.setAllowExternalResources(true); param.setBaseURI(resource.toURI().toASCIIString()); BufferedImage image = reader.read(0, param); @@ -244,71 +245,82 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest } @Test - public void testEmbeddedBeforeBaseURI() throws URISyntaxException, IOException { + public void testEmbeddedBeforeBaseURI_withSystemProperty() throws URISyntaxException, IOException { // Asking for metadata, width, height etc, before attempting to read using a param, // will cause the document to be parsed without a base URI. // This will work, but may not use the CSS... + // since the param is not available before the read operation is invoked, + // this test-case MUST use the system-property for backwards compatibility URL resource = getClassLoaderResource("/svg/barChart.svg"); - SVGImageReader reader = createReader(); + try { + System.setProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources", "true"); + SVGImageReader reader = createReader(); - TestData data = new TestData(resource, (Dimension) null); - try (ImageInputStream stream = data.getInputStream()) { - reader.setInput(stream); + TestData data = new TestData(resource, (Dimension) null); + try (ImageInputStream stream = data.getInputStream()) { + reader.setInput(stream); - IIOReadWarningListener listener = mock(IIOReadWarningListener.class); - reader.addIIOReadWarningListener(listener); + IIOReadWarningListener listener = mock(IIOReadWarningListener.class); + reader.addIIOReadWarningListener(listener); - assertEquals(450, reader.getWidth(0)); - assertEquals(500, reader.getHeight(0)); + assertEquals(450, reader.getWidth(0)); + assertEquals(500, reader.getHeight(0)); - // Expect the warning about the missing CSS - verify(listener, atMost(1)).warningOccurred(any(ImageReader.class), anyString()); - reset(listener); + // Expect the warning about the missing CSS + verify(listener, atMost(1)).warningOccurred(any(ImageReader.class), anyString()); + reset(listener); - SVGReadParam param = reader.getDefaultReadParam(); - param.setBaseURI(resource.toURI().toASCIIString()); - BufferedImage image = reader.read(0, param); + SVGReadParam param = reader.getDefaultReadParam(); + param.setBaseURI(resource.toURI().toASCIIString()); + BufferedImage image = reader.read(0, param); - assertNotNull(image); - assertEquals(450, image.getWidth()); - assertEquals(500, image.getHeight()); + assertNotNull(image); + assertEquals(450, image.getWidth()); + assertEquals(500, image.getHeight()); - // No more warnings now that the base URI is set - verifyZeroInteractions(listener); - } - finally { - reader.dispose(); + // No more warnings now that the base URI is set + verifyZeroInteractions(listener); + } + finally { + reader.dispose(); + } + } finally { + System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"); } } @Test - public void testEmbeddedNoBaseURI() throws IOException { + public void testEmbeddedNoBaseURI_withSystemProperty() throws IOException { // With no base URI, we will throw an exception, about the missing embedded resource URL resource = getClassLoaderResource("/svg/barChart.svg"); - SVGImageReader reader = createReader(); + try { + System.setProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources", "true"); - TestData data = new TestData(resource, (Dimension) null); - try (ImageInputStream stream = data.getInputStream()) { - reader.setInput(stream); + SVGImageReader reader = createReader(); - BufferedImage image = reader.read(0); + TestData data = new TestData(resource, (Dimension) null); + try (ImageInputStream stream = data.getInputStream()) { + reader.setInput(stream); - assertNotNull(image); - assertEquals(450, image.getWidth()); - assertEquals(500, image.getHeight()); - } - catch (IIOException allowed) { - assertTrue(allowed.getMessage().contains("batikLogo.svg")); // The embedded resource we don't find - } - finally { - reader.dispose(); + reader.read(0); + + assertTrue("reader.read should've thrown an exception, but didn't", false); + } + catch (IIOException allowed) { + assertTrue(allowed.getMessage().contains("batikLogo.svg")); // The embedded resource we don't find + } + finally { + reader.dispose(); + } + } finally { + System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"); } } @Test(expected = SecurityException.class) - public void testDisallowedExternalResources() throws URISyntaxException, IOException { + public void testDefaultDisallowedExternalResources() throws URISyntaxException, IOException { URL resource = getClassLoaderResource("/svg/barChart.svg"); SVGImageReader reader = createReader(); @@ -319,7 +331,6 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest SVGReadParam param = reader.getDefaultReadParam(); param.setBaseURI(resource.toURI().toASCIIString()); - param.allowExternalResources(false); // `reader.read` for `/svg/barChart.svg` should raise // a SecurityException when External Resources are blocked reader.read(0, param); @@ -328,4 +339,32 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest reader.dispose(); } } + + @Test(expected = SecurityException.class) + public void testDisallowedExternalResources_withSystemProperty() throws URISyntaxException, IOException { + URL resource = getClassLoaderResource("/svg/barChart.svg"); + try { + System.setProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources", "true"); + SVGImageReader reader = createReader(); + + TestData data = new TestData(resource, (Dimension) null); + try (ImageInputStream stream = data.getInputStream()) { + reader.setInput(stream); + + SVGReadParam param = reader.getDefaultReadParam(); + param.setBaseURI(resource.toURI().toASCIIString()); + param.setAllowExternalResources(false); + // even when the system-property is set to true, + // `reader.read` for `/svg/barChart.svg` should raise + // a SecurityException when External Resources are blocked + // because the API invocation gets preference + reader.read(0, param); + } + finally { + reader.dispose(); + } + } finally { + System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"); + } + } } From 5315caf83087c39531daaaa8760eddbb78404cb1 Mon Sep 17 00:00:00 2001 From: Ashish Chopra Date: Mon, 30 Mar 2020 10:38:52 +0530 Subject: [PATCH 3/4] #526 Fixed unnecessary 'overenginnering' in previous commit :facepalm: --- .../imageio/plugins/svg/SVGReadParam.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java index 78bbba71..0f7779ca 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java @@ -41,8 +41,8 @@ import java.awt.*; public class SVGReadParam extends ImageReadParam { private Paint background; private String baseURI; - private boolean allowExternalResources = false; - private boolean isAllowExternalResourcesSetExplicitly = false; + private boolean allowExternalResources = + "true".equals(System.getProperty(SVGImageReader.ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP)); public SVGReadParam() { super(); @@ -66,16 +66,10 @@ public class SVGReadParam extends ImageReadParam { public void setAllowExternalResources(boolean allow) { allowExternalResources = allow; - isAllowExternalResourcesSetExplicitly = true; } public boolean isAllowExternalResources() { - if (isAllowExternalResourcesSetExplicitly) { - return allowExternalResources; - } else { - // prefer the explicitly set value if invoked, read the system prop as a fallback if it wasn't - return "true".equals(System.getProperty(SVGImageReader.ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP)); - } + return allowExternalResources; } @Override From 6642b1647a0d09678ec7f98eee313b6c9c67b6c3 Mon Sep 17 00:00:00 2001 From: Ashish Chopra Date: Mon, 30 Mar 2020 18:29:34 +0530 Subject: [PATCH 4/4] #526 Incorporating review comments * moved the system-property-evaluation to static-block for more reliability * updated impacted existing tests (which relied on external-resources) to leverage the new API * set the value of system-property controlling external-resource-access to true in the test JVM for the sake of existing tests using SVGImageReader APIs not backed by SVRReadParams (.getWidth/.getHeight and such) --- imageio/imageio-batik/pom.xml | 15 ++ .../imageio/plugins/svg/SVGImageReader.java | 6 +- .../imageio/plugins/svg/SVGReadParam.java | 3 +- .../plugins/svg/SVGImageReaderTest.java | 155 +++++++----------- 4 files changed, 79 insertions(+), 100 deletions(-) diff --git a/imageio/imageio-batik/pom.xml b/imageio/imageio-batik/pom.xml index 8e162958..3fcd58d4 100644 --- a/imageio/imageio-batik/pom.xml +++ b/imageio/imageio-batik/pom.xml @@ -14,6 +14,21 @@ See the Batik Home page for more information.]]> + + + + org.apache.maven.plugins + maven-surefire-plugin + + + + true + + + + + + diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java index 951b4dd6..8f0c717c 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReader.java @@ -80,11 +80,11 @@ import java.util.Map; */ public class SVGImageReader extends ImageReaderBase { - static String ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP = "com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"; + final static boolean DEFAULT_ALLOW_EXTERNAL_RESOURCES = + "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources")); private Rasterizer rasterizer; - private boolean allowExternalResources = - "true".equalsIgnoreCase(System.getProperty(ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP)); + private boolean allowExternalResources = DEFAULT_ALLOW_EXTERNAL_RESOURCES; /** * Creates an {@code SVGImageReader}. diff --git a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java index 0f7779ca..6fa67199 100755 --- a/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java +++ b/imageio/imageio-batik/src/main/java/com/twelvemonkeys/imageio/plugins/svg/SVGReadParam.java @@ -41,8 +41,7 @@ import java.awt.*; public class SVGReadParam extends ImageReadParam { private Paint background; private String baseURI; - private boolean allowExternalResources = - "true".equals(System.getProperty(SVGImageReader.ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP)); + private boolean allowExternalResources = SVGImageReader.DEFAULT_ALLOW_EXTERNAL_RESOURCES; public SVGReadParam() { super(); diff --git a/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java b/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java index 7785492a..d63c4262 100755 --- a/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java +++ b/imageio/imageio-batik/src/test/java/com/twelvemonkeys/imageio/plugins/svg/SVGImageReaderTest.java @@ -245,7 +245,7 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest } @Test - public void testEmbeddedBeforeBaseURI_withSystemProperty() throws URISyntaxException, IOException { + public void testEmbeddedBeforeBaseURI() throws URISyntaxException, IOException { // Asking for metadata, width, height etc, before attempting to read using a param, // will cause the document to be parsed without a base URI. // This will work, but may not use the CSS... @@ -253,87 +253,56 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest // this test-case MUST use the system-property for backwards compatibility URL resource = getClassLoaderResource("/svg/barChart.svg"); - try { - System.setProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources", "true"); - SVGImageReader reader = createReader(); - - TestData data = new TestData(resource, (Dimension) null); - try (ImageInputStream stream = data.getInputStream()) { - reader.setInput(stream); - - IIOReadWarningListener listener = mock(IIOReadWarningListener.class); - reader.addIIOReadWarningListener(listener); - - assertEquals(450, reader.getWidth(0)); - assertEquals(500, reader.getHeight(0)); - - // Expect the warning about the missing CSS - verify(listener, atMost(1)).warningOccurred(any(ImageReader.class), anyString()); - reset(listener); - - SVGReadParam param = reader.getDefaultReadParam(); - param.setBaseURI(resource.toURI().toASCIIString()); - BufferedImage image = reader.read(0, param); - - assertNotNull(image); - assertEquals(450, image.getWidth()); - assertEquals(500, image.getHeight()); - - // No more warnings now that the base URI is set - verifyZeroInteractions(listener); - } - finally { - reader.dispose(); - } - } finally { - System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"); - } - } - - @Test - public void testEmbeddedNoBaseURI_withSystemProperty() throws IOException { - // With no base URI, we will throw an exception, about the missing embedded resource - URL resource = getClassLoaderResource("/svg/barChart.svg"); - - try { - System.setProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources", "true"); - - SVGImageReader reader = createReader(); - - TestData data = new TestData(resource, (Dimension) null); - try (ImageInputStream stream = data.getInputStream()) { - reader.setInput(stream); - - reader.read(0); - - assertTrue("reader.read should've thrown an exception, but didn't", false); - } - catch (IIOException allowed) { - assertTrue(allowed.getMessage().contains("batikLogo.svg")); // The embedded resource we don't find - } - finally { - reader.dispose(); - } - } finally { - System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"); - } - } - - @Test(expected = SecurityException.class) - public void testDefaultDisallowedExternalResources() throws URISyntaxException, IOException { - URL resource = getClassLoaderResource("/svg/barChart.svg"); - SVGImageReader reader = createReader(); TestData data = new TestData(resource, (Dimension) null); try (ImageInputStream stream = data.getInputStream()) { reader.setInput(stream); + IIOReadWarningListener listener = mock(IIOReadWarningListener.class); + reader.addIIOReadWarningListener(listener); + + assertEquals(450, reader.getWidth(0)); + assertEquals(500, reader.getHeight(0)); + + // Expect the warning about the missing CSS + verify(listener, atMost(1)).warningOccurred(any(ImageReader.class), anyString()); + reset(listener); + SVGReadParam param = reader.getDefaultReadParam(); param.setBaseURI(resource.toURI().toASCIIString()); - // `reader.read` for `/svg/barChart.svg` should raise - // a SecurityException when External Resources are blocked - reader.read(0, param); + BufferedImage image = reader.read(0, param); + + assertNotNull(image); + assertEquals(450, image.getWidth()); + assertEquals(500, image.getHeight()); + + // No more warnings now that the base URI is set + verifyZeroInteractions(listener); + } + finally { + reader.dispose(); + } + } + + @Test + public void testEmbeddedNoBaseURI() throws IOException { + // With no base URI, we will throw an exception, about the missing embedded resource + URL resource = getClassLoaderResource("/svg/barChart.svg"); + SVGImageReader reader = createReader(); + + TestData data = new TestData(resource, (Dimension) null); + try (ImageInputStream stream = data.getInputStream()) { + reader.setInput(stream); + + SVGReadParam params = reader.getDefaultReadParam(); + params.setAllowExternalResources(true); + reader.read(0, params); + + assertTrue("reader.read should've thrown an exception, but didn't", false); + } + catch (IIOException allowed) { + assertTrue(allowed.getMessage().contains("batikLogo.svg")); // The embedded resource we don't find } finally { reader.dispose(); @@ -341,30 +310,26 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest } @Test(expected = SecurityException.class) - public void testDisallowedExternalResources_withSystemProperty() throws URISyntaxException, IOException { + public void testDisallowedExternalResources() throws URISyntaxException, IOException { + // system-property set to true in surefire-plugin-settings in the pom URL resource = getClassLoaderResource("/svg/barChart.svg"); - try { - System.setProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources", "true"); - SVGImageReader reader = createReader(); + SVGImageReader reader = createReader(); - TestData data = new TestData(resource, (Dimension) null); - try (ImageInputStream stream = data.getInputStream()) { - reader.setInput(stream); + TestData data = new TestData(resource, (Dimension) null); + try (ImageInputStream stream = data.getInputStream()) { + reader.setInput(stream); - SVGReadParam param = reader.getDefaultReadParam(); - param.setBaseURI(resource.toURI().toASCIIString()); - param.setAllowExternalResources(false); - // even when the system-property is set to true, - // `reader.read` for `/svg/barChart.svg` should raise - // a SecurityException when External Resources are blocked - // because the API invocation gets preference - reader.read(0, param); - } - finally { - reader.dispose(); - } - } finally { - System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources"); + SVGReadParam param = reader.getDefaultReadParam(); + param.setBaseURI(resource.toURI().toASCIIString()); + param.setAllowExternalResources(false); + // even when the system-property is set to true, + // `reader.read` for `/svg/barChart.svg` should raise + // a SecurityException when External Resources are blocked + // because the API invocation gets preference + reader.read(0, param); + } + finally { + reader.dispose(); } } }