#61459 - HSLFShape.getShapeName() returns name of shapeType and not the shape name

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

8 files changed, 161 insertions, 55 deletions

diff --git a/src/java/org/apache/poi/sl/usermodel/Shape.java b/src/java/org/apache/poi/sl/usermodel/Shape.java
index 8afc26fcfd..a8ed1cfaaa 100644
--- a/src/java/org/apache/poi/sl/usermodel/Shape.java
+++ b/src/java/org/apache/poi/sl/usermodel/Shape.java
@@ -38,7 +38,14 @@ public interface Shape<
     * @return the anchor of this shape
     */
    Rectangle2D getAnchor();
-   
+
+   /**
+    * @return human-readable name of this shape, e.g. "Rectange 3"
+    *
+    * @since POI 4.0.0
+    */
+   String getShapeName();
+
    /**
     * Convenience method to draw a single shape
     *
diff --git a/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java b/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java
index 02e9fcb146..5f75258b9a 100644
--- a/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java
+++ b/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java
@@ -44,7 +44,10 @@ public class SlideShowFactory {
      *
      * @throws IOException if an error occurs while reading the data
      */
-    public static SlideShow<?,?> create(NPOIFSFileSystem fs) throws IOException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(NPOIFSFileSystem fs) throws IOException {
         return create(fs, null);
     }
 
