correctly append picture data if Pictures stream contains entries with zero length (see bugzilla 46627)

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

src/documentation/content/xdocs/changes.xml

@@ -37,7 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta6" date="2008-??-??">
-           <action dev="POI-DEVELOPERS" type="fix">remove me</action>
+           <action dev="POI-DEVELOPERS" type="fix">46627 - Fixed offset of added images if Pictures stream contains pictures with zero length</action>
         </release>
         <release version="3.5-beta5" date="2008-02-19">
            <action dev="POI-DEVELOPERS" type="fix">46536 - When shifting rows, update formulas on that sheet to point to the new location of those rows</action>

src/documentation/content/xdocs/status.xml

@@ -34,7 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta6" date="2008-??-??">
-           <action dev="POI-DEVELOPERS" type="fix">remove me</action>
+           <action dev="POI-DEVELOPERS" type="fix">46627 - Fixed offset of added images if Pictures stream contains pictures with zero length</action>
         </release>
         <release version="3.5-beta5" date="2008-02-19">
            <action dev="POI-DEVELOPERS" type="fix">46536 - When shifting rows, update formulas on that sheet to point to the new location of those rows</action>

src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java

@@ -66,7 +66,7 @@ public final class HSLFSlideShow extends POIDocument {
 	private Record[] _records;
 
 	// Raw Pictures contained in the pictures stream
-	private PictureData[] _pictures;
+	private List<PictureData> _pictures;
 
     // Embedded objects stored in storage records in the document stream, lazily populated.
     private ObjectData[] _objects;
@@ -291,6 +291,8 @@ public final class HSLFSlideShow extends POIDocument {
 	 * Find and read in pictures contained in this presentation
 	 */
 	private void readPictures() throws IOException {
+        _pictures = new ArrayList<PictureData>();
+
 		byte[] pictstream;
 
 		try {
@@ -304,9 +306,7 @@ public final class HSLFSlideShow extends POIDocument {
 			return;
 		}
 
-        List p = new ArrayList();
         int pos = 0;
-
 		// An empty picture record (length 0) will take up 8 bytes
         while (pos <= (pictstream.length-8)) {
             int offset = pos;
@@ -325,7 +325,7 @@ public final class HSLFSlideShow extends POIDocument {
 			// (0 is allowed, but odd, since we do wind on by the header each
 			//  time, so we won't get stuck)
 			if(imgsize < 0) {
-				throw new CorruptPowerPointFileException("The file contains a picture, at position " + p.size() + ", which has a negatively sized data length, so we can't trust any of the picture data");
+				throw new CorruptPowerPointFileException("The file contains a picture, at position " + _pictures.size() + ", which has a negatively sized data length, so we can't trust any of the picture data");
 			}
 
 			// If they type (including the bonus 0xF018) is 0, skip it
@@ -343,7 +343,7 @@ public final class HSLFSlideShow extends POIDocument {
                     pict.setRawData(imgdata);
 
                     pict.setOffset(offset);
-					p.add(pict);
+					_pictures.add(pict);
 				} catch(IllegalArgumentException e) {
 					logger.log(POILogger.ERROR, "Problem reading picture: " + e + "\nYou document will probably become corrupted if you save it!");
 				}
@@ -351,8 +351,6 @@ public final class HSLFSlideShow extends POIDocument {
             
             pos += imgsize;
         }
-
-		_pictures = (PictureData[])p.toArray(new PictureData[p.size()]);
 	}
 
 
@@ -453,10 +451,10 @@ public final class HSLFSlideShow extends POIDocument {
 
 	
         // Write any pictures, into another stream
-        if (_pictures != null) {
+        if (_pictures.size() > 0) {
             ByteArrayOutputStream pict = new ByteArrayOutputStream();
-            for (int i = 0; i < _pictures.length; i++ ) {
+            for (PictureData p : _pictures) {
-                _pictures[i].write(pict);
+                p.write(pict);
             }
             outFS.createDocument(
                 new ByteArrayInputStream(pict.toByteArray()), "Pictures"
@@ -502,21 +500,21 @@ public final class HSLFSlideShow extends POIDocument {
 	}
 	
 	/**
-	 *  Add a new picture to this presentation.
+	 * Add a new picture to this presentation.
+     *
+     * @return offset of this picture in the Pictures stream
 	 */
-	public void addPicture(PictureData img) {
+	public int addPicture(PictureData img) {
-		// Copy over the existing pictures, into an array one bigger
+		int offset = 0;
-		PictureData[] lst;
+
-		if(_pictures == null) {
+        if(_pictures.size() > 0){
-			lst = new PictureData[1];
+            PictureData prev = _pictures.get(_pictures.size() - 1);
-		} else {
+            offset = prev.getOffset() + prev.getRawData().length + 8;
-			lst = new PictureData[(_pictures.length+1)];
+        }
-			System.arraycopy(_pictures,0,lst,0,_pictures.length);
+        img.setOffset(offset);
-		}
+        _pictures.add(img);
-		// Add in the new image
+        return offset;
-		lst[lst.length - 1] = img;
+   }
-		_pictures = lst;
-	}
 
 	/* ******************* fetching methods follow ********************* */
 
@@ -544,7 +542,7 @@ public final class HSLFSlideShow extends POIDocument {
 	 *  presentation doesn't contain pictures.
 	 */
 	public PictureData[] getPictures() {
-		return _pictures;
+		return _pictures.toArray(new PictureData[_pictures.size()]);
 	}
 
     /**

src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java

@@ -739,7 +739,6 @@ public final class SlideShow {
         byte[] uid = PictureData.getChecksum(data);
 
         EscherContainerRecord bstore;
-        int offset = 0;
 
         EscherContainerRecord dggContainer = _documentRecord.getPPDrawingGroup().getDggContainer();
         bstore = (EscherContainerRecord)Shape.getEscherChild(dggContainer, EscherContainerRecord.BSTORE_CONTAINER);
@@ -763,13 +762,13 @@ public final class SlideShow {
                 if (Arrays.equals(bse.getUid(), uid)){
                     return i + 1;
                 }
-                offset += bse.getSize();
              }
         }
 
         PictureData pict = PictureData.create(format);
         pict.setData(data);
-        pict.setOffset(offset);
+
+        int offset = _hslfSlideShow.addPicture(pict);
 
         EscherBSERecord bse = new EscherBSERecord();
         bse.setRecordId(EscherBSERecord.RECORD_ID);
@@ -786,13 +785,12 @@ public final class SlideShow {
 
         bse.setRef(0);
         bse.setOffset(offset);
+        bse.setRemainingData( new byte[0] );
 
         bstore.addChildRecord(bse);
         int count = bstore.getChildRecords().size();
         bstore.setOptions((short)( (count << 4) | 0xF ));
 
-        _hslfSlideShow.addPicture(pict);
-
         return count;
     }
 

src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestPictures.java

@@ -436,7 +436,21 @@ public class TestPictures extends TestCase{
         pdata = pict.getPictureData();
         assertTrue(pdata instanceof WMF);
         assertEquals(Picture.WMF, pdata.getType());
-	}
+
+        //add a new picture, it should be correctly appended to the Pictures stream
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        for(PictureData p : pictures) p.write(out);
+        out.close();
+
+        int streamSize = out.size();
+
+        PictureData data = PictureData.create(Picture.JPEG);
+        data.setData(new byte[100]);
+        int offset = hslf.addPicture(data);
+        assertEquals(streamSize, offset);
+        assertEquals(3, ppt.getPictureData().length);
+
+    }
 
     public void testGetPictureName() throws Exception {
         SlideShow ppt = new SlideShow(new HSLFSlideShow(new File(cwd, "ppt_with_png.ppt").getPath()));