#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
This commit is contained in:
Ashish Chopra 2020-03-24 18:40:01 +05:30
parent 7bf99fb496
commit 872523b0f0
3 changed files with 97 additions and 49 deletions

View File

@ -79,8 +79,12 @@ import java.util.Map;
* @see <A href="http://www.mail-archive.com/batik-dev@xml.apache.org/msg00992.html">batik-dev</A> * @see <A href="http://www.mail-archive.com/batik-dev@xml.apache.org/msg00992.html">batik-dev</A>
*/ */
public class SVGImageReader extends ImageReaderBase { public class SVGImageReader extends ImageReaderBase {
static String ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP = "com.twelvemonkeys.imageio.plugins.svg.allowexternalresources";
private Rasterizer rasterizer; private Rasterizer rasterizer;
private boolean allowExternalResources; private boolean allowExternalResources =
"true".equalsIgnoreCase(System.getProperty(ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP));
/** /**
* Creates an {@code SVGImageReader}. * Creates an {@code SVGImageReader}.
@ -89,7 +93,6 @@ public class SVGImageReader extends ImageReaderBase {
*/ */
public SVGImageReader(final ImageReaderSpi pProvider) { public SVGImageReader(final ImageReaderSpi pProvider) {
super(pProvider); super(pProvider);
allowExternalResources = true;
} }
protected void resetMembers() { protected void resetMembers() {
@ -119,7 +122,7 @@ public class SVGImageReader extends ImageReaderBase {
SVGReadParam svgParam = (SVGReadParam) pParam; SVGReadParam svgParam = (SVGReadParam) pParam;
// set the external-resource-resolution preference // set the external-resource-resolution preference
allowExternalResources = svgParam.shouldAllowExternalResources(); allowExternalResources = svgParam.isAllowExternalResources();
// Get the base URI // Get the base URI
// This must be done before converting the params to hints // This must be done before converting the params to hints

View File

@ -41,11 +41,11 @@ import java.awt.*;
public class SVGReadParam extends ImageReadParam { public class SVGReadParam extends ImageReadParam {
private Paint background; private Paint background;
private String baseURI; private String baseURI;
private boolean allowExternalResources; private boolean allowExternalResources = false;
private boolean isAllowExternalResourcesSetExplicitly = false;
public SVGReadParam() { public SVGReadParam() {
super(); super();
allowExternalResources = true;
} }
public Paint getBackgroundColor() { public Paint getBackgroundColor() {
@ -64,12 +64,18 @@ public class SVGReadParam extends ImageReadParam {
baseURI = pBaseURI; baseURI = pBaseURI;
} }
public void allowExternalResources(boolean bAllow) { public void setAllowExternalResources(boolean allow) {
allowExternalResources = bAllow; allowExternalResources = allow;
isAllowExternalResourcesSetExplicitly = true;
} }
public boolean shouldAllowExternalResources() { public boolean isAllowExternalResources() {
return allowExternalResources; 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 @Override

View File

@ -228,6 +228,7 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
reader.addIIOReadWarningListener(listener); reader.addIIOReadWarningListener(listener);
SVGReadParam param = reader.getDefaultReadParam(); SVGReadParam param = reader.getDefaultReadParam();
param.setAllowExternalResources(true);
param.setBaseURI(resource.toURI().toASCIIString()); param.setBaseURI(resource.toURI().toASCIIString());
BufferedImage image = reader.read(0, param); BufferedImage image = reader.read(0, param);
@ -244,71 +245,82 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
} }
@Test @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, // Asking for metadata, width, height etc, before attempting to read using a param,
// will cause the document to be parsed without a base URI. // will cause the document to be parsed without a base URI.
// This will work, but may not use the CSS... // 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"); 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); TestData data = new TestData(resource, (Dimension) null);
try (ImageInputStream stream = data.getInputStream()) { try (ImageInputStream stream = data.getInputStream()) {
reader.setInput(stream); reader.setInput(stream);
IIOReadWarningListener listener = mock(IIOReadWarningListener.class); IIOReadWarningListener listener = mock(IIOReadWarningListener.class);
reader.addIIOReadWarningListener(listener); reader.addIIOReadWarningListener(listener);
assertEquals(450, reader.getWidth(0)); assertEquals(450, reader.getWidth(0));
assertEquals(500, reader.getHeight(0)); assertEquals(500, reader.getHeight(0));
// Expect the warning about the missing CSS // Expect the warning about the missing CSS
verify(listener, atMost(1)).warningOccurred(any(ImageReader.class), anyString()); verify(listener, atMost(1)).warningOccurred(any(ImageReader.class), anyString());
reset(listener); reset(listener);
SVGReadParam param = reader.getDefaultReadParam(); SVGReadParam param = reader.getDefaultReadParam();
param.setBaseURI(resource.toURI().toASCIIString()); param.setBaseURI(resource.toURI().toASCIIString());
BufferedImage image = reader.read(0, param); BufferedImage image = reader.read(0, param);
assertNotNull(image); assertNotNull(image);
assertEquals(450, image.getWidth()); assertEquals(450, image.getWidth());
assertEquals(500, image.getHeight()); assertEquals(500, image.getHeight());
// No more warnings now that the base URI is set // No more warnings now that the base URI is set
verifyZeroInteractions(listener); verifyZeroInteractions(listener);
} }
finally { finally {
reader.dispose(); reader.dispose();
}
} finally {
System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources");
} }
} }
@Test @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 // With no base URI, we will throw an exception, about the missing embedded resource
URL resource = getClassLoaderResource("/svg/barChart.svg"); 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); SVGImageReader reader = createReader();
try (ImageInputStream stream = data.getInputStream()) {
reader.setInput(stream);
BufferedImage image = reader.read(0); TestData data = new TestData(resource, (Dimension) null);
try (ImageInputStream stream = data.getInputStream()) {
reader.setInput(stream);
assertNotNull(image); reader.read(0);
assertEquals(450, image.getWidth());
assertEquals(500, image.getHeight()); assertTrue("reader.read should've thrown an exception, but didn't", false);
} }
catch (IIOException allowed) { catch (IIOException allowed) {
assertTrue(allowed.getMessage().contains("batikLogo.svg")); // The embedded resource we don't find assertTrue(allowed.getMessage().contains("batikLogo.svg")); // The embedded resource we don't find
} }
finally { finally {
reader.dispose(); reader.dispose();
}
} finally {
System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources");
} }
} }
@Test(expected = SecurityException.class) @Test(expected = SecurityException.class)
public void testDisallowedExternalResources() throws URISyntaxException, IOException { public void testDefaultDisallowedExternalResources() throws URISyntaxException, IOException {
URL resource = getClassLoaderResource("/svg/barChart.svg"); URL resource = getClassLoaderResource("/svg/barChart.svg");
SVGImageReader reader = createReader(); SVGImageReader reader = createReader();
@ -319,7 +331,6 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
SVGReadParam param = reader.getDefaultReadParam(); SVGReadParam param = reader.getDefaultReadParam();
param.setBaseURI(resource.toURI().toASCIIString()); param.setBaseURI(resource.toURI().toASCIIString());
param.allowExternalResources(false);
// `reader.read` for `/svg/barChart.svg` should raise // `reader.read` for `/svg/barChart.svg` should raise
// a SecurityException when External Resources are blocked // a SecurityException when External Resources are blocked
reader.read(0, param); reader.read(0, param);
@ -328,4 +339,32 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
reader.dispose(); 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");
}
}
} }