@@ -59,7 +62,10 @@ public class SlideShowFactory {
      *
      * @throws IOException if an error occurs while reading the data
      */
-    public static SlideShow<?,?> create(final NPOIFSFileSystem fs, String password) throws IOException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(final NPOIFSFileSystem fs, String password) throws IOException {
         return create(fs.getRoot(), password);
     }
 
@@ -72,7 +78,10 @@ public class SlideShowFactory {
      *
      * @throws IOException if an error occurs while reading the data
      */
-    public static SlideShow<?,?> create(final DirectoryNode root) throws IOException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(final DirectoryNode root) throws IOException {
         return create(root, null);
     }
 
@@ -88,7 +97,10 @@ public class SlideShowFactory {
      *
      * @throws IOException if an error occurs while reading the data
      */
-    public static SlideShow<?,?> create(final DirectoryNode root, String password) throws IOException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(final DirectoryNode root, String password) throws IOException {
         // Encrypted OOXML files go inside OLE2 containers, is this one?
         if (root.hasEntry(Decryptor.DEFAULT_POIFS_ENTRY)) {
             InputStream stream = null;
@@ -136,7 +148,10 @@ public class SlideShowFactory {
      *  @throws IOException if an error occurs while reading the data
      *  @throws EncryptedDocumentException If the SlideShow given is password protected
      */
-    public static SlideShow<?,?> create(InputStream inp) throws IOException, EncryptedDocumentException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(InputStream inp) throws IOException, EncryptedDocumentException {
         return create(inp, null);
     }
 
@@ -160,7 +175,10 @@ public class SlideShowFactory {
      *  @throws IOException if an error occurs while reading the data
      *  @throws EncryptedDocumentException If the wrong password is given for a protected file
      */
-    public static SlideShow<?,?> create(InputStream inp, String password) throws IOException, EncryptedDocumentException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(InputStream inp, String password) throws IOException, EncryptedDocumentException {
         InputStream is = FileMagic.prepareToCheckMagic(inp);
         FileMagic fm = FileMagic.valueOf(is);
         
@@ -188,7 +206,10 @@ public class SlideShowFactory {
      *  @throws IOException if an error occurs while reading the data
      *  @throws EncryptedDocumentException If the SlideShow given is password protected
      */
-    public static SlideShow<?,?> create(File file) throws IOException, EncryptedDocumentException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(File file) throws IOException, EncryptedDocumentException {
         return create(file, null);
     }
 
@@ -207,7 +228,10 @@ public class SlideShowFactory {
      *  @throws IOException if an error occurs while reading the data
      *  @throws EncryptedDocumentException If the wrong password is given for a protected file
      */
-    public static SlideShow<?,?> create(File file, String password) throws IOException, EncryptedDocumentException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(File file, String password) throws IOException, EncryptedDocumentException {
         return create(file, password, false);
     }
 
@@ -228,7 +252,10 @@ public class SlideShowFactory {
      *  @throws IOException if an error occurs while reading the data
      *  @throws EncryptedDocumentException If the wrong password is given for a protected file
      */
-    public static SlideShow<?,?> create(File file, String password, boolean readOnly) throws IOException, EncryptedDocumentException {
+    public static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> create(File file, String password, boolean readOnly) throws IOException, EncryptedDocumentException {
         if (!file.exists()) {
             throw new FileNotFoundException(file.toString());
         }
@@ -246,15 +273,24 @@ public class SlideShowFactory {
         }
     }
     
-    protected static SlideShow<?,?> createHSLFSlideShow(Object... args) throws IOException, EncryptedDocumentException {
+    private static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> createHSLFSlideShow(Object... args) throws IOException, EncryptedDocumentException {
         return createSlideShow("org.apache.poi.hslf.usermodel.HSLFSlideShowFactory", args);
     }
     
-    protected static SlideShow<?,?> createXSLFSlideShow(Object... args) throws IOException, EncryptedDocumentException {
+    private static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> createXSLFSlideShow(Object... args) throws IOException, EncryptedDocumentException {
         return createSlideShow("org.apache.poi.xslf.usermodel.XSLFSlideShowFactory", args);
     }
-    
-    protected static SlideShow<?,?> createSlideShow(String factoryClass, Object args[]) throws IOException, EncryptedDocumentException {
+
+    private static <
+        S extends Shape<S,P>,
+        P extends TextParagraph<S,P,? extends TextRun>
+    > SlideShow<S,P> createSlideShow(String factoryClass, Object args[]) throws IOException, EncryptedDocumentException {
         try {
             Class<?> clazz = Thread.currentThread().getContextClassLoader().loadClass(factoryClass);
             Class<?> argsClz[] = new Class<?>[args.length];
@@ -269,7 +305,7 @@ public class SlideShowFactory {
                 argsClz[i++] = c;
             }
             Method m = clazz.getMethod("createSlideShow", argsClz);
-            return (SlideShow<?,?>)m.invoke(null, args);
+            return (SlideShow<S,P>)m.invoke(null, args);
         } catch (InvocationTargetException e) {
             Throwable t = e.getCause();
             if (t instanceof IOException) {
diff --git a/src/java/org/apache/poi/util/StringUtil.java b/src/java/org/apache/poi/util/StringUtil.java
index ee0fe27215..302e532570 100644
--- a/src/java/org/apache/poi/util/StringUtil.java
+++ b/src/java/org/apache/poi/util/StringUtil.java
@@ -652,4 +652,76 @@ public class StringUtil {
         }
         return count;
     }
+
+
+    /**
+     * Given a byte array of 16-bit unicode characters in Little Endian
+     * format (most important byte last), return a Java String representation
+     * of it.
+     *
+     * Scans the byte array for two continous 0 bytes and returns the string before.
+     * <p>
+     *
+     * #61881: there seem to be programs out there, which write the 0-termination also
+     * at the beginning of the string. Check if the next two bytes contain a valid ascii char
+     * and correct the _recdata with a '?' char
+     *
+     *
+     * @param string the byte array to be converted
+     * @param offset the initial offset into the
+     *               byte array. it is assumed that string[ offset ] and string[ offset +
+     *               1 ] contain the first 16-bit unicode character
+     * @param len    the max. length of the final string
+     * @return the converted string, never <code>null</code>.
+     * @throws ArrayIndexOutOfBoundsException if offset is out of bounds for
+     *                                        the byte array (i.e., is negative or is greater than or equal to
+     *                                        string.length)
+     * @throws IllegalArgumentException       if len is too large (i.e.,
+     *                                        there is not enough data in string to create a String of that
+     *                                        length)
+     */
+    public static String getFromUnicodeLE0Terminated(
+            final byte[] string,
+            final int offset,
+            final int len)
+            throws ArrayIndexOutOfBoundsException, IllegalArgumentException {
+        if ((offset < 0) || (offset >= string.length)) {
+            throw new ArrayIndexOutOfBoundsException("Illegal offset " + offset + " (String data is of length " + string.length + ")");
+        }
+
+        if ((len < 0) || (((string.length - offset) / 2) < len)) {
+            throw new IllegalArgumentException("Illegal length " + len);
+        }
+
+        final int newOffset;
+        final int newMaxLen;
+        final String prefix;
+
+        // #61881 - for now we only check the first char
+        if (len > 0 && string[offset] == 0 && string[offset+1] == 0) {
+            newOffset = offset+2;
+            prefix = "?";
+
+            // check if the next char is garbage and limit the len if necessary
+            final int cp = (len > 1) ? LittleEndian.getShort(string, offset+2) : 0;
+            newMaxLen = Character.isJavaIdentifierPart(cp) ? len-1 : 0;
+        } else {
+            newOffset = offset;
+            prefix = "";
+            newMaxLen = len;
+        }
+
+        int newLen = 0;
+
+        // loop until we find a null-terminated end
+        for(; newLen < newMaxLen; newLen++) {
+            if (string[newOffset + newLen * 2] == 0 && string[newOffset + newLen * 2 + 1] == 0) {
+                break;
+            }
+        }
+        newLen = Math.min(newLen, newMaxLen);
+
+        return prefix + ((newLen == 0) ? "" : new String(string, newOffset, newLen * 2, UTF16LE));
+    }
+
 }
diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java
index 35a54e96ed..82449d846a 100644
--- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java
+++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java
@@ -97,9 +97,7 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
         return _sheet;
     }
     
-    /**
-     * @return human-readable name of this shape, e.g. "Rectange 3"
-     */
+    @Override
     public String getShapeName(){
         return getCNvPr().getName();
     }
diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlideShowFactory.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlideShowFactory.java
index 71378e70ec..501132c00c 100644
--- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlideShowFactory.java
+++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlideShowFactory.java
@@ -25,7 +25,6 @@ import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackageAccess;
-import org.apache.poi.sl.usermodel.SlideShow;
 import org.apache.poi.sl.usermodel.SlideShowFactory;
 import org.apache.poi.util.Internal;
 
@@ -45,7 +44,7 @@ public class XSLFSlideShowFactory extends SlideShowFactory {
      *  @throws IOException if an error occurs while reading the data
      * @throws InvalidFormatException 
      */
-    public static SlideShow<?,?> createSlideShow(OPCPackage pkg) throws IOException {
+    public static XMLSlideShow createSlideShow(OPCPackage pkg) throws IOException {
         try {
             return new XMLSlideShow(pkg);
         } catch (IllegalArgumentException ioe) {
@@ -72,7 +71,7 @@ public class XSLFSlideShowFactory extends SlideShowFactory {
      *  @throws EncryptedDocumentException If the wrong password is given for a protected file
      */
     @SuppressWarnings("resource")
-    public static SlideShow<?,?> createSlideShow(File file, boolean readOnly)
+    public static XMLSlideShow createSlideShow(File file, boolean readOnly)
     throws IOException, InvalidFormatException {
         OPCPackage pkg = OPCPackage.open(file, readOnly ? PackageAccess.READ : PackageAccess.READ_WRITE);
         return createSlideShow(pkg);
@@ -92,7 +91,7 @@ public class XSLFSlideShowFactory extends SlideShowFactory {
      * @throws InvalidFormatException 
      */
     @SuppressWarnings("resource")
-    public static SlideShow<?,?> createSlideShow(InputStream stream) throws IOException, InvalidFormatException {
+    public static XMLSlideShow createSlideShow(InputStream stream) throws IOException, InvalidFormatException {
         OPCPackage pkg = OPCPackage.open(stream);
         return createSlideShow(pkg);
     }
diff --git a/src/scratchpad/src/org/apache/poi/hslf/record/FontEntityAtom.java b/src/scratchpad/src/org/apache/poi/hslf/record/FontEntityAtom.java
index 118fc1d3bf..4f3cdd89bc 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/record/FontEntityAtom.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/record/FontEntityAtom.java
@@ -86,38 +86,8 @@ public final class FontEntityAtom extends RecordAtom {
      * @return font name
      */
     public String getFontName(){
-    	final int maxLen = Math.min(_recdata.length,64);
-        for(int i = 0; i+1 < maxLen; i+=2){
-            //loop until find null-terminated end of the font name
-            if(_recdata[i] == 0 && _recdata[i + 1] == 0 && !isFontNamePremature0terminated(i)) {
-                return StringUtil.getFromUnicodeLE(_recdata, 0, i/2);
-            }
-        }
-        return null;
-    }
-    
-    /**
-     * #61881: there seem to be programs out there, which write the 0-termination also
-     * at the beginning of the string. Check if the next two bytes contain a valid ascii char
-     * and correct the _recdata with a '?' char
-     */
-    private boolean isFontNamePremature0terminated(final int index) {
-        if (index > 0) {
-            // for now we only check the first char
-            return false;
-        }
-        
-        if (_recdata.length < index+4) {
-            return false;
-        }
-        
-        final int cp = LittleEndian.getShort(_recdata, index+2);
-        if (!Character.isJavaIdentifierPart(cp)) {
-            return false;
-        }
-        
-        _recdata[index] = '?';
-        return true;
+    	final int maxLen = Math.min(_recdata.length,64)/2;
+    	return StringUtil.getFromUnicodeLE0Terminated(_recdata, 0, maxLen);
     }
 
     /**
diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java
index a61bbaedc6..4edcb53603 100644
--- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java
+++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java
@@ -30,6 +30,7 @@ import org.apache.poi.ddf.EscherClientDataRecord;
 import org.apache.poi.ddf.EscherColorRef;
 import org.apache.poi.ddf.EscherColorRef.SysIndexProcedure;
 import org.apache.poi.ddf.EscherColorRef.SysIndexSource;
+import org.apache.poi.ddf.EscherComplexProperty;
 import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherProperties;
 import org.apache.poi.ddf.EscherProperty;
@@ -49,6 +50,7 @@ import org.apache.poi.sl.usermodel.ShapeContainer;
 import org.apache.poi.sl.usermodel.ShapeType;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
+import org.apache.poi.util.StringUtil;
 import org.apache.poi.util.Units;
 
 /**
@@ -123,8 +125,15 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
     /**
      * @return name of the shape.
      */
+    @Override
     public String getShapeName(){
-        return getShapeType().nativeName;
+        final EscherComplexProperty ep = getEscherProperty(getEscherOptRecord(), EscherProperties.GROUPSHAPE__SHAPENAME);
+        if (ep != null) {
+            final byte[] cd = ep.getComplexData();
+            return StringUtil.getFromUnicodeLE0Terminated(cd, 0, cd.length/2);
+        } else {
+            return getShapeType().nativeName+" "+getShapeId();
+        }
     }
 
     public ShapeType getShapeType(){
diff --git a/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java b/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java
index 47b886e174..347ea73a6c 100644
--- a/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java
+++ b/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java
@@ -157,4 +157,19 @@ public abstract class BaseTestSlideShow {
             }
         }        
     }
+
+    @Test
+    public void shapeName() throws IOException {
+        final String file = "SampleShow.ppt"+(getClass().getSimpleName().contains("XML")?"x":"");
+        try (final InputStream is = slTests.openResourceAsStream(file)) {
+            try (final SlideShow<? extends Shape, ?> ppt = SlideShowFactory.create(is)) {
+                final List<? extends Shape> shapes1 = ppt.getSlides().get(0).getShapes();
+                assertEquals("The Title", shapes1.get(0).getShapeName());
+                assertEquals("Another Subtitle", shapes1.get(1).getShapeName());
+                final List<? extends Shape> shapes2 = ppt.getSlides().get(1).getShapes();
+                assertEquals("Title 1", shapes2.get(0).getShapeName());
+                assertEquals("Content Placeholder 2", shapes2.get(1).getShapeName());
+            }
+        }
+    }
 }