Improved support for read-only recommended workbooks, fixing bug #44536

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

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index bd4f7dab037064e97eb3821b1b66627851f31553..9eb163fc8725c589c96275f03c6ed6f47e8ffb36 100644 (file)
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -36,6 +36,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">44536 - Improved support for detecting read-only recommended files</action>
            <action dev="POI-DEVELOPERS" type="fix">43901 - Correctly update the internal last cell number when adding and removing cells (previously sometimes off-by-one)</action>
            <action dev="POI-DEVELOPERS" type="fix">28231 - For apparently truncated files, which are somehow still valid, now issue a truncation warning but carry on, rather than giving an exception as before</action>
            <action dev="POI-DEVELOPERS" type="fix">44504 - Added initial support for recognising external functions like YEARFRAC and ISEVEN (using NameXPtg), via LinkTable support</action>

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 62d0a8dceb355c1a9f2e5764c7628d2cb0ef7d3b..201c4936d92995d70da800e71c2a8c2932481014 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -33,6 +33,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">44536 - Improved support for detecting read-only recommended files</action>
            <action dev="POI-DEVELOPERS" type="fix">43901 - Correctly update the internal last cell number when adding and removing cells (previously sometimes off-by-one)</action>
            <action dev="POI-DEVELOPERS" type="fix">44504 - Added initial support for recognising external functions like YEARFRAC and ISEVEN (using NameXPtg), via LinkTable support</action>
            <action dev="POI-DEVELOPERS" type="fix">44504 - Improvements to FormulaParser - operators, precedence, error literals, quotes in string literals, range checking on IntPtg, formulas with extra un-parsed stuff at the end, improved parse error handling</action>

diff --git a/src/java/org/apache/poi/hssf/model/Workbook.java b/src/java/org/apache/poi/hssf/model/Workbook.java
index 9bf1c8f475d9cc45fc95d3947296b94ba54a9f34..8fa3010a4b2ec6cbe64276d32d5e0db19ff1909b 100644 (file)
--- a/src/java/org/apache/poi/hssf/model/Workbook.java
+++ b/src/java/org/apache/poi/hssf/model/Workbook.java
@@ -2300,6 +2300,17 @@ public class Workbook implements Model
         }
         return this.fileShare;
     }
+    
+    /**
+     * is the workbook protected with a password (not encrypted)?
+     */
+    public boolean isWriteProtected() {
+        if (this.fileShare == null) {
+               return false;
+        }
+        FileSharingRecord frec = getFileSharing();
+        return (frec.getReadOnly() == 1);
+    }
 
     /**
      * protect a workbook with a password (not encypted, just sets writeprotect

diff --git a/src/java/org/apache/poi/hssf/record/FileSharingRecord.java b/src/java/org/apache/poi/hssf/record/FileSharingRecord.java
index 17f8cda64a14fbcf06959b98cdc4db4d7ffe401c..2f56eb092a0b481ac460ca2a8cbaed5311d2d051 100644 (file)
--- a/src/java/org/apache/poi/hssf/record/FileSharingRecord.java
+++ b/src/java/org/apache/poi/hssf/record/FileSharingRecord.java
@@ -19,6 +19,8 @@
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.POILogFactory;
+import org.apache.poi.util.POILogger;
 import org.apache.poi.util.StringUtil;
 
 /**
@@ -29,6 +31,8 @@ import org.apache.poi.util.StringUtil;
  */
 
 public class FileSharingRecord extends Record {
+    private static POILogger logger = POILogFactory.getLogger(FileSharingRecord.class);
+       
     public final static short sid = 0x5b;
     private short             field_1_readonly;
     private short             field_2_password;
@@ -58,8 +62,23 @@ public class FileSharingRecord extends Record {
         field_1_readonly = in.readShort();
         field_2_password = in.readShort();
         field_3_username_length = in.readByte();
+        
+        // Is this really correct? The latest docs
+        //  seem to hint there's nothing between the
+        //  username length and the username string
         field_4_unknown = in.readShort();
-        field_5_username = in.readCompressedUnicode(field_3_username_length);
+        
+        // Ensure we don't try to read more data than
+        //  there actually is
+        if(field_3_username_length > in.remaining()) {
+               logger.log(POILogger.WARN, "FileSharingRecord defined a username of length " + field_3_username_length + ", but only " + in.remaining() + " bytes were left, truncating");
+               field_3_username_length = (byte)in.remaining();
+        }
+        if(field_3_username_length > 0) {
+               field_5_username = in.readCompressedUnicode(field_3_username_length);
+        } else {
+               field_5_username = "";
+        }
     }
 
     //this is the world's lamest "security".  thanks to Wouter van Vugt for making me

diff --git a/src/java/org/apache/poi/hssf/record/RecordInputStream.java b/src/java/org/apache/poi/hssf/record/RecordInputStream.java
index 32094287fa73f26725c2d7cac219cb6f30812163..431558ccc8a2fd9cdf1cf831c4f090f3d9300256 100755 (executable)
--- a/src/java/org/apache/poi/hssf/record/RecordInputStream.java
+++ b/src/java/org/apache/poi/hssf/record/RecordInputStream.java
@@ -267,7 +267,7 @@ public class RecordInputStream extends InputStream
     
   public String readCompressedUnicode(int length) {
     if ((length < 0) || ((remaining() < length) && !isContinueNext())) {
-            throw new IllegalArgumentException("Illegal length");
+            throw new IllegalArgumentException("Illegal length " + length);
     }
 
     StringBuffer buf = new StringBuffer(length);

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
index e112da2212135b8c3e01338c6d7c3e9ce82601d7..1e89d6bebd3febf34bd192677d86dda0c4e9f2f8 100644 (file)
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
@@ -1379,6 +1379,13 @@ public class HSSFWorkbook extends POIDocument
         }
     }
 
+    /**
+     * Is the workbook protected with a password (not encrypted)?
+     */
+    public boolean isWriteProtected() {
+       return this.workbook.isWriteProtected();
+    }
+    
     /**
      * protect a workbook with a password (not encypted, just sets writeprotect
      * flags and the password.

diff --git a/src/testcases/org/apache/poi/hssf/data/ReadOnlyRecommended.xls b/src/testcases/org/apache/poi/hssf/data/ReadOnlyRecommended.xls
new file mode 100644 (file)
index 0000000..d479b94
Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/ReadOnlyRecommended.xls differ

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
index 04c729b3be8c40dd6ce42c357facfa125ad29fea..f9bb362c7b0b87743c5ac3da4a00009f3f739b17 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
@@ -1104,6 +1104,29 @@ extends TestCase {
         
         assertEquals(1, wb.getNumberOfSheets());
     }
+    
+    /**
+     * Files with "read only recommended" were giving
+     *  grief on the FileSharingRecord
+     */
+    public void test44536() throws Exception {
+        FileInputStream in = new FileInputStream(new File(cwd, "ReadOnlyRecommended.xls"));
+        
+        // Used to blow up with an IllegalArgumentException
+        //  when creating a FileSharingRecord
+        HSSFWorkbook wb = new HSSFWorkbook(in);
+        in.close();
+        
+        // Check read only advised
+        assertEquals(3, wb.getNumberOfSheets());
+        assertTrue(wb.isWriteProtected());
+        
+        // But also check that another wb isn't
+        in = new FileInputStream(new File(cwd, "SimpleWithChoose.xls"));
+        wb = new HSSFWorkbook(in);
+        in.close();
+        assertFalse(wb.isWriteProtected());
+    }
 }