diff --git a/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java b/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java index e7fb4bbe5..420bd383f 100644 --- 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 offset2id = new HashMap(); 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 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 901158349..22aa0b61d 100644 --- 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 mostRecentByBytesRev = new HashMap(mostRecentByBytes.size()); + for (Map.Entry 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(); + if (!(record instanceof PositionDependentRecord)) continue; + + PositionDependentRecord pdr = (PositionDependentRecord) record; + int recordAt = pdr.getLastOnDiskOffset(); - // Is it one we care about? - for (Integer thisID : allIDs) { - int thatRecordAt = mostRecentByBytes.get(thisID); + Integer thisID = mostRecentByBytesRev.get(recordAt); + + if (thisID == null) continue; + + // Bingo. Now, where do we store it? + int storeAt = _sheetIdToCoreRecordsLookup.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; - } - } + // 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 088e0bcae..f70238879 100644 --- 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 index 000000000..b78519189 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 index 000000000..1b1a9630b Binary files /dev/null and b/test-data/slideshow/41246-2.ppt differ