From: Yegor Kozlov Date: Sat, 26 May 2007 07:22:51 +0000 (+0000) Subject: fixed bug 42520: NPE in Picture.getPictureData() and bug 42524: NPE in Shape.getShap... X-Git-Tag: REL_3_0_1_RC1~15 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=bd4852d48bba8dc2036412ebf199a092eeac5f9a;p=poi.git fixed bug 42520: NPE in Picture.getPictureData() and bug 42524: NPE in Shape.getShapeType(); Also changed the code to write messages to POILogger instead of System.err/System.out git-svn-id: https://svn.apache.org/repos/asf/jakarta/poi/trunk@541867 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 83afe99615..84a03e7a52 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java +++ b/src/scratchpad/src/org/apache/poi/hslf/HSLFSlideShow.java @@ -25,6 +25,8 @@ import java.io.*; import org.apache.poi.POIDocument; import org.apache.poi.util.LittleEndian; +import org.apache.poi.util.POILogger; +import org.apache.poi.util.POILogFactory; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.poifs.filesystem.DocumentEntry; import org.apache.poi.poifs.filesystem.DocumentInputStream; @@ -50,6 +52,9 @@ import org.apache.poi.hslf.usermodel.PictureData; public class HSLFSlideShow extends POIDocument { + // For logging + protected POILogger logger = POILogFactory.getLogger(this.getClass()); + private InputStream istream; // Holds metadata on where things are in our document @@ -226,7 +231,7 @@ public class HSLFSlideShow extends POIDocument try { currentUser = new CurrentUserAtom(filesystem); } catch(IOException ie) { - System.err.println("Error finding Current User Atom:\n" + ie); + logger.log(POILogger.ERROR, "Error finding Current User Atom:\n" + ie); currentUser = new CurrentUserAtom(); } } @@ -281,8 +286,8 @@ public class HSLFSlideShow extends POIDocument // If they type (including the bonus 0xF018) is 0, skip it if(type == 0) { - System.err.println("Problem reading picture: Invalid image type 0, on picture with length " + imgsize + ".\nYou document will probably become corrupted if you save it!"); - System.err.println(pos); + logger.log(POILogger.ERROR, "Problem reading picture: Invalid image type 0, on picture with length " + imgsize + ".\nYou document will probably become corrupted if you save it!"); + logger.log(POILogger.ERROR, "" + pos); } else { // Copy the data, ready to pass to PictureData byte[] imgdata = new byte[imgsize]; @@ -297,7 +302,7 @@ public class HSLFSlideShow extends POIDocument pict.setOffset(offset); p.add(pict); } catch(IllegalArgumentException e) { - System.err.println("Problem reading picture: " + e + "\nYou document will probably become corrupted if you save it!"); + logger.log(POILogger.ERROR, "Problem reading picture: " + e + "\nYou document will probably become corrupted if you save it!"); } } diff --git a/src/scratchpad/src/org/apache/poi/hslf/model/Picture.java b/src/scratchpad/src/org/apache/poi/hslf/model/Picture.java index 6de56b4035..d6f4887e27 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/model/Picture.java +++ b/src/scratchpad/src/org/apache/poi/hslf/model/Picture.java @@ -21,6 +21,7 @@ import org.apache.poi.hslf.usermodel.PictureData; import org.apache.poi.hslf.usermodel.SlideShow; import org.apache.poi.hslf.record.Document; import org.apache.poi.hslf.blip.Bitmap; +import org.apache.poi.util.POILogger; import javax.imageio.ImageIO; import java.awt.image.BufferedImage; @@ -99,7 +100,7 @@ public class Picture extends SimpleShape { public int getPictureIndex(){ EscherOptRecord opt = (EscherOptRecord)getEscherChild(_escherContainer, EscherOptRecord.RECORD_ID); EscherSimpleProperty prop = (EscherSimpleProperty)getEscherProperty(opt, EscherProperties.BLIP__BLIPTODISPLAY + 0x4000); - return prop.getPropertyValue(); + return prop == null ? 0 : prop.getPropertyValue(); } /** @@ -166,14 +167,18 @@ public class Picture extends SimpleShape { EscherContainerRecord bstore = (EscherContainerRecord)Shape.getEscherChild(dggContainer, EscherContainerRecord.BSTORE_CONTAINER); List lst = bstore.getChildRecords(); - int idx = getPictureIndex()-1; - EscherBSERecord bse = (EscherBSERecord)lst.get(idx); - for ( int i = 0; i < pict.length; i++ ) { - if (pict[i].getOffset() == bse.getOffset()){ - return pict[i]; + int idx = getPictureIndex(); + if (idx == 0){ + logger.log(POILogger.ERROR, "no reference to picture data found "); + } else { + EscherBSERecord bse = (EscherBSERecord)lst.get(idx-1); + for ( int i = 0; i < pict.length; i++ ) { + if (pict[i].getOffset() == bse.getOffset()){ + return pict[i]; + } } + logger.log(POILogger.ERROR, "no picture found for our BSE offset " + bse.getOffset()); } - System.err.println("Warning - no picture found for our BSE offset " + bse.getOffset()); return null; } diff --git a/src/scratchpad/src/org/apache/poi/hslf/model/Shape.java b/src/scratchpad/src/org/apache/poi/hslf/model/Shape.java index f1826b528b..65827bf94d 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/model/Shape.java +++ b/src/scratchpad/src/org/apache/poi/hslf/model/Shape.java @@ -19,6 +19,8 @@ package org.apache.poi.hslf.model; import org.apache.poi.ddf.*; import org.apache.poi.hslf.model.ShapeTypes; import org.apache.poi.hslf.record.ColorSchemeAtom; +import org.apache.poi.util.POILogger; +import org.apache.poi.util.POILogFactory; import java.util.Iterator; import java.awt.*; @@ -41,6 +43,9 @@ import java.awt.*; */ public abstract class Shape { + // For logging + protected POILogger logger = POILogFactory.getLogger(this.getClass()); + /** * In Escher absolute distances are specified in * English Metric Units (EMUs), occasionally referred to as A units; @@ -110,8 +115,7 @@ public abstract class Shape { * @return name of the shape. */ public String getShapeName(){ - EscherSpRecord spRecord = _escherContainer.getChildById(EscherSpRecord.RECORD_ID); - return ShapeTypes.typeName(spRecord.getOptions() >> 4); + return ShapeTypes.typeName(getShapeType()); } /** diff --git a/src/scratchpad/src/org/apache/poi/hslf/model/ShapeGroup.java b/src/scratchpad/src/org/apache/poi/hslf/model/ShapeGroup.java index caea86ae6e..d474a22f6f 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/model/ShapeGroup.java +++ b/src/scratchpad/src/org/apache/poi/hslf/model/ShapeGroup.java @@ -18,6 +18,7 @@ package org.apache.poi.hslf.model; import org.apache.poi.ddf.*; import org.apache.poi.util.LittleEndian; +import org.apache.poi.util.POILogger; import java.util.ArrayList; import java.util.List; @@ -69,7 +70,7 @@ public class ShapeGroup extends Shape{ } else { // Should we do anything special with these non // Container records? - System.err.println("Shape contained non container escher record, was " + r.getClass().getName()); + logger.log(POILogger.ERROR, "Shape contained non container escher record, was " + r.getClass().getName()); } } @@ -197,4 +198,17 @@ public class ShapeGroup extends Shape{ anchor.height = (spgr.getRectY2() - spgr.getRectY1())*POINT_DPI/MASTER_DPI; return anchor; } + + /** + * Return type of the shape. + * In most cases shape group type is {@link org.apache.poi.hslf.model.ShapeTypes#NotPrimitive} + * + * @return type of the shape. + */ + public int getShapeType(){ + EscherContainerRecord groupInfoContainer = (EscherContainerRecord)_escherContainer.getChild(0); + EscherSpRecord spRecord = groupInfoContainer.getChildById(EscherSpRecord.RECORD_ID); + return spRecord.getOptions() >> 4; + } + } diff --git a/src/scratchpad/src/org/apache/poi/hslf/model/TextBox.java b/src/scratchpad/src/org/apache/poi/hslf/model/TextBox.java index 67d2fee3a9..82d9a4de7f 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/model/TextBox.java +++ b/src/scratchpad/src/org/apache/poi/hslf/model/TextBox.java @@ -22,6 +22,7 @@ import org.apache.poi.ddf.*; import org.apache.poi.hslf.record.*; import org.apache.poi.hslf.usermodel.RichTextRun; import org.apache.poi.hslf.exceptions.HSLFException; +import org.apache.poi.util.POILogger; import java.awt.*; import java.awt.font.FontRenderContext; @@ -500,7 +501,7 @@ public class TextBox extends SimpleShape { _txtrun = new TextRun(tha,tca,sta); } else { // Empty text box - System.err.println("Warning - no text records found for TextBox"); + logger.log(POILogger.WARN, "no text records found for TextBox"); } } } diff --git a/src/scratchpad/testcases/org/apache/poi/hslf/data/42520.ppt b/src/scratchpad/testcases/org/apache/poi/hslf/data/42520.ppt new file mode 100644 index 0000000000..15bcf64106 Binary files /dev/null and b/src/scratchpad/testcases/org/apache/poi/hslf/data/42520.ppt differ diff --git a/src/scratchpad/testcases/org/apache/poi/hslf/model/TestSheet.java b/src/scratchpad/testcases/org/apache/poi/hslf/model/TestSheet.java index b583e2340a..be22db0d54 100644 --- a/src/scratchpad/testcases/org/apache/poi/hslf/model/TestSheet.java +++ b/src/scratchpad/testcases/org/apache/poi/hslf/model/TestSheet.java @@ -71,6 +71,8 @@ public class TestSheet extends TestCase{ } private void verify(Sheet sheet){ + assertNotNull(sheet.getSlideShow()); + ColorSchemeAtom colorscheme = sheet.getColorScheme(); assertNotNull(colorscheme); @@ -92,9 +94,11 @@ public class TestSheet extends TestCase{ Shape[] shape = sheet.getShapes(); assertTrue(shape != null); for (int i = 0; i < shape.length; i++) { + assertNotNull(shape[i].getSpContainer()); assertNotNull(shape[i].getSheet()); + assertNotNull(shape[i].getShapeName()); + assertNotNull(shape[i].getAnchor()); } - assertNotNull(sheet.getSlideShow()); } } 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 4e7f2ddaa2..800194e585 100644 --- a/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java +++ b/src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestBugs.java @@ -20,10 +20,12 @@ package org.apache.poi.hslf.usermodel; import junit.framework.TestCase; import org.apache.poi.hslf.HSLFSlideShow; import org.apache.poi.hslf.model.*; +import org.apache.poi.hslf.model.Shape; import java.io.*; import java.util.HashSet; import java.util.HashMap; +import java.awt.*; /** * Testcases for bugs entered in bugzilla @@ -201,4 +203,70 @@ public class TestBugs extends TestCase { } + /** + * Bug 42524: NPE in Shape.getShapeType() + */ + public void test42524 () throws Exception { + FileInputStream is = new FileInputStream(new File(cwd, "42486.ppt")); //test file is the same as for Bug 42486 + HSLFSlideShow hslf = new HSLFSlideShow(is); + is.close(); + + SlideShow ppt = new SlideShow(hslf); + //walk down the tree and see if there were no errors while reading + Slide[] slide = ppt.getSlides(); + for (int i = 0; i < slide.length; i++) { + Shape[] shape = slide[i].getShapes(); + for (int j = 0; j < shape.length; j++) { + assertNotNull(shape[j].getShapeName()); + if (shape[j] instanceof ShapeGroup){ + ShapeGroup group = (ShapeGroup)shape[j]; + Shape[] comps = group.getShapes(); + for (int k = 0; k < comps.length; k++) { + assertNotNull(comps[k].getShapeName()); + } + } + } + + } + assertTrue("No Exceptions while reading file", true); + + } + + /** + * Bug 42520: NPE in Picture.getPictureData() + */ + public void test42520 () throws Exception { + FileInputStream is = new FileInputStream(new File(cwd, "42520.ppt")); //test file is the same as for Bug 42486 + HSLFSlideShow hslf = new HSLFSlideShow(is); + is.close(); + + SlideShow ppt = new SlideShow(hslf); + + //test case from the bug report + ShapeGroup shapeGroup = (ShapeGroup)ppt.getSlides()[11].getShapes()[10]; + Picture picture = (Picture)shapeGroup.getShapes()[0]; + picture.getPictureData(); + + //walk down the tree and see if there were no errors while reading + Slide[] slide = ppt.getSlides(); + for (int i = 0; i < slide.length; i++) { + Shape[] shape = slide[i].getShapes(); + for (int j = 0; j < shape.length; j++) { + if (shape[j] instanceof ShapeGroup){ + ShapeGroup group = (ShapeGroup)shape[j]; + Shape[] comps = group.getShapes(); + for (int k = 0; k < comps.length; k++) { + Shape comp = comps[k]; + if (comp instanceof Picture){ + PictureData pict = ((Picture)comp).getPictureData(); + } + } + } + } + + } + assertTrue("No Exceptions while reading file", true); + + } + }