#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)
This commit is contained in:
Ashish Chopra 2020-03-30 18:29:34 +05:30
parent 5315caf830
commit 6642b1647a
4 changed files with 79 additions and 100 deletions

View File

@ -14,6 +14,21 @@
See the <a href="http://xmlgraphics.apache.org/batik/">Batik Home page</a> See the <a href="http://xmlgraphics.apache.org/batik/">Batik Home page</a>
for more information.]]> for more information.]]>
</description> </description>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<systemPropertyVariables>
<com.twelvemonkeys.imageio.plugins.svg.allowexternalresources>
true
</com.twelvemonkeys.imageio.plugins.svg.allowexternalresources>
</systemPropertyVariables>
</configuration>
</plugin>
</plugins>
</build>
<dependencies> <dependencies>
<dependency> <dependency>

View File

@ -80,11 +80,11 @@ import java.util.Map;
*/ */
public class SVGImageReader extends ImageReaderBase { 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 Rasterizer rasterizer;
private boolean allowExternalResources = private boolean allowExternalResources = DEFAULT_ALLOW_EXTERNAL_RESOURCES;
"true".equalsIgnoreCase(System.getProperty(ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP));
/** /**
* Creates an {@code SVGImageReader}. * Creates an {@code SVGImageReader}.

View File

@ -41,8 +41,7 @@ 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 = SVGImageReader.DEFAULT_ALLOW_EXTERNAL_RESOURCES;
"true".equals(System.getProperty(SVGImageReader.ALLOW_EXTERNAL_RESOURCES_SYSTEM_PROP));
public SVGReadParam() { public SVGReadParam() {
super(); super();

View File

@ -245,7 +245,7 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
} }
@Test @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, // 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...
@ -253,87 +253,56 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
// this test-case MUST use the system-property for backwards compatibility // this test-case MUST use the system-property for backwards compatibility
URL resource = getClassLoaderResource("/svg/barChart.svg"); 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(); 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);
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(); SVGReadParam param = reader.getDefaultReadParam();
param.setBaseURI(resource.toURI().toASCIIString()); param.setBaseURI(resource.toURI().toASCIIString());
// `reader.read` for `/svg/barChart.svg` should raise BufferedImage image = reader.read(0, param);
// a SecurityException when External Resources are blocked
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 { finally {
reader.dispose(); reader.dispose();
@ -341,30 +310,26 @@ public class SVGImageReaderTest extends ImageReaderAbstractTest<SVGImageReader>
} }
@Test(expected = SecurityException.class) @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"); URL resource = getClassLoaderResource("/svg/barChart.svg");
try { SVGImageReader reader = createReader();
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);
SVGReadParam param = reader.getDefaultReadParam(); SVGReadParam param = reader.getDefaultReadParam();
param.setBaseURI(resource.toURI().toASCIIString()); param.setBaseURI(resource.toURI().toASCIIString());
param.setAllowExternalResources(false); param.setAllowExternalResources(false);
// even when the system-property is set to true, // even when the system-property is set to true,
// `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
// because the API invocation gets preference // because the API invocation gets preference
reader.read(0, param); reader.read(0, param);
} }
finally { finally {
reader.dispose(); reader.dispose();
}
} finally {
System.clearProperty("com.twelvemonkeys.imageio.plugins.svg.allowexternalresources");
} }
} }
} }