Fix for bug 45582 - handle extra bytes after the EOFRecord

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

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">45582 - Fix for workbook streams with extra bytes trailing the EOFRecord</action>
            <action dev="POI-DEVELOPERS" type="add">45537 - Include headers and footers (of slides and notes) in the extracted text from HSLF</action>
            <action dev="POI-DEVELOPERS" type="fix">45472 - Fixed incorrect default row height in OpenOffice 2.3</action>
            <action dev="POI-DEVELOPERS" type="fix">44692 - HSSFPicture.resize() stretched image when there was a text next to it</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">45582 - Fix for workbook streams with extra bytes trailing the EOFRecord</action>
            <action dev="POI-DEVELOPERS" type="add">45537 - Include headers and footers (of slides and notes) in the extracted text from HSLF</action>
            <action dev="POI-DEVELOPERS" type="fix">45472 - Fixed incorrect default row height in OpenOffice 2.3</action>
            <action dev="POI-DEVELOPERS" type="fix">44692 - HSSFPicture.resize() stretched image when there was a text next to it</action>

src/java/org/apache/poi/hssf/eventusermodel/EventWorkbookBuilder.java

@@ -96,7 +96,7 @@ public class EventWorkbookBuilder {
 		}
 		
 		// Finally we need an EoF record
-		wbRecords.add(new EOFRecord());
+		wbRecords.add(EOFRecord.instance);
 		
 		return Workbook.createWorkbook(wbRecords);
 	}

src/java/org/apache/poi/hssf/model/Sheet.java

@@ -550,7 +550,7 @@ public final class Sheet implements Model {
         retval.setLoc(records.size() - 1);
         retval.selection = createSelection();
         records.add(retval.selection);
-        records.add(new EOFRecord());
+        records.add(EOFRecord.instance);
 
 
         retval.records = records;

src/java/org/apache/poi/hssf/model/Workbook.java

@@ -351,7 +351,7 @@ public class Workbook implements Model
         records.add( retval.sst );
         records.add( retval.createExtendedSST() );
 
-        records.add( retval.createEOF() );
+        records.add(EOFRecord.instance);
         if (log.check( POILogger.DEBUG ))
             log.log( DEBUG, "exit create new workbook from scratch" );
         return retval;
@@ -1858,17 +1858,6 @@ public class Workbook implements Model
         return retval;
     }
     
-    /**
-     * creates the EOF record
-     * @see org.apache.poi.hssf.record.EOFRecord
-     * @see org.apache.poi.hssf.record.Record
-     * @return record containing a EOFRecord
-     */
-
-    protected Record createEOF() {
-        return new EOFRecord();
-    }
-    
     /**
      * lazy initialization
      * Note - creating the link table causes creation of 1 EXTERNALBOOK and 1 EXTERNALSHEET record

src/java/org/apache/poi/hssf/record/EOFRecord.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.LittleEndian;
@@ -31,14 +29,14 @@ import org.apache.poi.util.LittleEndian;
  * @author Jason Height (jheight at chariot dot net dot au)
  * @version 2.0-pre
  */
