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

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!");
+				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!");
-				System.err.println(pos);
+				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!");
 				}
 			}
             

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;
+        int idx = getPictureIndex();
-        EscherBSERecord bse = (EscherBSERecord)lst.get(idx);
+        if (idx == 0){
-        for ( int i = 0; i < pict.length; i++ ) {
+            logger.log(POILogger.ERROR, "no reference to picture data found ");
-			if (pict[i].getOffset() ==  bse.getOffset()){
+        } else {
-                return pict[i];
+            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;
     }
 

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(getShapeType());
-        return ShapeTypes.typeName(spRecord.getOptions() >> 4);
     }
 
     /**

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;
+    }
+
 }

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");
         }
     }
 }

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());
     }
 }

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);
+
+    }
+
 }