Bugzilla 53446 - Fixed some problems extracting PNGs

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1369263 13f79535-47bb-0310-9956-ffa450edef68

11 files changed, 187 insertions, 18 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 0602dfd241..d3a744b48e 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,7 +34,8 @@
 
     <changes>
         <release version="3.9-beta1" date="2012-??-??">
-          <action dev="poi-developers" type="fix">53205 - Fix some parsing errors and encoding issues in HDGF </action>
+          <action dev="poi-developers" type="add">53446 - Fixed some problems extracting PNGs </action>
+          <action dev="poi-developers" type="fix">53205 - Fixed some parsing errors and encoding issues in HDGF </action>
           <action dev="poi-developers" type="add">53204 - Improved performanceof PageSettingsBlock in HSSF </action>
           <action dev="poi-developers" type="add">53500 - Getter for repeating rows and columns</action>
           <action dev="poi-developers" type="fix">53369 - Fixed tests failing on JDK 1.7</action>
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFPictureData.java b/src/java/org/apache/poi/hssf/usermodel/HSSFPictureData.java
index 0516cccf6b..69bdae1521 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFPictureData.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFPictureData.java
@@ -22,6 +22,7 @@ import org.apache.poi.ddf.EscherBitmapBlip;
 import org.apache.poi.ddf.EscherBlipRecord;
 import org.apache.poi.ddf.EscherMetafileBlip;
 import org.apache.poi.ss.usermodel.PictureData;
+import org.apache.poi.util.PngUtils;
 
 /**
  * Represents binary data stored in the file.  Eg. A GIF, JPEG etc...
@@ -60,7 +61,18 @@ public class HSSFPictureData implements PictureData
      */
     public byte[] getData()
     {
-        return blip.getPicturedata();
+        byte[] pictureData = blip.getPicturedata();
+
+        //PNG created on MAC may have a 16-byte prefix which prevents successful reading.
+        //Just cut it off!.
+        if (PngUtils.matchesPngHeader(pictureData, 16))
+        {
+            byte[] png = new byte[pictureData.length-16];
+            System.arraycopy(pictureData, 16, png, 0, png.length);
+            pictureData = png;
+        }
+
+        return pictureData;
     }
 
     /**
diff --git a/src/java/org/apache/poi/util/PngUtils.java b/src/java/org/apache/poi/util/PngUtils.java
new file mode 100644
index 0000000000..785675830b
--- /dev/null
+++ b/src/java/org/apache/poi/util/PngUtils.java
@@ -0,0 +1,57 @@
+/* ====================================================================

+   Licensed to the Apache Software Foundation (ASF) under one or more

+   contributor license agreements.  See the NOTICE file distributed with

+   this work for additional information regarding copyright ownership.

+   The ASF licenses this file to You 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.apache.poi.util;

+

+import java.io.ByteArrayOutputStream;

+import java.io.IOException;

+import java.io.InputStream;

+import java.io.OutputStream;

+

+public final class PngUtils {

+

+    /**

+     * File header for PNG format.

+     */

+    private static final byte[] PNG_FILE_HEADER =

+        new byte[] { (byte) 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A };

+

+    private PngUtils() {

+        // no instances of this class

+    }

+

+    /**

+     * Checks if the offset matches the PNG header.

+     *

+     * @param data the data to check.

+     * @param offset the offset to check at.

+     * @return {@code true} if the offset matches.

+     */

+    public static boolean matchesPngHeader(byte[] data, int offset) {

+        if (data == null || data.length - offset < PNG_FILE_HEADER.length) {

+            return false;

+        }

+

+        for (int i = 0; i < PNG_FILE_HEADER.length; i++) {

+            if (PNG_FILE_HEADER[i] != data[i + offset]) {

+                return false;

+            }

+        }

+

+        return true;

+    }

+}

diff --git a/src/scratchpad/src/org/apache/poi/hslf/blip/PNG.java b/src/scratchpad/src/org/apache/poi/hslf/blip/PNG.java
index 20016fd6b3..12b98f1802 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/blip/PNG.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/blip/PNG.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.hslf.blip;
 
+import org.apache.poi.util.PngUtils;
 import org.apache.poi.hslf.model.Picture;
 import org.apache.poi.hslf.exceptions.HSLFException;
 
