Bug 41246 - AIOOBE with missing notes entries

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

diff --git a/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java b/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java
index e7fb4bbe5818958103c6034410fcf76dff11284f..420bd383f655fe2b6e196210d54a68a593a07586 100644 (file)
--- a/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java
@@ -274,11 +274,11 @@ public final class HSLFSlideShow extends POIDocument {
         HashMap<Integer,Integer> offset2id = new HashMap<Integer,Integer>();
         while (usrOffset != 0){
             UserEditAtom usr = (UserEditAtom) Record.buildRecordAtOffset(docstream, usrOffset);
-            lst.add(Integer.valueOf(usrOffset));
+            lst.add(usrOffset);
             int psrOffset = usr.getPersistPointersOffset();
 
             PersistPtrHolder ptr = (PersistPtrHolder)Record.buildRecordAtOffset(docstream, psrOffset);
-            lst.add(Integer.valueOf(psrOffset));
+            lst.add(psrOffset);
             Hashtable<Integer,Integer> entries = ptr.getSlideLocationsLookup();
             for(Integer id : entries.keySet()) {
                 Integer offset = entries.get(id);

diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java
index 9011583494661a82c64502512e4479f2ae60faf4..22aa0b61d015c44bcb920397ac30f45a0ecb505c 100644 (file)
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java
@@ -203,32 +203,33 @@ public final class SlideShow {
                        _sheetIdToCoreRecordsLookup.put(allIDs[i], i);
                }
 
+               Map<Integer,Integer> mostRecentByBytesRev = new HashMap<Integer,Integer>(mostRecentByBytes.size());
+               for (Map.Entry<Integer,Integer> me : mostRecentByBytes.entrySet()) {
+                   mostRecentByBytesRev.put(me.getValue(), me.getKey());
+               }
+               
                // Now convert the byte offsets back into record offsets
                for (Record record : _hslfSlideShow.getRecords()) {
-                       if (record instanceof PositionDependentRecord) {
-                               PositionDependentRecord pdr = (PositionDependentRecord) record;
-                               int recordAt = pdr.getLastOnDiskOffset();
-
-                               // Is it one we care about?
-                               for (Integer thisID : allIDs) {
-                                       int thatRecordAt = mostRecentByBytes.get(thisID);
-
-                                       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) record;
-                                                       pdrc.setSheetId(thisID);
-                                               }
-
-                                               // Finally, save the record
-                                               _mostRecentCoreRecords[storeAt] = record;
-                                       }
-                               }
+                       if (!(record instanceof PositionDependentRecord)) continue;
+                       
+                       PositionDependentRecord pdr = (PositionDependentRecord) record;
+                       int recordAt = pdr.getLastOnDiskOffset();
+
+                       Integer thisID = mostRecentByBytesRev.get(recordAt);
+                       
+                       if (thisID == null) continue;
+                       
+                       // Bingo. Now, where do we store it?
+                       int storeAt = _sheetIdToCoreRecordsLookup.get(thisID);
+
+                       // Tell it its Sheet ID, if it cares
+                       if (pdr instanceof PositionDependentRecordContainer) {
+                               PositionDependentRecordContainer pdrc = (PositionDependentRecordContainer) record;
+                               pdrc.setSheetId(thisID);
                        }
+
+                       // Finally, save the record
+                       _mostRecentCoreRecords[storeAt] = record;
                }
 
                // Now look for the interesting records in there
@@ -265,9 +266,9 @@ public final class SlideShow {
         *            the refID
         */
        private Record getCoreRecordForRefID(int refID) {
-               Integer coreRecordId = _sheetIdToCoreRecordsLookup.get(Integer.valueOf(refID));
+               Integer coreRecordId = _sheetIdToCoreRecordsLookup.get(refID);
                if (coreRecordId != null) {
-                       Record r = _mostRecentCoreRecords[coreRecordId.intValue()];
+                       Record r = _mostRecentCoreRecords[coreRecordId];
                        return r;
                }
                logger.log(POILogger.ERROR,
@@ -361,7 +362,15 @@ public final class SlideShow {
                                Record r = getCoreRecordForSAS(notesSets[i]);
 
                                // Ensure it really is a notes record
-                               if (r instanceof org.apache.poi.hslf.record.Notes) {
+                               if (r == null || r instanceof org.apache.poi.hslf.record.Notes) {
+                                   if (r == null) {
+                           logger.log(POILogger.WARN, "A Notes SlideAtomSet at " + i
+                                   + " said its record was at refID "
+                                   + notesSets[i].getSlidePersistAtom().getRefID()
+                                   + ", but that record didn't exist - record ignored.");
+                                   }
+                                   // we need to add also null-records, otherwise the index references to other existing
+                                   // don't work anymore
                                        org.apache.poi.hslf.record.Notes notesRecord = (org.apache.poi.hslf.record.Notes) r;
                                        notesRecordsL.add(notesRecord);
 
@@ -410,8 +419,10 @@ public final class SlideShow {
                // Notes first
                _notes = new Notes[notesRecords.length];
                for (int i = 0; i < _notes.length; i++) {
-                       _notes[i] = new Notes(notesRecords[i]);
-                       _notes[i].setSlideShow(this);
+                   if (notesRecords[i] != null) {
+                   _notes[i] = new Notes(notesRecords[i]);
+                       _notes[i].setSlideShow(this);
+                   }
                }
                // Then slides
                _slides = new Slide[slidesRecords.length];
@@ -425,11 +436,12 @@ public final class SlideShow {
                        // 0 if slide has no notes.
                        int noteId = slidesRecords[i].getSlideAtom().getNotesID();
                        if (noteId != 0) {
-                               Integer notesPos = (Integer) slideIdToNotes.get(Integer.valueOf(noteId));
-                               if (notesPos != null)
-                                       notes = _notes[notesPos.intValue()];
-                               else
+                               Integer notesPos = slideIdToNotes.get(noteId);
+                               if (notesPos != null) {
+                                       notes = _notes[notesPos];
+                               } else {
                                        logger.log(POILogger.ERROR, "Notes not found for noteId=" + noteId);
+                               }
                        }
 
                        // Now, build our slide

diff --git a/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java b/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
index 088e0bcaefb1d0b1015e0058296bd540043322b6..f7023887931acbe8902670673a36f384977bd140 100644 (file)
--- a/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
+++ b/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.hslf.usermodel;
 
+import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -27,7 +28,9 @@ import java.util.Set;
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
+import org.apache.poi.POIDataSamples;
 import org.apache.poi.hslf.HSLFSlideShow;
+import org.apache.poi.hslf.HSLFTestDataSamples;
 import org.apache.poi.hslf.exceptions.OldPowerPointFormatException;
 import org.apache.poi.hslf.model.Background;
 import org.apache.poi.hslf.model.Fill;
@@ -42,7 +45,6 @@ import org.apache.poi.hslf.model.TextBox;
 import org.apache.poi.hslf.model.TextRun;
 import org.apache.poi.hslf.model.TextShape;
 import org.apache.poi.hslf.model.TitleMaster;
-import org.apache.poi.POIDataSamples;
 
 /**
  * Testcases for bugs entered in bugzilla
@@ -406,4 +408,31 @@ public final class TestBugs extends TestCase {
           }
        }
     }
+
+    /**
+     * Bug 41246: AIOOB with illegal note references
+     */
+    public void test41246a() throws Exception {
+        InputStream fis = _slTests.openResourceAsStream("41246-1.ppt");
+        HSLFSlideShow hslf = new HSLFSlideShow(fis);
+        fis.close();
+
+        SlideShow ppt = new SlideShow(hslf);
+        assertTrue("No Exceptions while reading file", true);
+
+        ppt = HSLFTestDataSamples.writeOutAndReadBack(ppt);
+        assertTrue("No Exceptions while rewriting file", true);
+    }
+
+    public void test41246b() throws Exception {
+        InputStream fis = _slTests.openResourceAsStream("41246-2.ppt");
+        HSLFSlideShow hslf = new HSLFSlideShow(fis);
+        fis.close();
+
+        SlideShow ppt = new SlideShow(hslf);
+        assertTrue("No Exceptions while reading file", true);
+
+        ppt = HSLFTestDataSamples.writeOutAndReadBack(ppt);
+        assertTrue("No Exceptions while rewriting file", true);
+    }
 }

diff --git a/test-data/slideshow/41246-1.ppt b/test-data/slideshow/41246-1.ppt
new file mode 100644 (file)
index 0000000..b785191
Binary files /dev/null and b/test-data/slideshow/41246-1.ppt differ

diff --git a/test-data/slideshow/41246-2.ppt b/test-data/slideshow/41246-2.ppt
new file mode 100644 (file)
index 0000000..1b1a963
Binary files /dev/null and b/test-data/slideshow/41246-2.ppt differ