-
+public final class EOFRecord extends Record {
-public class EOFRecord
-    extends Record
-{
     public final static short sid = 0x0A;
+	public static final int ENCODED_SIZE = 4;
 
-    public EOFRecord()
+	public static final EOFRecord instance = new EOFRecord();
-    {
+	
+    private EOFRecord() {
+    	// no data fields
     }
 
     /**
@@ -82,7 +80,7 @@ public class EOFRecord
 
     public int getRecordSize()
     {
-        return 4;
+        return ENCODED_SIZE;
     }
 
     public short getSid()
@@ -91,7 +89,6 @@ public class EOFRecord
     }
 
     public Object clone() {
-      EOFRecord rec = new EOFRecord();
+      return instance;
-      return rec;
     }
 }

src/java/org/apache/poi/hssf/record/ObjRecord.java

@@ -30,7 +30,7 @@ import org.apache.poi.util.LittleEndian;
  * @author Glen Stampoultzis (glens at apache.org)
  */
 public final class ObjRecord extends Record {
-    public final static short      sid                             = 0x5D;
+    public final static short      sid                             = 0x005D;
     private List subrecords;
 
     //00000000 15 00 12 00 01 00 01 00 11 60 00 00 00 00 00 0D .........`......
@@ -69,6 +69,12 @@ public final class ObjRecord extends Record {
 
     protected void fillFields(RecordInputStream in)
     {
+    	// TODO - problems with OBJ sub-records stream
+    	// MS spec says first sub-records is always CommonObjectDataSubRecord, and last is 
+    	// always EndSubRecord.  OOO spec does not mention ObjRecord(0x005D).
+    	// Existing POI test data seems to violate that rule.  Some test data seems to contain
+    	// garbage, and a crash is only averted by stopping at what looks like the 'EndSubRecord'
+    	
         subrecords = new ArrayList();
         //Check if this can be continued, if so then the
         //following wont work properly
@@ -81,6 +87,9 @@ public final class ObjRecord extends Record {
           Record subRecord = SubRecord.createSubRecord(subRecStream);
           subSize += subRecord.getRecordSize();
           subrecords.add(subRecord);
+          if (subRecord instanceof EndSubRecord) {
+        	  break;
+          }
         }
 
         /**

src/java/org/apache/poi/hssf/record/RecordInputStream.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.LittleEndian;
@@ -31,16 +29,15 @@ import java.io.ByteArrayOutputStream;
  *
  * @author Jason Height (jheight @ apache dot org)
  */
-
+public class RecordInputStream extends InputStream {
-public class RecordInputStream extends InputStream
-{
   /** Maximum size of a single record (minus the 4 byte header) without a continue*/
   public final static short MAX_RECORD_DATA_SIZE = 8224;
+  private static final int INVALID_SID_VALUE = -1;
   
   private InputStream in;
   protected short currentSid;
   protected short currentLength = -1;
-  protected short nextSid = -1;
+  protected short nextSid;
 
   protected byte[] data = new byte[MAX_RECORD_DATA_SIZE];
   protected short recordOffset;
@@ -60,7 +57,7 @@ public class RecordInputStream extends InputStream
   }
   
   /** This method will read a byte from the current record*/
-  public int read() throws IOException {
+  public int read() {
     checkRecordPosition();
 
     byte result = data[recordOffset];
@@ -86,7 +83,7 @@ public class RecordInputStream extends InputStream
   }
 
   public boolean hasNextRecord() {
-    return (nextSid != 0);
+    return nextSid != INVALID_SID_VALUE;
   }
   
   /** Moves to the next record in the stream.
@@ -110,7 +107,20 @@ public class RecordInputStream extends InputStream
       in.read(data, 0, currentLength);
 
       //Read the Sid of the next record
+      if (in.available() < EOFRecord.ENCODED_SIZE) {
+          if (in.available() > 0) {
+              // some scrap left over?
+              // ex45582-22397.xls has one extra byte after the last record
+              // Excel reads that file OK
+          }
+          nextSid = INVALID_SID_VALUE;  
+      } else {
           nextSid = LittleEndian.readShort(in);
+          if (nextSid == INVALID_SID_VALUE) {
+              throw new RecordFormatException("Found sid " + nextSid + " after record with sid 0x"
+                      + Integer.toHexString(currentSid).toUpperCase());
+          }
+      }      
     } catch (IOException ex) {
       throw new RecordFormatException("Error reading bytes", ex);
     }

src/java/org/apache/poi/util/ByteField.java

@@ -22,6 +22,7 @@ package org.apache.poi.util;
 import org.apache.poi.util.LittleEndian.BufferUnderrunException;
 
 import java.io.*;
+import java.nio.BufferUnderflowException;
 
 /**
  * representation of a byte (8-bit) field at a fixed location within a
@@ -183,9 +184,12 @@ public class ByteField
     public void readFromStream(final InputStream stream)
         throws IOException, BufferUnderrunException
     {
-        _value =
+    	// TODO - are these ~Field used / necessary
-            (LittleEndian.readFromStream(stream,
+    	int ib = stream.read();
-                                         LittleEndianConsts.BYTE_SIZE))[ 0 ];
+    	if (ib < 0) {
+    		throw new BufferUnderflowException();
+    	}
+        _value = (byte) ib;
     }
 
     /**

src/java/org/apache/poi/util/LittleEndian.java

@@ -19,7 +19,6 @@ package org.apache.poi.util;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Arrays;
 
 /**
  *  a utility class for handling little-endian numbers, which the 80x86 world is
@@ -29,16 +28,11 @@ import java.util.Arrays;
  *@author     Marc Johnson (mjohnson at apache dot org)
  *@author     Andrew Oliver (acoliver at apache dot org)
  */
+public final class LittleEndian implements LittleEndianConsts {
 
-public class LittleEndian
+    private LittleEndian() {
-         implements LittleEndianConsts {
+    	// no instances of this class
-
+    }
-    // all methods are static, so an accessible constructor makes no
-    // sense
-    /**
-     *  Constructor for the LittleEndian object
-     */
-    private LittleEndian() { }
 
 
     /**
@@ -385,12 +379,7 @@ public class LittleEndian
      *@author     Marc Johnson (mjohnson at apache dot org)
      */
 
-    public static class BufferUnderrunException
+    public static final class BufferUnderrunException extends IOException {
-             extends IOException {
-
-        /**
-         *  simple constructor
-         */
 
         BufferUnderrunException() {
             super("buffer underrun");
@@ -408,10 +397,19 @@ public class LittleEndian
      *@exception  BufferUnderrunException  if the stream cannot provide enough
      *      bytes
      */
+    public static short readShort(InputStream stream) throws IOException, BufferUnderrunException {
 
-    public static short readShort(final InputStream stream)
+		return (short) readUShort(stream);
-             throws IOException, BufferUnderrunException {
+	}
-        return getShort(readFromStream(stream, SHORT_SIZE));
+
+	public static int readUShort(InputStream stream) throws IOException, BufferUnderrunException {
+
+		int ch1 = stream.read();
+		int ch2 = stream.read();
+		if ((ch1 | ch2) < 0) {
+			throw new BufferUnderrunException();
+		}
+		return ((ch2 << 8) + (ch1 << 0));
 	}
     
 
@@ -425,10 +423,16 @@ public class LittleEndian
      *@exception  BufferUnderrunException  if the stream cannot provide enough
      *      bytes
      */
-
     public static int readInt(final InputStream stream)
              throws IOException, BufferUnderrunException {
-        return getInt(readFromStream(stream, INT_SIZE));
+		int ch1 = stream.read();
+		int ch2 = stream.read();
+		int ch3 = stream.read();
+		int ch4 = stream.read();
+		if ((ch1 | ch2 | ch3 | ch4) < 0) {
+			throw new BufferUnderrunException();
+		}
+		return ((ch4 << 24) + (ch3<<16) + (ch2 << 8) + (ch1 << 0));
     }
 
 
@@ -445,45 +449,28 @@ public class LittleEndian
 
     public static long readLong(final InputStream stream)
              throws IOException, BufferUnderrunException {
-        return getLong(readFromStream(stream, LONG_SIZE));
+		int ch1 = stream.read();
-    }
+		int ch2 = stream.read();
-
+		int ch3 = stream.read();
-    /**
+		int ch4 = stream.read();
-     *  Read the appropriate number of bytes from the stream and return them to
+		int ch5 = stream.read();
-     *  the caller. <p>
+		int ch6 = stream.read();
-     *
+		int ch7 = stream.read();
-     *  However, for the purposes of the POI project, this risk is deemed
+		int ch8 = stream.read();
-     *  negligible. It is, however, so noted.
+		if ((ch1 | ch2 | ch3 | ch4 | ch5 | ch6 | ch7 | ch8) < 0) {
-     *
-     *@param  stream                       the InputStream we're reading from
-     *@param  size                         the number of bytes to read; in
-     *      99.99% of cases, this will be SHORT_SIZE, INT_SIZE, or LONG_SIZE --
-     *      but it doesn't have to be.
-     *@return                              the byte array containing the
-     *      required number of bytes. The array will contain all zero's on end
-     *      of stream
-     *@exception  IOException              will be propagated back to the caller
-     *@exception  BufferUnderrunException  if the stream cannot provide enough
-     *      bytes
-     */
-
-    public static byte[] readFromStream(final InputStream stream,
-            final int size)
-             throws IOException, BufferUnderrunException {
-        byte[] buffer = new byte[size];
-
-        int count = stream.read(buffer);
-
-        if (count == -1) {
-
-            // return a zero-filled buffer
-            Arrays.fill(buffer, (byte) 0);
-        } else if (count != size) {
 			throw new BufferUnderrunException();
 		}
-        return buffer;
-    }
 		
+		return 
+			((long)ch8 << 56) +
+            ((long)ch7 << 48) +
+            ((long)ch6 << 40) +
+            ((long)ch5 << 32) +
+            ((long)ch4 << 24) + // cast to long to preserve bit 31 (sign bit for ints)
+                  (ch3 << 16) +
+                  (ch2 <<  8) +
+                  (ch1 <<  0);
+    }
 
     /**
      *  Gets the number attribute of the LittleEndian class

src/scratchpad/src/org/apache/poi/hssf/usermodel/HSSFChart.java

@@ -153,7 +153,7 @@ public final class HSSFChart {
 		records.add( createSeriesIndexRecord(2) );
 		records.add( createSeriesIndexRecord(1) );
 		records.add( createSeriesIndexRecord(3) );
-		records.add( createEOFRecord() );
+		records.add(EOFRecord.instance);
 
 
 
@@ -260,12 +260,6 @@ public final class HSSFChart {
 		}
 	}
 
-
-	private EOFRecord createEOFRecord()
-	{
-		return new EOFRecord();
-	}
-
 	private SeriesIndexRecord createSeriesIndexRecord( int index )
 	{
 		SeriesIndexRecord r = new SeriesIndexRecord();

src/testcases/org/apache/poi/hssf/eventmodel/TestEventRecordFactory.java

@@ -63,7 +63,7 @@ public final class TestEventRecordFactory extends TestCase {
         bof.setVersion((short)0x06);
         bof.setHistoryBitMask(BOFRecord.HISTORY_MASK);
         
-        EOFRecord eof = new EOFRecord();
+        EOFRecord eof = EOFRecord.instance;
     	byte[] bytes = new byte[bof.getRecordSize() + eof.getRecordSize()];
         int offset = 0;
         offset = bof.serialize(offset,bytes);

src/testcases/org/apache/poi/hssf/model/TestSheet.java

@@ -54,7 +54,7 @@ public final class TestSheet extends TestCase {
         List records = new ArrayList();
         records.add( new BOFRecord() );
         records.add( new DimensionsRecord() );
-        records.add( new EOFRecord() );
+        records.add(EOFRecord.instance);
         Sheet sheet = Sheet.createSheet( records, 0, 0 );
 
         int pos = 0;
@@ -396,7 +396,7 @@ public final class TestSheet extends TestCase {
         records.add(new BOFRecord());
         records.add(new UncalcedRecord());
         records.add(new DimensionsRecord());
-        records.add(new EOFRecord());
+        records.add(EOFRecord.instance);
         Sheet sheet = Sheet.createSheet(records, 0, 0);
 
         int estimatedSize = sheet.getSize();

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java

@@ -1,19 +1,19 @@
-/*
+/* ====================================================================
-* Licensed to the Apache Software Foundation (ASF) under one or more
+   Licensed to the Apache Software Foundation (ASF) under one or more
-* contributor license agreements.  See the NOTICE file distributed with
+   contributor license agreements.  See the NOTICE file distributed with
-* this work for additional information regarding copyright ownership.
+   this work for additional information regarding copyright ownership.
-* The ASF licenses this file to You under the Apache License, Version 2.0
+   The ASF licenses this file to You under the Apache License, Version 2.0
-* (the "License"); you may not use this file except in compliance with
+   (the "License"); you may not use this file except in compliance with
-* the License.  You may obtain a copy of the License at
+   the License.  You may obtain a copy of the License at
-*
+
-*     http://www.apache.org/licenses/LICENSE-2.0
+       http://www.apache.org/licenses/LICENSE-2.0
-*
+
-* Unless required by applicable law or agreed to in writing, software
+   Unless required by applicable law or agreed to in writing, software
-* distributed under the License is distributed on an "AS IS" BASIS,
+   distributed under the License is distributed on an "AS IS" BASIS,
-* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-* See the License for the specific language governing permissions and
+   See the License for the specific language governing permissions and
-* limitations under the License.
+   limitations under the License.
-*/
+==================================================================== */
 
 package org.apache.poi.hssf.usermodel;
 
@@ -29,8 +29,10 @@ import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.model.Sheet;
 import org.apache.poi.hssf.record.NameRecord;
 import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.hssf.record.formula.Area3DPtg;
+import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.TempFile;
 /**
  *
@@ -512,4 +514,17 @@ public final class TestHSSFWorkbook extends TestCase {
             return 8;
         }
     }
+    
+    /**
+     * The sample file provided with bug 45582 seems to have one extra byte after the EOFRecord
+     */
+    public void testExtraDataAfterEOFRecord() {
+    	try {
+    		HSSFTestDataSamples.openSampleWorkbook("ex45582-22397.xls");
+    	} catch (RecordFormatException e) {
+    		if (e.getCause() instanceof LittleEndian.BufferUnderrunException) {
+    			throw new AssertionFailedError("Identified bug 45582");
     		}
+    	}
+	}
+}

src/testcases/org/apache/poi/hssf/usermodel/TestSanityChecker.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -27,27 +26,20 @@ import java.util.ArrayList;
 import org.apache.poi.hssf.record.*;
 
 /**
+ * A Test case for a test utility class.<br/>
  * Okay, this may seem strange but I need to test my test logic.
  *
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class TestSanityChecker
+public final class TestSanityChecker extends TestCase {
-        extends TestCase
-{
-    public TestSanityChecker( String s )
-    {
-        super( s );
-    }
 
-    public void testCheckRecordOrder()
+    public void testCheckRecordOrder() {
-            throws Exception
-    {
         final SanityChecker c = new SanityChecker();
         List records = new ArrayList();
         records.add(new BOFRecord());
         records.add(new InterfaceHdrRecord());
         records.add(new BoundSheetRecord());
-        records.add(new EOFRecord());
+        records.add(EOFRecord.instance);
         final SanityChecker.CheckRecord[] check = {
             new SanityChecker.CheckRecord(BOFRecord.class, '1'),
             new SanityChecker.CheckRecord(InterfaceHdrRecord.class, '0'),
@@ -74,7 +66,7 @@ public class TestSanityChecker
                 records.add(new BOFRecord());
                 records.add(new BoundSheetRecord());
                 records.add(new InterfaceHdrRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 c.checkRecordOrder(records, check);
             }
         });
@@ -88,7 +80,7 @@ public class TestSanityChecker
                 records.add(new InterfaceHdrRecord());
                 records.add(new BoundSheetRecord());
                 records.add(new InterfaceHdrRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 c.checkRecordOrder(records, check);
             }
         });
@@ -101,7 +93,7 @@ public class TestSanityChecker
                 records.add(new BOFRecord());
                 records.add(new BoundSheetRecord());
                 records.add(new NameRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 records.add(new NameRecord());
                 c.checkRecordOrder(records, check);
             }
@@ -114,7 +106,7 @@ public class TestSanityChecker
                 List records = new ArrayList();
                 records.add(new InterfaceHdrRecord());
                 records.add(new BoundSheetRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 c.checkRecordOrder(records, check);
             }
         });
@@ -126,7 +118,7 @@ public class TestSanityChecker
                 List records = new ArrayList();
                 records.add(new BOFRecord());
                 records.add(new InterfaceHdrRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 c.checkRecordOrder(records, check);
             }
         });
@@ -139,7 +131,7 @@ public class TestSanityChecker
                 records.add(new InterfaceHdrRecord());
                 records.add(new BoundSheetRecord());
                 records.add(new BOFRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 c.checkRecordOrder(records, check);
             }
         });
@@ -152,7 +144,7 @@ public class TestSanityChecker
                 records.add(new BOFRecord());
                 records.add(new BoundSheetRecord());
                 records.add(new InterfaceHdrRecord());
-                records.add(new EOFRecord());
+                records.add(EOFRecord.instance);
                 c.checkRecordOrder(records, check);
             }
         });

src/testcases/org/apache/poi/util/TestLittleEndian.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.util;
 
 import junit.framework.TestCase;
@@ -31,79 +29,54 @@ import java.io.InputStream;
  *
  * @author Marc Johnson
  */
-
+public final class TestLittleEndian extends TestCase {
-public class TestLittleEndian
-    extends TestCase
-{
-
-    /**
-     * Constructor TestLittleEndian
-     *
-     * @param name
-     */
-    public TestLittleEndian(String name)
-    {
-        super(name);
-    }
 
     /**
      * test the getShort() method
      */
-
+    public void testGetShort() {
-    public void testGetShort()
-    {
         byte[] testdata = new byte[ LittleEndian.SHORT_SIZE + 1 ];
 
-        testdata[ 0 ] = 0x01;
+        testdata[0] = 0x01;
-        testdata[ 1 ] = ( byte ) 0xFF;
+        testdata[1] = (byte) 0xFF;
-        testdata[ 2 ] = 0x02;
+        testdata[2] = 0x02;
-        short expected[] = new short[ 2 ];
+        short expected[] = new short[2];
 
-        expected[ 0 ] = ( short ) 0xFF01;
+        expected[0] = ( short ) 0xFF01;
-        expected[ 1 ] = 0x02FF;
+        expected[1] = 0x02FF;
-        assertEquals(expected[ 0 ], LittleEndian.getShort(testdata));
+        assertEquals(expected[0], LittleEndian.getShort(testdata));
-        assertEquals(expected[ 1 ], LittleEndian.getShort(testdata, 1));
+        assertEquals(expected[1], LittleEndian.getShort(testdata, 1));
     }
 
-    public void testGetUShort()
+    public void testGetUShort() {
-    {
+        byte[] testdata = {
-        byte[] testdata = new byte[ LittleEndian.SHORT_SIZE + 1 ];
+            (byte) 0x01,
+            (byte) 0xFF,
+            (byte) 0x02,
+        };
+        byte[] testdata2 = {
+            (byte) 0x0D,
+            (byte) 0x93,
+            (byte) 0xFF,
+        };
 
-        testdata[ 0 ] = 0x01;
+        int expected0 = 0xFF01;
-        testdata[ 1 ] = ( byte ) 0xFF;
+        int expected1 = 0x02FF;
-        testdata[ 2 ] = 0x02;
+        int expected2 = 0x930D;
-
+        int expected3 = 0xFF93;
-        byte[] testdata2 = new byte[ LittleEndian.SHORT_SIZE + 1 ];
+        assertEquals(expected0, LittleEndian.getUShort(testdata));
-        
+        assertEquals(expected1, LittleEndian.getUShort(testdata, 1));
-        testdata2[ 0 ] = 0x0D;
+        assertEquals(expected2, LittleEndian.getUShort(testdata2));
-        testdata2[ 1 ] = ( byte )0x93;
+        assertEquals(expected3, LittleEndian.getUShort(testdata2, 1));
-        testdata2[ 2 ] = ( byte )0xFF;
-
-        int expected[] = new int[ 4 ];
-
-        expected[ 0 ] = 0xFF01;
-        expected[ 1 ] = 0x02FF;
-        expected[ 2 ] = 0x930D;
-        expected[ 3 ] = 0xFF93;
-        assertEquals(expected[ 0 ], LittleEndian.getUShort(testdata));
-        assertEquals(expected[ 1 ], LittleEndian.getUShort(testdata, 1));
-        assertEquals(expected[ 2 ], LittleEndian.getUShort(testdata2));
-        assertEquals(expected[ 3 ], LittleEndian.getUShort(testdata2, 1));
 
         byte[] testdata3 = new byte[ LittleEndian.SHORT_SIZE + 1 ];
-        LittleEndian.putShort(testdata3, 0, ( short ) expected[2] );
+        LittleEndian.putUShort(testdata3, 0, expected2);
-        LittleEndian.putShort(testdata3, 1, ( short ) expected[3] );
+        LittleEndian.putUShort(testdata3, 1, expected3);
-        assertEquals(testdata3[ 0 ], 0x0D);
+        assertEquals(testdata3[0], 0x0D);
-        assertEquals(testdata3[ 1 ], (byte)0x93);
+        assertEquals(testdata3[1], (byte)0x93);
-        assertEquals(testdata3[ 2 ], (byte)0xFF);
+        assertEquals(testdata3[2], (byte)0xFF);
-        assertEquals(expected[ 2 ], LittleEndian.getUShort(testdata3));
+        assertEquals(expected2, LittleEndian.getUShort(testdata3));
-        assertEquals(expected[ 3 ], LittleEndian.getUShort(testdata3, 1));
+        assertEquals(expected3, LittleEndian.getUShort(testdata3, 1));
-        //System.out.println("TD[1][0]: "+LittleEndian.getUShort(testdata)+" expecting 65281");
-        //System.out.println("TD[1][1]: "+LittleEndian.getUShort(testdata, 1)+" expecting 767");
-        //System.out.println("TD[2][0]: "+LittleEndian.getUShort(testdata2)+" expecting 37645");
-        //System.out.println("TD[2][1]: "+LittleEndian.getUShort(testdata2, 1)+" expecting 65427");
-        //System.out.println("TD[3][0]: "+LittleEndian.getUShort(testdata3)+" expecting 37645");
-        //System.out.println("TD[3][1]: "+LittleEndian.getUShort(testdata3, 1)+" expecting 65427");
         
     }
 
@@ -123,19 +96,15 @@ public class TestLittleEndian
     /**
      * test the getDouble() method
      */
-
+    public void testGetDouble() {
-    public void testGetDouble()
+        assertEquals(_doubles[0], LittleEndian.getDouble(_double_array), 0.000001 );
-    {
+        assertEquals(_doubles[1], LittleEndian.getDouble( _double_array, LittleEndian.DOUBLE_SIZE), 0.000001);
-        assertEquals(_doubles[ 0 ], LittleEndian.getDouble(_double_array), 0.000001 );
-        assertEquals(_doubles[ 1 ], LittleEndian.getDouble( _double_array, LittleEndian.DOUBLE_SIZE), 0.000001);
         assertTrue(Double.isNaN(LittleEndian.getDouble(_nan_double_array)));
 
         double nan = LittleEndian.getDouble(_nan_double_array);
         byte[] data = new byte[8];
         LittleEndian.putDouble(data, nan);
-        for ( int i = 0; i < data.length; i++ )
+        for ( int i = 0; i < data.length; i++ ) {
-        {
-            byte b = data[i];
             assertEquals(data[i], _nan_double_array[i]);
         }
     }
@@ -143,192 +112,154 @@ public class TestLittleEndian
     /**
      * test the getInt() method
      */
+    public void testGetInt() {
+        // reading 4 byte data from a 5 byte buffer
+        byte[] testdata = {
+                (byte) 0x01,
+                (byte) 0xFF,
+                (byte) 0xFF,
+                (byte) 0xFF,
+                (byte) 0x02,
+        };
 
-    public void testGetInt()
+        assertEquals(0xFFFFFF01, LittleEndian.getInt(testdata));
-    {
+        assertEquals(0x02FFFFFF, LittleEndian.getInt(testdata, 1));
-        byte[] testdata = new byte[ LittleEndian.INT_SIZE + 1 ];
-
-        testdata[ 0 ] = 0x01;
-        testdata[ 1 ] = ( byte ) 0xFF;
-        testdata[ 2 ] = ( byte ) 0xFF;
-        testdata[ 3 ] = ( byte ) 0xFF;
-        testdata[ 4 ] = 0x02;
-        int expected[] = new int[ 2 ];
-
-        expected[ 0 ] = 0xFFFFFF01;
-        expected[ 1 ] = 0x02FFFFFF;
-        assertEquals(expected[ 0 ], LittleEndian.getInt(testdata));
-        assertEquals(expected[ 1 ], LittleEndian.getInt(testdata, 1));
     }
 
     /**
      * test the getLong method
      */
+    public void testGetLong() {
 
-    public void testGetLong()
+        // reading 8 byte values from a 9 byte buffer
-    {
+        byte[] testdata = {
-        byte[] testdata = new byte[ LittleEndian.LONG_SIZE + 1 ];
+            (byte) 0x01,
+            (byte) 0xFF,
+            (byte) 0xFF,
+            (byte) 0xFF,
+            (byte) 0xFF,
+            (byte) 0xFF,
+            (byte) 0xFF,
+            (byte) 0xFF,
+            (byte) 0x02,
+        };
 
-        testdata[ 0 ] = 0x01;
+        assertEquals(0xFFFFFFFFFFFFFF01L, LittleEndian.getLong(testdata));
-        testdata[ 1 ] = ( byte ) 0xFF;
+        assertEquals(0x02FFFFFFFFFFFFFFL, LittleEndian.getLong(testdata, 1));
-        testdata[ 2 ] = ( byte ) 0xFF;
-        testdata[ 3 ] = ( byte ) 0xFF;
-        testdata[ 4 ] = ( byte ) 0xFF;
-        testdata[ 5 ] = ( byte ) 0xFF;
-        testdata[ 6 ] = ( byte ) 0xFF;
-        testdata[ 7 ] = ( byte ) 0xFF;
-        testdata[ 8 ] = 0x02;
-        long expected[] = new long[ 2 ];
-
-        expected[ 0 ] = 0xFFFFFFFFFFFFFF01L;
-        expected[ 1 ] = 0x02FFFFFFFFFFFFFFL;
-        assertEquals(expected[ 0 ], LittleEndian.getLong(testdata));
-        assertEquals(expected[ 1 ], LittleEndian.getLong(testdata, 1));
     }
 
     /**
      * test the PutShort method
      */
-
+    public void testPutShort() {
-    public void testPutShort()
-    {
         byte[] expected = new byte[ LittleEndian.SHORT_SIZE + 1 ];
 
-        expected[ 0 ] = 0x01;
+        expected[0] = 0x01;
-        expected[ 1 ] = ( byte ) 0xFF;
+        expected[1] = (byte) 0xFF;
-        expected[ 2 ] = 0x02;
+        expected[2] = 0x02;
         byte[] received   = new byte[ LittleEndian.SHORT_SIZE + 1 ];
-        short  testdata[] = new short[ 2 ];
+        short  testdata[] = new short[2];
 
-        testdata[ 0 ] = ( short ) 0xFF01;
+        testdata[0] = ( short ) 0xFF01;
-        testdata[ 1 ] = 0x02FF;
+        testdata[1] = 0x02FF;
-        LittleEndian.putShort(received, testdata[ 0 ]);
+        LittleEndian.putShort(received, testdata[0]);
-        assertTrue(ba_equivalent(received, expected, 0,
+        assertTrue(compareByteArrays(received, expected, 0, LittleEndian.SHORT_SIZE));
-                                 LittleEndian.SHORT_SIZE));
+        LittleEndian.putShort(received, 1, testdata[1]);
-        LittleEndian.putShort(received, 1, testdata[ 1 ]);
+        assertTrue(compareByteArrays(received, expected, 1, LittleEndian.SHORT_SIZE));
-        assertTrue(ba_equivalent(received, expected, 1,
-                                 LittleEndian.SHORT_SIZE));
     }
 
     /**
      * test the putInt method
      */
-
+    public void testPutInt() {
-    public void testPutInt()
+        // writing 4 byte data to a 5 byte buffer
-    {
+        byte[] expected = {
-        byte[] expected = new byte[ LittleEndian.INT_SIZE + 1 ];
+                (byte) 0x01,
-
+                (byte) 0xFF,
-        expected[ 0 ] = 0x01;
+                (byte) 0xFF,
-        expected[ 1 ] = ( byte ) 0xFF;
+                (byte) 0xFF,
-        expected[ 2 ] = ( byte ) 0xFF;
+                (byte) 0x02,
-        expected[ 3 ] = ( byte ) 0xFF;
+        };
-        expected[ 4 ] = 0x02;
         byte[] received = new byte[ LittleEndian.INT_SIZE + 1 ];
-        int    testdata[] = new int[ 2 ];
 
-        testdata[ 0 ] = 0xFFFFFF01;
+        LittleEndian.putInt(received, 0xFFFFFF01);
-        testdata[ 1 ] = 0x02FFFFFF;
+        assertTrue(compareByteArrays(received, expected, 0, LittleEndian.INT_SIZE));
-        LittleEndian.putInt(received, testdata[ 0 ]);
+        LittleEndian.putInt(received, 1, 0x02FFFFFF);
-        assertTrue(ba_equivalent(received, expected, 0,
+        assertTrue(compareByteArrays(received, expected, 1, LittleEndian.INT_SIZE));
-                                 LittleEndian.INT_SIZE));
-        LittleEndian.putInt(received, 1, testdata[ 1 ]);
-        assertTrue(ba_equivalent(received, expected, 1,
-                                 LittleEndian.INT_SIZE));
     }
 
     /**
      * test the putDouble methods
      */
-
+    public void testPutDouble() {
-    public void testPutDouble()
-    {
         byte[] received = new byte[ LittleEndian.DOUBLE_SIZE + 1 ];
 
-        LittleEndian.putDouble(received, _doubles[ 0 ]);
+        LittleEndian.putDouble(received, _doubles[0]);
-        assertTrue(ba_equivalent(received, _double_array, 0,
+        assertTrue(compareByteArrays(received, _double_array, 0, LittleEndian.DOUBLE_SIZE));
-                                 LittleEndian.DOUBLE_SIZE));
+        LittleEndian.putDouble(received, 1, _doubles[1]);
-        LittleEndian.putDouble(received, 1, _doubles[ 1 ]);
         byte[] expected = new byte[ LittleEndian.DOUBLE_SIZE + 1 ];
 
         System.arraycopy(_double_array, LittleEndian.DOUBLE_SIZE, expected,
                          1, LittleEndian.DOUBLE_SIZE);
-        assertTrue(ba_equivalent(received, expected, 1,
+        assertTrue(compareByteArrays(received, expected, 1, LittleEndian.DOUBLE_SIZE));
-                                 LittleEndian.DOUBLE_SIZE));
     }
 
     /**
      * test the putLong method
      */
-
+    public void testPutLong() {
-    public void testPutLong()
+        // writing 8 byte values to a 9 byte buffer
-    {
+        byte[] expected = {
-        byte[] expected = new byte[ LittleEndian.LONG_SIZE + 1 ];
+            (byte) 0x01,
-
+            (byte) 0xFF,
-        expected[ 0 ] = 0x01;
+            (byte) 0xFF,
-        expected[ 1 ] = ( byte ) 0xFF;
+            (byte) 0xFF,
-        expected[ 2 ] = ( byte ) 0xFF;
+            (byte) 0xFF,
-        expected[ 3 ] = ( byte ) 0xFF;
+            (byte) 0xFF,
-        expected[ 4 ] = ( byte ) 0xFF;
+            (byte) 0xFF,
-        expected[ 5 ] = ( byte ) 0xFF;
+            (byte) 0xFF,
-        expected[ 6 ] = ( byte ) 0xFF;
+            (byte) 0x02,
-        expected[ 7 ] = ( byte ) 0xFF;
+        };
-        expected[ 8 ] = 0x02;
         byte[] received   = new byte[ LittleEndian.LONG_SIZE + 1 ];
-        long   testdata[] = new long[ 2 ];
 
-        testdata[ 0 ] = 0xFFFFFFFFFFFFFF01L;
+        long testdata0 = 0xFFFFFFFFFFFFFF01L;
-        testdata[ 1 ] = 0x02FFFFFFFFFFFFFFL;
+        long testdata1 = 0x02FFFFFFFFFFFFFFL;
-        LittleEndian.putLong(received, testdata[ 0 ]);
+        LittleEndian.putLong(received, testdata0);
-        assertTrue(ba_equivalent(received, expected, 0,
+        assertTrue(compareByteArrays(received, expected, 0, LittleEndian.LONG_SIZE));
-                                 LittleEndian.LONG_SIZE));
+        LittleEndian.putLong(received, 1, testdata1);
-        LittleEndian.putLong(received, 1, testdata[ 1 ]);
+        assertTrue(compareByteArrays(received, expected, 1, LittleEndian.LONG_SIZE));
-        assertTrue(ba_equivalent(received, expected, 1,
-                                 LittleEndian.LONG_SIZE));
     }
 
-    private static byte[] _good_array =
+    private static byte[] _good_array = {
-    {
+        0x01, 0x02, 0x01, 0x02, 
-        0x01, 0x02, 0x01, 0x02, 0x01, 0x02, 0x01, 0x02, 0x01, 0x02, 0x01,
+        0x01, 0x02, 0x01, 0x02, 
-        0x02, 0x01, 0x02, 0x01, 0x02
+        0x01, 0x02, 0x01, 0x02,
+        0x01, 0x02, 0x01, 0x02,
     };
-    private static byte[] _bad_array  =
+    private static byte[] _bad_array  = {
-    {
         0x01
     };
 
     /**
      * test the readShort method
      */
-
+    public void testReadShort() throws IOException {
-    public void testReadShort()
-        throws IOException
-    {
         short       expected_value = 0x0201;
         InputStream stream         = new ByteArrayInputStream(_good_array);
         int         count          = 0;
 
-        while (true)
+        while (stream.available() > 0) {
-        {
             short value = LittleEndian.readShort(stream);
-
-            if (value == 0)
-            {
-                break;
-            }
             assertEquals(value, expected_value);
             count++;
         }
         assertEquals(count,
                      _good_array.length / LittleEndianConsts.SHORT_SIZE);
         stream = new ByteArrayInputStream(_bad_array);
-        try
+        try {
-        {
             LittleEndian.readShort(stream);
             fail("Should have caught BufferUnderrunException");
-        }
+        } catch (BufferUnderrunException ignored) {
-        catch (BufferUnderrunException ignored)
-        {
-
             // as expected
         }
     }
@@ -336,34 +267,22 @@ public class TestLittleEndian
     /**
      * test the readInt method
      */
-
+    public void testReadInt() throws IOException {
-    public void testReadInt()
-        throws IOException
-    {
         int         expected_value = 0x02010201;
         InputStream stream         = new ByteArrayInputStream(_good_array);
         int         count          = 0;
 
-        while (true)
+        while (stream.available() > 0) {
-        {
             int value = LittleEndian.readInt(stream);
-
-            if (value == 0)
-            {
-                break;
-            }
             assertEquals(value, expected_value);
             count++;
         }
         assertEquals(count, _good_array.length / LittleEndianConsts.INT_SIZE);
         stream = new ByteArrayInputStream(_bad_array);
-        try
+        try {
-        {
             LittleEndian.readInt(stream);
             fail("Should have caught BufferUnderrunException");
-        }
+        } catch (BufferUnderrunException ignored) {
-        catch (BufferUnderrunException ignored)
-        {
 
             // as expected
         }
@@ -372,104 +291,60 @@ public class TestLittleEndian
     /**
      * test the readLong method
      */
-
+    public void testReadLong() throws IOException {
-    public void testReadLong()
-        throws IOException
-    {
         long        expected_value = 0x0201020102010201L;
         InputStream stream         = new ByteArrayInputStream(_good_array);
         int         count          = 0;
 
-        while (true)
+        while (stream.available() > 0) {
-        {
             long value = LittleEndian.readLong(stream);
-
-            if (value == 0)
-            {
-                break;
-            }
             assertEquals(value, expected_value);
             count++;
         }
         assertEquals(count,
                      _good_array.length / LittleEndianConsts.LONG_SIZE);
         stream = new ByteArrayInputStream(_bad_array);
-        try
+        try {
-        {
             LittleEndian.readLong(stream);
             fail("Should have caught BufferUnderrunException");
-        }
+        } catch (BufferUnderrunException ignored) {
-        catch (BufferUnderrunException ignored)
-        {
-
             // as expected
         }
     }
 
-    /**
+//    public void testReadFromStream() throws IOException {
-     * test the readFromStream method
+//        int actual;
-     */
+//        actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[] { 5, -128, }));
+//        assertEquals(32773, actual);
+//        
+//        actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[] { 1, 2, 3, 4, }));
+//        assertEquals(513, actual);
+//
+//        try {
+//            LittleEndian.readInt(new ByteArrayInputStream(new byte[] { 1, 2, 3, }));
+//            fail("Should have caught BufferUnderrunException");
+//        } catch (BufferUnderrunException ignored) {
+//            // as expected
+//        }
+//    }
 
-    public void testReadFromStream()
+    public void testUnsignedByteToInt() {
-        throws IOException
-    {
-        InputStream stream = new ByteArrayInputStream(_good_array);
-        byte[]      value  = LittleEndian.readFromStream(stream,
-                                 _good_array.length);
-
-        assertTrue(ba_equivalent(value, _good_array, 0, _good_array.length));
-        stream = new ByteArrayInputStream(_good_array);
-        try
-        {
-            value = LittleEndian.readFromStream(stream,
-                                                _good_array.length + 1);
-            fail("Should have caught BufferUnderrunException");
-        }
-        catch (BufferUnderrunException ignored)
-        {
-
-            // as expected
-        }
-    }
-
-    public void testUnsignedByteToInt()
-            throws Exception
-    {
         assertEquals(255, LittleEndian.ubyteToInt((byte)255));
     }
 
-    private boolean ba_equivalent(byte [] received, byte [] expected,
+    private static boolean compareByteArrays(byte [] received, byte [] expected,
-                                  int offset, int size)
+                                  int offset, int size) {
-    {
-        boolean result = true;
 
-        for (int j = offset; j < offset + size; j++)
+        for (int j = offset; j < offset + size; j++) {
-        {
+            if (received[j] != expected[j]) {
-            if (received[ j ] != expected[ j ])
-            {
                 System.out.println("difference at index " + j);
-                result = false;
+                return false;
-                break;
             }
         }
-        return result;
+        return true;
     }
 
-    public void testUnsignedShort()
+    public void testUnsignedShort() {
-            throws Exception
-    {
         assertEquals(0xffff, LittleEndian.getUShort(new byte[] { (byte)0xff, (byte)0xff }, 0));
     }
-
-    /**
-     * main method to run the unit tests
-     *
-     * @param ignored_args
-     */
-
-    public static void main(String [] ignored_args)
-    {
-        System.out.println("Testing util.LittleEndian functionality");
-        junit.textui.TestRunner.run(TestLittleEndian.class);
-    }
 }

src/testcases/org/apache/poi/util/TestLongField.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,7 +15,6 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.util;
 
 import junit.framework.*;
@@ -28,31 +26,13 @@ import java.io.*;
  *
  * @author  Marc Johnson (mjohnson at apache dot org)
  */
-
+public final class TestLongField extends TestCase {
-public class TestLongField
-    extends TestCase
-{
-
-    /**
-     * Constructor
-     *
-     * @param name
-     */
-
-    public TestLongField(String name)
-    {
-        super(name);
-    }
 
     static private final long[] _test_array =
     {
         Long.MIN_VALUE, -1L, 0L, 1L, Long.MAX_VALUE
     };
 
-    /**
-     * Test constructors.
-     */
-
     public void testConstructors()
     {
         try
@@ -121,10 +101,6 @@ public class TestLongField
         }
     }
 
-    /**
-     * Test set() methods
-     */
-
     public void testSet()
     {
         LongField field = new LongField(0);
@@ -163,10 +139,6 @@ public class TestLongField
         }
     }
 
-    /**
-     * Test readFromBytes
-     */
-
     public void testReadFromBytes()
     {
         LongField field = new LongField(1);
@@ -198,12 +170,6 @@ public class TestLongField
         }
     }
 
-    /**
-     * Test readFromStream
-     *
-     * @exception IOException
-     */
-
     public void testReadFromStream()
         throws IOException
     {
@@ -212,7 +178,7 @@ public class TestLongField
 
         for (int j = 0; j < _test_array.length; j++)
         {
-            buffer[ (j * 8) + 0 ] = ( byte ) (_test_array[ j ] % 256);
+            buffer[ (j * 8) + 0 ] = ( byte ) ((_test_array[ j ] >>  0) % 256);
             buffer[ (j * 8) + 1 ] = ( byte ) ((_test_array[ j ] >>  8) % 256);
             buffer[ (j * 8) + 2 ] = ( byte ) ((_test_array[ j ] >> 16) % 256);
             buffer[ (j * 8) + 3 ] = ( byte ) ((_test_array[ j ] >> 24) % 256);
@@ -230,10 +196,6 @@ public class TestLongField
         }
     }
 
-    /**
-     * test writeToBytes
-     */
-
     public void testWriteToBytes()
     {
         LongField field = new LongField(0);
@@ -256,16 +218,4 @@ public class TestLongField
             assertEquals("testing ", _test_array[ j ], val);
         }
     }
-
-    /**
-     * Main
-     *
-     * @param args
-     */
-
-    public static void main(String [] args)
-    {
-        System.out.println("Testing util.LongField functionality");
-        junit.textui.TestRunner.run(TestLongField.class);
-    }
 }