@@ -35,22 +36,19 @@ public final class PNG extends Bitmap {
     /**
      * @return PNG data
      */
-    public byte[] getData(){
-         byte[] data = super.getData();
-          try {
-              //PNG created on MAC may have a 16-byte prefix which prevents successful reading.
-              //Just cut it off!.
-              BufferedImage bi = ImageIO.read(new ByteArrayInputStream(data));
-              if (bi == null){
-                  byte[] png = new byte[data.length-16];
-                  System.arraycopy(data, 16, png, 0, png.length);
-                  data = png;
-              }
-          } catch (IOException e){
-              throw new HSLFException(e);
-          }
-         return data;
-     }
+    public byte[] getData() {
+        byte[] data = super.getData();
+
+        //PNG created on MAC may have a 16-byte prefix which prevents successful reading.
+        //Just cut it off!.
+        if (PngUtils.matchesPngHeader(data, 16)) {
+            byte[] png = new byte[data.length-16];
+            System.arraycopy(data, 16, png, 0, png.length);
+            data = png;
+        }
+
+        return data;
+    }
 
     /**
      * @return type of  this picture
diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java
index ff7af7cf21..670755dbf5 100644
--- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java
+++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java
@@ -34,6 +34,7 @@ import org.apache.poi.ddf.EscherProperty;
 import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.hwpf.model.PICF;
 import org.apache.poi.hwpf.model.PICFAndOfficeArtData;
+import org.apache.poi.util.PngUtils;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
 import org.apache.poi.util.StringUtil;
@@ -191,6 +192,15 @@ public final class Picture
         {
             // Raw data is not compressed.
             content = rawContent;
+
+            //PNG created on MAC may have a 16-byte prefix which prevents successful reading.
+            //Just cut it off!.
+            if (PngUtils.matchesPngHeader(content, 16))
+            {
+                byte[] png = new byte[content.length-16];
+                System.arraycopy(content, 16, png, 0, png.length);
+                content = png;
+            }
         }
     }
 
diff --git a/src/scratchpad/testcases/org/apache/poi/hslf/model/TestPicture.java b/src/scratchpad/testcases/org/apache/poi/hslf/model/TestPicture.java
index 871893210a..3444ae5fe8 100644
--- a/src/scratchpad/testcases/org/apache/poi/hslf/model/TestPicture.java
+++ b/src/scratchpad/testcases/org/apache/poi/hslf/model/TestPicture.java
@@ -20,10 +20,13 @@ package org.apache.poi.hslf.model;
 import java.awt.Graphics2D;
 import java.awt.Rectangle;
 import java.awt.image.BufferedImage;
+import java.io.ByteArrayInputStream;
+import javax.imageio.ImageIO;
 
 import junit.framework.TestCase;
 
 import org.apache.poi.ddf.EscherBSERecord;
+import org.apache.poi.hslf.HSLFSlideShow;
 import org.apache.poi.hslf.usermodel.PictureData;
 import org.apache.poi.hslf.usermodel.SlideShow;
 import org.apache.poi.POIDataSamples;
@@ -88,4 +91,41 @@ public final class TestPicture extends TestCase {
         Graphics2D graphics = img.createGraphics();
         pict.draw(graphics);
     }
+
+    public void testMacImages() throws Exception {
+        HSLFSlideShow hss = new HSLFSlideShow(_slTests.openResourceAsStream("53446.ppt"));
+
+        PictureData[] pictures = hss.getPictures();
+        assertEquals(15, pictures.length);
+
+        int[][] expectedSizes = {
+                null,           // WMF
+                { 427, 428 },   // PNG
+                { 371, 370 },   // PNG
+                { 288, 183 },   // PNG
+                { 285, 97 },    // PNG
+                { 288, 168 },   // PNG
+                null,           // WMF
+                null,           // WMF
+                { 199, 259 },   // PNG
+                { 432, 244 },   // PNG
+                { 261, 258 },   // PNG
+                null,           // WMF
+                null,           // WMF
+                null,           // WMF
+                null            // EMF
+        };
+
+        for (int i = 0; i < pictures.length; i++) {
+            BufferedImage image = ImageIO.read(new ByteArrayInputStream(pictures[i].getData()));
+
+            if (pictures[i].getType() != Picture.WMF && pictures[i].getType() != Picture.EMF) {
+                assertNotNull(image);
+
+                int[] dimensions = expectedSizes[i];
+                assertEquals(dimensions[0], image.getWidth());
+                assertEquals(dimensions[1], image.getHeight());
+            }
+        }
+    }
 }
diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/TestHWPFPictures.java b/src/scratchpad/testcases/org/apache/poi/hwpf/TestHWPFPictures.java
index f650f0d890..ae7cc3df24 100644
--- a/src/scratchpad/testcases/org/apache/poi/hwpf/TestHWPFPictures.java
+++ b/src/scratchpad/testcases/org/apache/poi/hwpf/TestHWPFPictures.java
@@ -17,10 +17,14 @@
 
 package org.apache.poi.hwpf;
 
+import javax.imageio.ImageIO;
+import java.awt.image.BufferedImage;
+import java.io.ByteArrayInputStream;
 import java.util.List;
 
 import junit.framework.TestCase;
 
+import org.apache.poi.POIDataSamples;
 import org.apache.poi.hwpf.model.PicturesTable;
 import org.apache.poi.hwpf.usermodel.Picture;
 import org.apache.poi.POIDataSamples;
@@ -128,6 +132,30 @@ public final class TestHWPFPictures extends TestCase {
 		assertBytesSame(picBytes, pic.getContent());
 	}
 
+   	public void testMacImages() throws Exception {
+        HWPFDocument docC = HWPFTestDataSamples.openSampleFile("53446.doc");
+   		PicturesTable picturesTable = docC.getPicturesTable();
+   		List<Picture> pictures = picturesTable.getAllPictures();
+
+   		assertEquals(4, pictures.size());
+
+        int[][] expectedSizes = {
+            { 185, 42 },  // PNG
+            { 260, 114 }, // PNG
+            { 185, 42 },  // PNG
+            { 260, 114 }, // PNG
+       };
+
+       for (int i = 0; i < pictures.size(); i++) {
+           BufferedImage image = ImageIO.read(new ByteArrayInputStream(pictures.get(i).getContent()));
+           assertNotNull(image);
+
+           int[] dimensions = expectedSizes[i];
+           assertEquals(dimensions[0], image.getWidth());
+           assertEquals(dimensions[1], image.getHeight());
+       }
+   	}
+
 	/**
 	 * Pending the missing files being uploaded to
 	 *  bug #44937
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFPictureData.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFPictureData.java
index 99f4ad8068..b9ff69543a 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFPictureData.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFPictureData.java
@@ -71,6 +71,29 @@ public final class TestHSSFPictureData extends TestCase{
             }
         }
     }
+	
+	public void testMacPicture() throws IOException {
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("53446.xls");
+
+        @SuppressWarnings("unchecked")
+        List<HSSFPictureData> lst = (List<HSSFPictureData>)(List<?>)wb.getAllPictures();
+        assertEquals(1, lst.size());
+
+        HSSFPictureData pict = lst.get(0);
+        String ext = pict.suggestFileExtension();
+        if (!ext.equals("png")) {
+            fail("Expected a PNG.");
+        }
+
+        //try to read image data using javax.imageio.* (JDK 1.4+)
+        byte[] data = pict.getData();
+        BufferedImage png = ImageIO.read(new ByteArrayInputStream(data));
+        assertNotNull(png);
+        assertEquals(78, png.getWidth());
+        assertEquals(76, png.getHeight());
+        assertEquals(HSSFWorkbook.PICTURE_TYPE_PNG, pict.getFormat());
+        assertEquals("image/png", pict.getMimeType());
+    }
 
     public void testNotNullPictures() throws IOException {
 
diff --git a/test-data/document/53446.doc b/test-data/document/53446.doc
new file mode 100644
index 0000000000..4844cc937a
--- /dev/null
+++ b/test-data/document/53446.doc
diff --git a/test-data/slideshow/53446.ppt b/test-data/slideshow/53446.ppt
new file mode 100644
index 0000000000..55333fc359
--- /dev/null
+++ b/test-data/slideshow/53446.ppt
diff --git a/test-data/spreadsheet/53446.xls b/test-data/spreadsheet/53446.xls
new file mode 100644
index 0000000000..b33bd7aeca
--- /dev/null
+++ b/test-data/spreadsheet/53446.xls