Merge branch 'pr-37'
This closes #37
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 052f04a..c2ae80f 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -45,6 +45,9 @@
</properties>
<body>
<release version="1.0-alpha2" date="2020-??-??" description="Second 1.0 alpha release">
+ <action issue="IMAGING-248" dev="kinow" type="add" due-to="Greg Shrago">
+ ICNS: missing element types; some safety checks
+ </action>
<action issue="IMAGING-246" dev="kinow" type="fix">
Invalid Block Size error prevents handling of block 1084, Macintosh NSPrintInfo
</action>
diff --git a/src/main/java/org/apache/commons/imaging/formats/icns/IcnsDecoder.java b/src/main/java/org/apache/commons/imaging/formats/icns/IcnsDecoder.java
index 664e194..c68b544 100644
--- a/src/main/java/org/apache/commons/imaging/formats/icns/IcnsDecoder.java
+++ b/src/main/java/org/apache/commons/imaging/formats/icns/IcnsDecoder.java
@@ -21,6 +21,7 @@
import java.util.List;
import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.Imaging;
import org.apache.commons.imaging.common.ImageBuilder;
import org.apache.commons.imaging.formats.icns.IcnsImageParser.IcnsElement;
@@ -191,66 +192,77 @@
}
public static List<BufferedImage> decodeAllImages(final IcnsImageParser.IcnsElement[] icnsElements)
- throws ImageReadException {
+ throws ImageReadException {
final List<BufferedImage> result = new ArrayList<>();
- for (final IcnsElement imageElement : icnsElements) {
- final IcnsType imageType = IcnsType.findImageType(imageElement.type);
- if (imageType == null) {
- continue;
+ for (int i = 0; i < icnsElements.length; i++) {
+ BufferedImage image = decodeImage(icnsElements, i);
+ if (image != null) {
+ result.add(image);
}
+ }
+ return result;
+ }
- IcnsType maskType;
- IcnsImageParser.IcnsElement maskElement = null;
- if (imageType.hasMask()) {
- maskType = imageType;
- maskElement = imageElement;
+ public static BufferedImage decodeImage(final IcnsImageParser.IcnsElement[] icnsElements, int index)
+ throws ImageReadException {
+ IcnsImageParser.IcnsElement imageElement = icnsElements[index];
+ final IcnsType imageType = IcnsType.findImageType(imageElement.type);
+ if (imageType == null) {
+ return null;
+ }
+
+ // PNG or JPEG 2000
+ if (imageType == IcnsType.ICNS_16x16_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_32x32_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_64x64_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_128x128_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_256x256_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_512x512_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_1024x1024_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_32x32_2x_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_64x64_2x_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_256x256_2x_32BIT_ARGB_IMAGE
+ || imageType == IcnsType.ICNS_512x512_2x_32BIT_ARGB_IMAGE) {
+ BufferedImage image = null;
+ try {
+ image = Imaging.getBufferedImage(imageElement.data);
+ } catch (Exception ex) {
+ if (imageType.getWidth() <= 32) {
+ try {
+ image = decodeImageImpl(imageType, imageElement, icnsElements);
+ } catch (Exception ignored) { }
+ }
+ if (image == null) {
+ image = new BufferedImage(imageType.getWidth(), imageType.getHeight(), BufferedImage.TYPE_INT_ARGB);
+ }
+ }
+ return image;
+ }
+
+ return decodeImageImpl(imageType, imageElement, icnsElements);
+ }
+
+ private static BufferedImage decodeImageImpl(IcnsType imageType,
+ IcnsElement imageElement,
+ IcnsElement[] icnsElements) throws ImageReadException {
+ final int expectedSize = (imageType.getWidth() * imageType.getHeight()
+ * imageType.getBitsPerPixel() + 7) / 8;
+ byte[] imageData;
+ if (imageElement.data.length < expectedSize) {
+ if (imageType.getBitsPerPixel() == 32) {
+ imageData = Rle24Compression.decompress(
+ imageType.getWidth(), imageType.getHeight(),
+ imageElement.data);
} else {
- maskType = IcnsType.find8BPPMaskType(imageType);
- if (maskType != null) {
- for (final IcnsElement icnsElement : icnsElements) {
- if (icnsElement.type == maskType.getType()) {
- maskElement = icnsElement;
- break;
- }
- }
- }
- if (maskElement == null) {
- maskType = IcnsType.find1BPPMaskType(imageType);
- if (maskType != null) {
- for (final IcnsElement icnsElement : icnsElements) {
- if (icnsElement.type == maskType.getType()) {
- maskElement = icnsElement;
- break;
- }
- }
- }
- }
+ throw new ImageReadException("Short image data but not a 32 bit compressed type");
}
+ } else {
+ imageData = imageElement.data;
+ }
- // FIXME: don't skip these when JPEG 2000 support is added:
- if (imageType == IcnsType.ICNS_256x256_32BIT_ARGB_IMAGE
- || imageType == IcnsType.ICNS_512x512_32BIT_ARGB_IMAGE) {
- continue;
- }
-
- final int expectedSize = (imageType.getWidth() * imageType.getHeight()
- * imageType.getBitsPerPixel() + 7) / 8;
- byte[] imageData;
- if (imageElement.data.length < expectedSize) {
- if (imageType.getBitsPerPixel() == 32) {
- imageData = Rle24Compression.decompress(
- imageType.getWidth(), imageType.getHeight(),
- imageElement.data);
- } else {
- throw new ImageReadException("Short image data but not a 32 bit compressed type");
- }
- } else {
- imageData = imageElement.data;
- }
-
- final ImageBuilder imageBuilder = new ImageBuilder(imageType.getWidth(),
- imageType.getHeight(), true);
- switch (imageType.getBitsPerPixel()) {
+ final ImageBuilder imageBuilder = new ImageBuilder(imageType.getWidth(),
+ imageType.getHeight(), true);
+ switch (imageType.getBitsPerPixel()) {
case 1:
decode1BPPImage(imageType, imageData, imageBuilder);
break;
@@ -265,20 +277,46 @@
break;
default:
throw new ImageReadException("Unsupported bit depth " + imageType.getBitsPerPixel());
- }
+ }
- if (maskElement != null) {
- if (maskType.getBitsPerPixel() == 1) {
- apply1BPPMask(maskElement.data, imageBuilder);
- } else if (maskType.getBitsPerPixel() == 8) {
- apply8BPPMask(maskElement.data, imageBuilder);
- } else {
- throw new ImageReadException("Unsupport mask bit depth " + maskType.getBitsPerPixel());
+ IcnsType maskType;
+ IcnsElement maskElement = null;
+ if (imageType.hasMask()) {
+ maskType = imageType;
+ maskElement = imageElement;
+ } else {
+ maskType = IcnsType.find8BPPMaskType(imageType);
+ if (maskType != null) {
+ for (final IcnsElement icnsElement : icnsElements) {
+ if (icnsElement.type == maskType.getType()) {
+ maskElement = icnsElement;
+ break;
+ }
}
}
-
- result.add(imageBuilder.getBufferedImage());
+ if (maskElement == null) {
+ maskType = IcnsType.find1BPPMaskType(imageType);
+ if (maskType != null) {
+ for (final IcnsElement icnsElement : icnsElements) {
+ if (icnsElement.type == maskType.getType()) {
+ maskElement = icnsElement;
+ break;
+ }
+ }
+ }
+ }
}
- return result;
+
+ if (maskElement != null) {
+ if (maskType.getBitsPerPixel() == 1) {
+ apply1BPPMask(maskElement.data, imageBuilder);
+ } else if (maskType.getBitsPerPixel() == 8) {
+ apply8BPPMask(maskElement.data, imageBuilder);
+ } else {
+ throw new ImageReadException("Unsupported mask bit depth " + maskType.getBitsPerPixel());
+ }
+ }
+
+ return imageBuilder.getBufferedImage();
}
}
diff --git a/src/main/java/org/apache/commons/imaging/formats/icns/IcnsImageParser.java b/src/main/java/org/apache/commons/imaging/formats/icns/IcnsImageParser.java
index faa87aa..0851774 100644
--- a/src/main/java/org/apache/commons/imaging/formats/icns/IcnsImageParser.java
+++ b/src/main/java/org/apache/commons/imaging/formats/icns/IcnsImageParser.java
@@ -186,21 +186,16 @@
}
}
- private IcnsElement readIcnsElement(final InputStream is) throws IOException {
- final int type = read4Bytes("Type", is, "Not a Valid ICNS File", getByteOrder()); // Icon type
- // (4 bytes)
- final int elementSize = read4Bytes("ElementSize", is, "Not a Valid ICNS File", getByteOrder()); // Length
- // of
- // data
- // (4
- // bytes),
- // in
- // bytes,
- // including
- // this
- // header
- final byte[] data = readBytes("Data", is, elementSize - 8,
- "Not a Valid ICNS File");
+ private IcnsElement readIcnsElement(final InputStream is, final int remainingSize) throws IOException {
+ // Icon type (4 bytes)
+ final int type = read4Bytes("Type", is, "Not a valid ICNS file", getByteOrder());
+ // Length of data (4 bytes), in bytes, including this header
+ final int elementSize = read4Bytes("ElementSize", is, "Not a valid ICNS file", getByteOrder());
+ if (elementSize > remainingSize) {
+ throw new IOException(String.format("Corrupted ICNS file: element size %d is greater than "
+ + "remaining size %d", elementSize, remainingSize));
+ }
+ final byte[] data = readBytes("Data", is, elementSize - 8, "Not a valid ICNS file");
return new IcnsElement(type, elementSize, data);
}
@@ -223,7 +218,7 @@
final List<IcnsElement> icnsElementList = new ArrayList<>();
for (int remainingSize = icnsHeader.fileSize - 8; remainingSize > 0;) {
- final IcnsElement icnsElement = readIcnsElement(is);
+ final IcnsElement icnsElement = readIcnsElement(is, remainingSize);
icnsElementList.add(icnsElement);
remainingSize -= icnsElement.elementSize;
}
diff --git a/src/main/java/org/apache/commons/imaging/formats/icns/IcnsType.java b/src/main/java/org/apache/commons/imaging/formats/icns/IcnsType.java
index 91b6468..7cefa09 100644
--- a/src/main/java/org/apache/commons/imaging/formats/icns/IcnsType.java
+++ b/src/main/java/org/apache/commons/imaging/formats/icns/IcnsType.java
@@ -31,6 +31,7 @@
ICNS_16x16_32BIT_IMAGE("is32", 16, 16, 32, false),
ICNS_32x32_8BIT_MASK("l8mk", 32, 32, 8, true),
+ ICNS_32x32_1BIT_IMAGE("ICON", 32, 32, 1, false),
ICNS_32x32_1BIT_IMAGE_AND_MASK("ICN#", 32, 32, 1, true),
ICNS_32x32_4BIT_IMAGE("icl4", 32, 32, 4, false),
ICNS_32x32_8BIT_IMAGE("icl8", 32, 32, 8, false),
@@ -45,9 +46,17 @@
ICNS_128x128_8BIT_MASK("t8mk", 128, 128, 8, true),
ICNS_128x128_32BIT_IMAGE("it32", 128, 128, 32, false),
+ ICNS_16x16_32BIT_ARGB_IMAGE("icp4", 16, 16, 32, false),
+ ICNS_32x32_32BIT_ARGB_IMAGE("icp5", 32, 32, 32, false),
+ ICNS_64x64_32BIT_ARGB_IMAGE("icp6", 64, 64, 32, false),
+ ICNS_128x128_32BIT_ARGB_IMAGE("ic07", 128, 128, 32, false),
ICNS_256x256_32BIT_ARGB_IMAGE("ic08", 256, 256, 32, false),
-
- ICNS_512x512_32BIT_ARGB_IMAGE("ic09", 512, 512, 32, false);
+ ICNS_512x512_32BIT_ARGB_IMAGE("ic09", 512, 512, 32, false),
+ ICNS_1024x1024_32BIT_ARGB_IMAGE("ic10", 1024, 1024, 32, false),
+ ICNS_32x32_2x_32BIT_ARGB_IMAGE("ic11", 32, 32, 32, false),
+ ICNS_64x64_2x_32BIT_ARGB_IMAGE("ic12", 64, 64, 32, false),
+ ICNS_256x256_2x_32BIT_ARGB_IMAGE("ic13", 256, 256, 32, false),
+ ICNS_512x512_2x_32BIT_ARGB_IMAGE("ic14", 512, 512, 32, false);
private static final IcnsType[] ALL_IMAGE_TYPES = {
ICNS_16x12_1BIT_IMAGE_AND_MASK,
@@ -57,6 +66,7 @@
ICNS_16x16_4BIT_IMAGE,
ICNS_16x16_8BIT_IMAGE,
ICNS_16x16_32BIT_IMAGE,
+ ICNS_32x32_1BIT_IMAGE,
ICNS_32x32_1BIT_IMAGE_AND_MASK,
ICNS_32x32_4BIT_IMAGE,
ICNS_32x32_8BIT_IMAGE,
@@ -66,8 +76,17 @@
ICNS_48x48_8BIT_IMAGE,
ICNS_48x48_32BIT_IMAGE,
ICNS_128x128_32BIT_IMAGE,
+ ICNS_16x16_32BIT_ARGB_IMAGE,
+ ICNS_32x32_32BIT_ARGB_IMAGE,
+ ICNS_64x64_32BIT_ARGB_IMAGE,
+ ICNS_128x128_32BIT_ARGB_IMAGE,
ICNS_256x256_32BIT_ARGB_IMAGE,
- ICNS_512x512_32BIT_ARGB_IMAGE};
+ ICNS_512x512_32BIT_ARGB_IMAGE,
+ ICNS_1024x1024_32BIT_ARGB_IMAGE,
+ ICNS_32x32_2x_32BIT_ARGB_IMAGE,
+ ICNS_64x64_2x_32BIT_ARGB_IMAGE,
+ ICNS_256x256_2x_32BIT_ARGB_IMAGE,
+ ICNS_512x512_2x_32BIT_ARGB_IMAGE};
private static final IcnsType[] ALL_MASK_TYPES = {
ICNS_16x12_1BIT_IMAGE_AND_MASK,
diff --git a/src/test/java/org/apache/commons/imaging/formats/icns/IcnsReadTest.java b/src/test/java/org/apache/commons/imaging/formats/icns/IcnsReadTest.java
index 14376f4..0c74ebb 100644
--- a/src/test/java/org/apache/commons/imaging/formats/icns/IcnsReadTest.java
+++ b/src/test/java/org/apache/commons/imaging/formats/icns/IcnsReadTest.java
@@ -17,18 +17,24 @@
package org.apache.commons.imaging.formats.icns;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.awt.image.BufferedImage;
import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.stream.Stream;
import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageReadException;
import org.apache.commons.imaging.Imaging;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
public class IcnsReadTest extends IcnsBaseTest {
@@ -37,6 +43,23 @@
return getIcnsImages().stream();
}
+ /**
+ * Provide data for tests of ICNS images with mono and/or JPEG png data. The
+ * logo in the issue IMAGING-248 was the groovy.icns. But the Python project
+ * also provides an image that contains a ic09 image.
+ *
+ * <p>icns2png was used to retrieve the number of images (i.e. icns2png -l
+ * image_file.icns, then counted the number of ico entries, ignoring the masks).</p>
+ * @return stream of test arguments
+ */
+ public static Stream<Arguments> provideIcnsImagesWithMonoAndJpegPngData() {
+ return Arrays
+ .asList(
+ Arguments.of("/images/icns/IMAGING-248/python.icns", 7),
+ Arguments.of("/images/icns/IMAGING-248/groovy.icns", 3))
+ .stream();
+ }
+
@Disabled(value = "RoundtripTest has to be fixed befor implementation can throw UnsupportedOperationException")
@ParameterizedTest
@MethodSource("data")
@@ -60,4 +83,18 @@
assertNotNull(image);
// TODO assert more
}
+
+ /**
+ * Test ICNS types such as mono (ICON) and some types for either JPEG2000 or PNG
+ * (icp4, icp5, ic11, etc). For IMAGING-248.
+ * @throws IOException if it fails to read the input stream
+ * @throws ImageReadException if the image is corrupted or invalid
+ */
+ @ParameterizedTest()
+ @MethodSource("provideIcnsImagesWithMonoAndJpegPngData")
+ public void testIcnsElementMonoPngJpeg(String file, int numberOfImages) throws ImageReadException, IOException {
+ File testFile = new File(IcnsReadTest.class.getResource(file).getFile());
+ List<BufferedImage> images = new IcnsImageParser().getAllBufferedImages(testFile);
+ assertEquals(numberOfImages, images.size());
+ }
}
diff --git a/src/test/resources/images/icns/IMAGING-248/groovy.icns b/src/test/resources/images/icns/IMAGING-248/groovy.icns
new file mode 100644
index 0000000..3e661dc
--- /dev/null
+++ b/src/test/resources/images/icns/IMAGING-248/groovy.icns
Binary files differ
diff --git a/src/test/resources/images/icns/IMAGING-248/python.icns b/src/test/resources/images/icns/IMAGING-248/python.icns
new file mode 100644
index 0000000..fc53e02
--- /dev/null
+++ b/src/test/resources/images/icns/IMAGING-248/python.icns
Binary files differ
