Bug 48593 - [PATCH] Multiple Saves Causes Slide Corruption

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

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

@@ -29,6 +29,7 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.poi.POIDocument;
 import org.apache.poi.hslf.exceptions.CorruptPowerPointFileException;
@@ -40,12 +41,14 @@ import org.apache.poi.hslf.record.PersistPtrHolder;
 import org.apache.poi.hslf.record.PersistRecord;
 import org.apache.poi.hslf.record.PositionDependentRecord;
 import org.apache.poi.hslf.record.Record;
+import org.apache.poi.hslf.record.RecordTypes;
 import org.apache.poi.hslf.record.UserEditAtom;
 import org.apache.poi.hslf.usermodel.ObjectData;
 import org.apache.poi.hslf.usermodel.PictureData;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.DocumentEntry;
 import org.apache.poi.poifs.filesystem.DocumentInputStream;
+import org.apache.poi.poifs.filesystem.EntryUtils;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.LittleEndian;
@@ -59,6 +62,8 @@ import org.apache.poi.util.POILogger;
  * @author Nick Burch
  */
 public final class HSLFSlideShow extends POIDocument {
+    public static final int UNSET_OFFSET = -1;
+    
     // For logging
     private POILogger logger = POILogFactory.getLogger(this.getClass());
 
@@ -346,6 +351,7 @@ public final class HSLFSlideShow extends POIDocument {
             int offset = pos;
 
             // Image signature
+            @SuppressWarnings("unused")
             int signature = LittleEndian.getUShort(pictstream, pos);
             pos += LittleEndian.SHORT_SIZE;
             // Image type + 0xF018
@@ -392,7 +398,88 @@ public final class HSLFSlideShow extends POIDocument {
         }
 	}
 
+	/**
+     * This is a helper functions, which is needed for adding new position dependent records
+     * or finally write the slideshow to a file.
+	 *
+	 * @param os the stream to write to, if null only the references are updated
+	 * @param interestingRecords a map of interesting records (PersistPtrHolder and UserEditAtom)
+	 *        referenced by their RecordType. Only the very last of each type will be saved to the map.
+	 *        May be null, if not needed. 
+	 * @throws IOException
+	 */
+	public void updateAndWriteDependantRecords(OutputStream os, Map<RecordTypes.Type,PositionDependentRecord> interestingRecords)
+	throws IOException {
+        // For position dependent records, hold where they were and now are
+        // As we go along, update, and hand over, to any Position Dependent
+        //  records we happen across
+        Hashtable<Integer,Integer> oldToNewPositions = new Hashtable<Integer,Integer>();
 
+        // First pass - figure out where all the position dependent
+        //   records are going to end up, in the new scheme
+        // (Annoyingly, some powerpoint files have PersistPtrHolders
+        //  that reference slides after the PersistPtrHolder)
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        for (Record record : _records) {
+            if(record instanceof PositionDependentRecord) {
+                PositionDependentRecord pdr = (PositionDependentRecord)record;
+                int oldPos = pdr.getLastOnDiskOffset();
+                int newPos = baos.size();
+                pdr.setLastOnDiskOffset(newPos);
+                if (oldPos != UNSET_OFFSET) {
+                    // new records don't need a mapping, as they aren't in a relation yet
+                    oldToNewPositions.put(Integer.valueOf(oldPos),Integer.valueOf(newPos));
+                }
+            }
+            
+            // Dummy write out, so the position winds on properly
+            record.writeOut(baos);
+        }
+        baos = null;
+        
+        // For now, we're only handling PositionDependentRecord's that
+        // happen at the top level.
+        // In future, we'll need the handle them everywhere, but that's
+        // a bit trickier
+	    UserEditAtom usr = null;
+        for (Record record : _records) {
+            if (record instanceof PositionDependentRecord) {
+                // We've already figured out their new location, and
+                // told them that
+                // Tell them of the positions of the other records though
+                PositionDependentRecord pdr = (PositionDependentRecord)record;
+                pdr.updateOtherRecordReferences(oldToNewPositions);
+    
+                // Grab interesting records as they come past
+                // this will only save the very last record of each type
+                RecordTypes.Type saveme = null;
+                int recordType = (int)record.getRecordType();
+                if (recordType == RecordTypes.PersistPtrIncrementalBlock.typeID) {
+                    saveme = RecordTypes.PersistPtrIncrementalBlock;
+                } else if (recordType == RecordTypes.UserEditAtom.typeID) {
+                    saveme = RecordTypes.UserEditAtom;
+                    usr = (UserEditAtom)pdr;
+                }
+                if (interestingRecords != null && saveme != null) {
+                    interestingRecords.put(saveme,pdr);
+                }
+            }
+            
+            // Whatever happens, write out that record tree
+            if (os != null) {
+                record.writeOut(os);
+            }
+        }
+
+        // Update and write out the Current User atom
+        int oldLastUserEditAtomPos = (int)currentUser.getCurrentEditOffset();
+        Integer newLastUserEditAtomPos = oldToNewPositions.get(oldLastUserEditAtomPos);
+        if(usr == null || newLastUserEditAtomPos == null || usr.getLastOnDiskOffset() != newLastUserEditAtomPos) {
+            throw new HSLFException("Couldn't find the new location of the last UserEditAtom that used to be at " + oldLastUserEditAtomPos);
+        }
+        currentUser.setCurrentEditOffset(usr.getLastOnDiskOffset());
+	}
+	
     /**
      * Writes out the slideshow file the is represented by an instance
      *  of this class.
@@ -426,49 +513,13 @@ public final class HSLFSlideShow extends POIDocument {
         // Write out the Property Streams
         writeProperties(outFS, writtenEntries);
 
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
 
         // For position dependent records, hold where they were and now are
         // As we go along, update, and hand over, to any Position Dependent
-        //  records we happen across
+        // records we happen across
-        Hashtable<Integer,Integer> oldToNewPositions = new Hashtable<Integer,Integer>();
+        updateAndWriteDependantRecords(baos, null);
 
-        // First pass - figure out where all the position dependent
-        //   records are going to end up, in the new scheme
-        // (Annoyingly, some powerpoing files have PersistPtrHolders
-        //  that reference slides after the PersistPtrHolder)
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        for(int i=0; i<_records.length; i++) {
-            if(_records[i] instanceof PositionDependentRecord) {
-                PositionDependentRecord pdr = (PositionDependentRecord)_records[i];
-                int oldPos = pdr.getLastOnDiskOffset();
-                int newPos = baos.size();
-                pdr.setLastOnDiskOffset(newPos);
-                oldToNewPositions.put(Integer.valueOf(oldPos),Integer.valueOf(newPos));
-                //System.out.println(oldPos + " -> " + newPos);
-            }
-
-            // Dummy write out, so the position winds on properly
-            _records[i].writeOut(baos);
-        }
-
-        // No go back through, actually writing ourselves out
-        baos.reset();
-        for(int i=0; i<_records.length; i++) {
-            // For now, we're only handling PositionDependentRecord's that
-            //  happen at the top level.
-            // In future, we'll need the handle them everywhere, but that's
-            //  a bit trickier
-            if(_records[i] instanceof PositionDependentRecord) {
-                // We've already figured out their new location, and
-                //  told them that
-                // Tell them of the positions of the other records though
-                PositionDependentRecord pdr = (PositionDependentRecord)_records[i];
-                pdr.updateOtherRecordReferences(oldToNewPositions);
-            }
-
-            // Whatever happens, write out that record tree
-            _records[i].writeOut(baos);
-        }
         // Update our cached copy of the bytes that make up the PPT stream
         _docstream = baos.toByteArray();
 
@@ -476,15 +527,6 @@ public final class HSLFSlideShow extends POIDocument {
         ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
         outFS.createDocument(bais,"PowerPoint Document");
         writtenEntries.add("PowerPoint Document");
-
-
-        // Update and write out the Current User atom
-        int oldLastUserEditAtomPos = (int)currentUser.getCurrentEditOffset();
-        Integer newLastUserEditAtomPos = (Integer)oldToNewPositions.get(Integer.valueOf(oldLastUserEditAtomPos));
-        if(newLastUserEditAtomPos == null) {
-            throw new HSLFException("Couldn't find the new location of the UserEditAtom that used to be at " + oldLastUserEditAtomPos);
-        }
-        currentUser.setCurrentEditOffset(newLastUserEditAtomPos.intValue());
         currentUser.writeToFS(outFS);
         writtenEntries.add("Current User");
 
@@ -506,7 +548,7 @@ public final class HSLFSlideShow extends POIDocument {
 
         // If requested, write out any other streams we spot
         if(preserveNodes) {
-            copyNodes(directory.getFileSystem(), outFS, writtenEntries);
+            EntryUtils.copyNodes(directory.getFileSystem(), outFS, writtenEntries);
         }
 
         // Send the POIFSFileSystem object out to the underlying stream
@@ -612,9 +654,9 @@ public final class HSLFSlideShow extends POIDocument {
     public ObjectData[] getEmbeddedObjects() {
         if (_objects == null) {
             List<ObjectData> objects = new ArrayList<ObjectData>();
-            for (int i = 0; i < _records.length; i++) {
+            for (Record r : _records) {
-                if (_records[i] instanceof ExOleObjStg) {
+                if (r instanceof ExOleObjStg) {
-                    objects.add(new ObjectData((ExOleObjStg) _records[i]));
+                    objects.add(new ObjectData((ExOleObjStg)r));
                 }
             }
             _objects = objects.toArray(new ObjectData[objects.size()]);

src/scratchpad/src/org/apache/poi/hslf/record/ExOleObjStg.java

@@ -17,10 +17,14 @@
 
 package org.apache.poi.hslf.record;
 
-import java.io.*;
+import java.io.ByteArrayInputStream;
-import java.util.zip.InflaterInputStream;
+import java.io.ByteArrayOutputStream;
-import java.util.zip.DeflaterOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.util.Hashtable;
+import java.util.zip.DeflaterOutputStream;
+import java.util.zip.InflaterInputStream;
 
 import org.apache.poi.util.BoundedInputStream;
 import org.apache.poi.util.LittleEndian;
@@ -120,7 +124,7 @@ public class ExOleObjStg extends RecordAtom implements PositionDependentRecord,
         ByteArrayOutputStream out = new ByteArrayOutputStream();
         //first four bytes is the length of the raw data
         byte[] b = new byte[4];
-        LittleEndian.putInt(b, data.length);
+        LittleEndian.putInt(b, 0, data.length);
         out.write(b);
 
         DeflaterOutputStream def = new DeflaterOutputStream(out);

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

@@ -25,7 +25,12 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
 
 import org.apache.poi.ddf.EscherBSERecord;
 import org.apache.poi.ddf.EscherContainerRecord;
@@ -35,11 +40,43 @@ import org.apache.poi.hpsf.ClassID;
 import org.apache.poi.hslf.HSLFSlideShow;
 import org.apache.poi.hslf.exceptions.CorruptPowerPointFileException;
 import org.apache.poi.hslf.exceptions.HSLFException;
-import org.apache.poi.hslf.model.*;
+import org.apache.poi.hslf.model.HeadersFooters;
+import org.apache.poi.hslf.model.Hyperlink;
+import org.apache.poi.hslf.model.MovieShape;
 import org.apache.poi.hslf.model.Notes;
+import org.apache.poi.hslf.model.PPFont;
+import org.apache.poi.hslf.model.Picture;
+import org.apache.poi.hslf.model.Shape;
 import org.apache.poi.hslf.model.Slide;
-import org.apache.poi.hslf.record.*;
+import org.apache.poi.hslf.model.SlideMaster;
+import org.apache.poi.hslf.model.TitleMaster;
+import org.apache.poi.hslf.record.Document;
+import org.apache.poi.hslf.record.DocumentAtom;
+import org.apache.poi.hslf.record.ExAviMovie;
+import org.apache.poi.hslf.record.ExControl;
+import org.apache.poi.hslf.record.ExEmbed;
+import org.apache.poi.hslf.record.ExEmbedAtom;
+import org.apache.poi.hslf.record.ExHyperlink;
+import org.apache.poi.hslf.record.ExHyperlinkAtom;
+import org.apache.poi.hslf.record.ExMCIMovie;
+import org.apache.poi.hslf.record.ExObjList;
+import org.apache.poi.hslf.record.ExObjListAtom;
+import org.apache.poi.hslf.record.ExOleObjAtom;
+import org.apache.poi.hslf.record.ExOleObjStg;
+import org.apache.poi.hslf.record.ExVideoContainer;
+import org.apache.poi.hslf.record.FontCollection;
+import org.apache.poi.hslf.record.FontEntityAtom;
+import org.apache.poi.hslf.record.HeadersFootersContainer;
+import org.apache.poi.hslf.record.PersistPtrHolder;
+import org.apache.poi.hslf.record.PositionDependentRecord;
+import org.apache.poi.hslf.record.PositionDependentRecordContainer;
+import org.apache.poi.hslf.record.Record;
+import org.apache.poi.hslf.record.RecordContainer;
+import org.apache.poi.hslf.record.RecordTypes;
+import org.apache.poi.hslf.record.SlideListWithText;
 import org.apache.poi.hslf.record.SlideListWithText.SlideAtomsSet;
+import org.apache.poi.hslf.record.SlidePersistAtom;
+import org.apache.poi.hslf.record.UserEditAtom;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.POILogFactory;
@@ -58,15 +95,12 @@ public final class SlideShow {
 	// What we're based on
 	private HSLFSlideShow _hslfSlideShow;
 
-	// Low level contents, as taken from HSLFSlideShow
-	private Record[] _records;
-
 	// Pointers to the most recent versions of the core records
 	// (Document, Notes, Slide etc)
 	private Record[] _mostRecentCoreRecords;
 	// Lookup between the PersitPtr "sheet" IDs, and the position
 	// in the mostRecentCoreRecords array
-	private Hashtable<Integer,Integer> _sheetIdToCoreRecordsLookup;
+	private Map<Integer,Integer> _sheetIdToCoreRecordsLookup;
 
 	// Records that are interesting
 	private Document _documentRecord;
@@ -97,10 +131,9 @@ public final class SlideShow {
 	public SlideShow(HSLFSlideShow hslfSlideShow) {
 	    // Get useful things from our base slideshow
 	    _hslfSlideShow = hslfSlideShow;
-		_records = _hslfSlideShow.getRecords();
 
 		// Handle Parent-aware Records
-		for (Record record : _records) {
+		for (Record record : _hslfSlideShow.getRecords()) {
 			if(record instanceof RecordContainer){
                 RecordContainer.handleParentAwareRecords((RecordContainer)record);
             }
@@ -135,25 +168,23 @@ public final class SlideShow {
 	 */
 	private void findMostRecentCoreRecords() {
 		// To start with, find the most recent in the byte offset domain
-		Hashtable<Integer,Integer> mostRecentByBytes = new Hashtable<Integer,Integer>();
+		Map<Integer,Integer> mostRecentByBytes = new HashMap<Integer,Integer>();
-		for (int i = 0; i < _records.length; i++) {
+		for (Record record : _hslfSlideShow.getRecords()) {
-			if (_records[i] instanceof PersistPtrHolder) {
+			if (record instanceof PersistPtrHolder) {
-				PersistPtrHolder pph = (PersistPtrHolder) _records[i];
+				PersistPtrHolder pph = (PersistPtrHolder) record;
 
 				// If we've already seen any of the "slide" IDs for this
 				// PersistPtr, remove their old positions
 				int[] ids = pph.getKnownSlideIDs();
-				for (int j = 0; j < ids.length; j++) {
+				for (int id : ids) {
-					Integer id = Integer.valueOf(ids[j]);
 					if (mostRecentByBytes.containsKey(id)) {
 						mostRecentByBytes.remove(id);
 					}
 				}
 
 				// Now, update the byte level locations with their latest values
-				Hashtable<Integer,Integer> thisSetOfLocations = pph.getSlideLocationsLookup();
+				Map<Integer,Integer> thisSetOfLocations = pph.getSlideLocationsLookup();
-				for (int j = 0; j < ids.length; j++) {
+				for (int id : ids) {
-					Integer id = Integer.valueOf(ids[j]);
 					mostRecentByBytes.put(id, thisSetOfLocations.get(id));
 				}
 			}
@@ -165,54 +196,48 @@ public final class SlideShow {
 
 		// We'll also want to be able to turn the slide IDs into a position
 		// in this array
-		_sheetIdToCoreRecordsLookup = new Hashtable<Integer,Integer>();
+		_sheetIdToCoreRecordsLookup = new HashMap<Integer,Integer>();
-		int[] allIDs = new int[_mostRecentCoreRecords.length];
+		Integer[] allIDs = mostRecentByBytes.keySet().toArray(new Integer[mostRecentByBytes.size()]); 
-		Enumeration<Integer> ids = mostRecentByBytes.keys();
-		for (int i = 0; i < allIDs.length; i++) {
-			Integer id = ids.nextElement();
-			allIDs[i] = id.intValue();
-		}
 		Arrays.sort(allIDs);
 		for (int i = 0; i < allIDs.length; i++) {
-			_sheetIdToCoreRecordsLookup.put(Integer.valueOf(allIDs[i]), Integer.valueOf(i));
+			_sheetIdToCoreRecordsLookup.put(allIDs[i], i);
 		}
 
 		// Now convert the byte offsets back into record offsets
-		for (int i = 0; i < _records.length; i++) {
+		for (Record record : _hslfSlideShow.getRecords()) {
-			if (_records[i] instanceof PositionDependentRecord) {
+			if (record instanceof PositionDependentRecord) {
-				PositionDependentRecord pdr = (PositionDependentRecord) _records[i];
+				PositionDependentRecord pdr = (PositionDependentRecord) record;
-				Integer recordAt = Integer.valueOf(pdr.getLastOnDiskOffset());
+				int recordAt = pdr.getLastOnDiskOffset();
 
 				// Is it one we care about?
-				for (int j = 0; j < allIDs.length; j++) {
+				for (Integer thisID : allIDs) {
-					Integer thisID = Integer.valueOf(allIDs[j]);
+					int thatRecordAt = mostRecentByBytes.get(thisID);
-					Integer thatRecordAt = mostRecentByBytes.get(thisID);
 
-					if (thatRecordAt.equals(recordAt)) {
+					if (thatRecordAt == recordAt) {
 						// Bingo. Now, where do we store it?
 						Integer storeAtI = _sheetIdToCoreRecordsLookup.get(thisID);
 						int storeAt = storeAtI.intValue();
 
 						// Tell it its Sheet ID, if it cares
 						if (pdr instanceof PositionDependentRecordContainer) {
-							PositionDependentRecordContainer pdrc = (PositionDependentRecordContainer) _records[i];
+							PositionDependentRecordContainer pdrc = (PositionDependentRecordContainer) record;
-							pdrc.setSheetId(thisID.intValue());
+							pdrc.setSheetId(thisID);
 						}
 
 						// Finally, save the record
-						_mostRecentCoreRecords[storeAt] = _records[i];
+						_mostRecentCoreRecords[storeAt] = record;
 					}
 				}
 			}
 		}
 
 		// Now look for the interesting records in there
-		for (int i = 0; i < _mostRecentCoreRecords.length; i++) {
+		for (Record record : _mostRecentCoreRecords) {
 			// Check there really is a record at this number
-			if (_mostRecentCoreRecords[i] != null) {
+			if (record != null) {
 				// Find the Document, and interesting things in it
-				if (_mostRecentCoreRecords[i].getRecordType() == RecordTypes.Document.typeID) {
+				if (record.getRecordType() == RecordTypes.Document.typeID) {
-					_documentRecord = (Document) _mostRecentCoreRecords[i];
+					_documentRecord = (Document) record;
 					_fonts = _documentRecord.getEnvironment().getFontCollection();
 				}
 			} else {
@@ -296,9 +321,8 @@ public final class SlideShow {
 			ArrayList<SlideMaster> mmr = new ArrayList<SlideMaster>();
 			ArrayList<TitleMaster> tmr = new ArrayList<TitleMaster>();
 
-			for (int i = 0; i < masterSets.length; i++) {
+			for (SlideAtomsSet sas : masterSets) {
-				Record r = getCoreRecordForSAS(masterSets[i]);
+				Record r = getCoreRecordForSAS(sas);
-				SlideAtomsSet sas = masterSets[i];
 				int sheetNo = sas.getSlidePersistAtom().getSlideIdentifier();
 				if (r instanceof org.apache.poi.hslf.record.Slide) {
 					TitleMaster master = new TitleMaster((org.apache.poi.hslf.record.Slide) r,
@@ -313,11 +337,8 @@ public final class SlideShow {
 				}
 			}
 
-			_masters = new SlideMaster[mmr.size()];
+			_masters = mmr.toArray(new SlideMaster[mmr.size()]);
-			mmr.toArray(_masters);
+			_titleMasters = tmr.toArray(new TitleMaster[tmr.size()]);
-
-			_titleMasters = new TitleMaster[tmr.size()];
-			tmr.toArray(_titleMasters);
 		}
 
 		// Having sorted out the masters, that leaves the notes and slides
@@ -326,14 +347,14 @@ public final class SlideShow {
 		// notesSLWT
 		org.apache.poi.hslf.record.Notes[] notesRecords;
 		SlideAtomsSet[] notesSets = new SlideAtomsSet[0];
-		Hashtable<Integer,Integer> slideIdToNotes = new Hashtable<Integer,Integer>();
+		Map<Integer,Integer> slideIdToNotes = new HashMap<Integer,Integer>();
 		if (notesSLWT == null) {
 			// None
 			notesRecords = new org.apache.poi.hslf.record.Notes[0];
 		} else {
 			// Match up the records and the SlideAtomSets
 			notesSets = notesSLWT.getSlideAtomsSets();
-			ArrayList<org.apache.poi.hslf.record.Notes> notesRecordsL = 
+			List<org.apache.poi.hslf.record.Notes> notesRecordsL = 
 			   new ArrayList<org.apache.poi.hslf.record.Notes>();
 			for (int i = 0; i < notesSets.length; i++) {
 				// Get the right core record
@@ -346,8 +367,8 @@ public final class SlideShow {
 
 					// Record the match between slide id and these notes
 					SlidePersistAtom spa = notesSets[i].getSlidePersistAtom();
-					Integer slideId = Integer.valueOf(spa.getSlideIdentifier());
+					int slideId = spa.getSlideIdentifier();
-					slideIdToNotes.put(slideId, Integer.valueOf(i));
+					slideIdToNotes.put(slideId, i);
 				} else {
 					logger.log(POILogger.ERROR, "A Notes SlideAtomSet at " + i
 							+ " said its record was at refID "
@@ -686,9 +707,8 @@ public final class SlideShow {
 		// (Will stay as null if no SlidePersistAtom exists yet in
 		// the slide, or only master slide's ones do)
 		SlidePersistAtom prev = null;
-		SlideAtomsSet[] sas = slist.getSlideAtomsSets();
+		for (SlideAtomsSet sas : slist.getSlideAtomsSets()) {
-		for (int j = 0; j < sas.length; j++) {
+			SlidePersistAtom spa = sas.getSlidePersistAtom();
-			SlidePersistAtom spa = sas[j].getSlidePersistAtom();
 			if (spa.getSlideIdentifier() < 0) {
 				// This is for a master slide
 				// Odd, since we only deal with the Slide SLWT
@@ -850,19 +870,16 @@ public final class SlideShow {
 	 *         found
 	 */
 	public PPFont getFont(int idx) {
-		PPFont font = null;
 		FontCollection fonts = getDocumentRecord().getEnvironment().getFontCollection();
-		Record[] ch = fonts.getChildRecords();
+		for (Record ch : fonts.getChildRecords()) {
-		for (int i = 0; i < ch.length; i++) {
+			if (ch instanceof FontEntityAtom) {
-			if (ch[i] instanceof FontEntityAtom) {
+				FontEntityAtom atom = (FontEntityAtom) ch;
-				FontEntityAtom atom = (FontEntityAtom) ch[i];
 				if (atom.getFontIndex() == idx) {
-					font = new PPFont(atom);
+					return new PPFont(atom);
-					break;
 				}
 			}
 		}
-		return font;
+		return null;
 	}
 
 	/**
@@ -885,11 +902,10 @@ public final class SlideShow {
 		boolean ppt2007 = "___PPT12".equals(tag);
 
 		HeadersFootersContainer hdd = null;
-		Record[] ch = _documentRecord.getChildRecords();
+		for (Record ch : _documentRecord.getChildRecords()) {
-		for (int i = 0; i < ch.length; i++) {
+			if (ch instanceof HeadersFootersContainer
-			if (ch[i] instanceof HeadersFootersContainer
+				&& ((HeadersFootersContainer) ch).getOptions() == HeadersFootersContainer.SlideHeadersFootersContainer) {
-					&& ((HeadersFootersContainer) ch[i]).getOptions() == HeadersFootersContainer.SlideHeadersFootersContainer) {
+				hdd = (HeadersFootersContainer) ch;
-				hdd = (HeadersFootersContainer) ch[i];
 				break;
 			}
 		}
@@ -912,11 +928,10 @@ public final class SlideShow {
 		boolean ppt2007 = "___PPT12".equals(tag);
 
 		HeadersFootersContainer hdd = null;
-		Record[] ch = _documentRecord.getChildRecords();
+		for (Record ch : _documentRecord.getChildRecords()) {
-		for (int i = 0; i < ch.length; i++) {
+			if (ch instanceof HeadersFootersContainer
-			if (ch[i] instanceof HeadersFootersContainer
+					&& ((HeadersFootersContainer) ch).getOptions() == HeadersFootersContainer.NotesHeadersFootersContainer) {
-					&& ((HeadersFootersContainer) ch[i]).getOptions() == HeadersFootersContainer.NotesHeadersFootersContainer) {
+				hdd = (HeadersFootersContainer) ch;
-				hdd = (HeadersFootersContainer) ch[i];
 				break;
 			}
 		}
@@ -1107,37 +1122,23 @@ public final class SlideShow {
     }
 
     protected int addPersistentObject(PositionDependentRecord slideRecord) {
-		int slideRecordPos = _hslfSlideShow.appendRootLevelRecord((Record)slideRecord);
+    	slideRecord.setLastOnDiskOffset(HSLFSlideShow.UNSET_OFFSET);
-		_records = _hslfSlideShow.getRecords();
+		_hslfSlideShow.appendRootLevelRecord((Record)slideRecord);
 
-		// Add the new Slide into the PersistPtr stuff
+        // For position dependent records, hold where they were and now are
-		int offset = 0;
+        // As we go along, update, and hand over, to any Position Dependent
-		int slideOffset = 0;
+        // records we happen across
-		PersistPtrHolder ptr = null;
+		Map<RecordTypes.Type,PositionDependentRecord> interestingRecords =
-		UserEditAtom usr = null;
+                new HashMap<RecordTypes.Type,PositionDependentRecord>();
-		int i = 0;
-		for (Record record : _records) {
-            // Grab interesting records as they come past
-			int recordType = (int)record.getRecordType();
-            if (recordType == RecordTypes.PersistPtrIncrementalBlock.typeID) {
-                ptr = (PersistPtrHolder)record;
-            }
-            if (recordType == RecordTypes.UserEditAtom.typeID) {
-                usr = (UserEditAtom)record;
-            }
 
-			if (i++ == slideRecordPos) {
+		try {
-				slideOffset = offset;
+            _hslfSlideShow.updateAndWriteDependantRecords(null,interestingRecords);
-			}
+        } catch (IOException e) {
-			
+            throw new HSLFException(e);
-			try {
+        }
-				ByteArrayOutputStream out = new ByteArrayOutputStream();
+		
-				record.writeOut(out);
+		PersistPtrHolder ptr = (PersistPtrHolder)interestingRecords.get(RecordTypes.PersistPtrIncrementalBlock);
-				offset += out.size();
+		UserEditAtom usr = (UserEditAtom)interestingRecords.get(RecordTypes.UserEditAtom);
-			} catch (IOException e) {
-				throw new HSLFException(e);
-			}
-		}
 
 		// persist ID is UserEditAtom.maxPersistWritten + 1
 		int psrId = usr.getMaxPersistWritten() + 1;
@@ -1149,6 +1150,7 @@ public final class SlideShow {
 
 		// Add the new slide into the last PersistPtr
 		// (Also need to tell it where it is)
+		int slideOffset = slideRecord.getLastOnDiskOffset();
 		slideRecord.setLastOnDiskOffset(slideOffset);
 		ptr.addSlideLookup(psrId, slideOffset);
 		logger.log(POILogger.INFO, "New slide/object ended up at " + slideOffset);

src/scratchpad/testcases/org/apache/poi/hslf/HSLFTestDataSamples.java

@@ -0,0 +1,74 @@
+/* ====================================================================
+   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.hslf;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.poi.POIDataSamples;
+import org.apache.poi.hslf.usermodel.SlideShow;
+
+public class HSLFTestDataSamples {
+
+	private static final POIDataSamples _inst = POIDataSamples.getSlideShowInstance();
+
+	public static InputStream openSampleFileStream(String sampleFileName) {
+		return _inst.openResourceAsStream(sampleFileName);
+	}
+	public static File getSampleFile(String sampleFileName) {
+	   return _inst.getFile(sampleFileName);
+	}
+	public static byte[] getTestDataFileContent(String fileName) {
+		return _inst.readFile(fileName);
+	}
+
+	/**
+	 * Writes a slideshow to a <tt>ByteArrayOutputStream</tt> and reads it back
+	 * from a <tt>ByteArrayInputStream</tt>.<p/>
+	 * Useful for verifying that the serialisation round trip
+	 */
+	public static HSLFSlideShow writeOutAndReadBack(HSLFSlideShow original) {
+		try {
+			ByteArrayOutputStream baos = new ByteArrayOutputStream(4096);
+			original.write(baos);
+			ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+			return new HSLFSlideShow(bais);
+		} catch (IOException e) {
+			throw new RuntimeException(e);
+		}
+	}
+
+	/**
+	 * Writes a slideshow to a <tt>ByteArrayOutputStream</tt> and reads it back
+	 * from a <tt>ByteArrayInputStream</tt>.<p/>
+	 * Useful for verifying that the serialisation round trip
+	 */
+	public static SlideShow writeOutAndReadBack(SlideShow original) {
+		try {
+			ByteArrayOutputStream baos = new ByteArrayOutputStream(4096);
+			original.write(baos);
+			ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+			return new SlideShow(bais);
+		} catch (IOException e) {
+			throw new RuntimeException(e);
+		}
+	}
+}

src/scratchpad/testcases/org/apache/poi/hslf/TestReWrite.java

@@ -18,15 +18,16 @@
 package org.apache.poi.hslf;
 
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.FileNotFoundException;
+
 import junit.framework.TestCase;
 
-import org.apache.poi.hslf.usermodel.SlideShow;
-import org.apache.poi.poifs.filesystem.*;
 import org.apache.poi.POIDataSamples;
-
+import org.apache.poi.hslf.usermodel.SlideShow;
-import java.io.ByteArrayOutputStream;
+import org.apache.poi.poifs.filesystem.DocumentEntry;
-import java.io.ByteArrayInputStream;
+import org.apache.poi.poifs.filesystem.POIFSFileSystem;
-import java.io.FileNotFoundException;
 
 /**
  * Tests that HSLFSlideShow writes the powerpoint bit of data back out
@@ -160,4 +161,16 @@ public final class TestReWrite extends TestCase {
 			assertEquals(_oData[i], _nData[i]);
 		}
 	}
+    
+    public void test48593() throws Exception {
+		SlideShow slideShow = new SlideShow();
+		slideShow.createSlide();
+		slideShow = HSLFTestDataSamples.writeOutAndReadBack(slideShow);
+		slideShow.createSlide();
+		slideShow = HSLFTestDataSamples.writeOutAndReadBack(slideShow);
+		slideShow.createSlide();
+		slideShow = HSLFTestDataSamples.writeOutAndReadBack(slideShow);
+		slideShow.createSlide();
+		slideShow = HSLFTestDataSamples.writeOutAndReadBack(slideShow);
+    }
 }