Bugzilla 47250 - Fixed FontRecord to expect unicode flags even when name length is zero

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@777717 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.5-beta6" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="add">47250 - Fixed FontRecord to expect unicode flags even when name length is zero</action>
            <action dev="POI-DEVELOPERS" type="add">47198 - Fixed formula evaluator comparison of -0.0 and 0.0</action>
            <action dev="POI-DEVELOPERS" type="add">47229 - Fixed ExternalNameRecord to handle DDE links</action>
            <action dev="POI-DEVELOPERS" type="add">46287 - Control of header and footer extraction in ExcelExtractor / XSSFExcelExtractor</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta6" date="2009-??-??">
+           <action dev="POI-DEVELOPERS" type="add">47250 - Fixed FontRecord to expect unicode flags even when name length is zero</action>
            <action dev="POI-DEVELOPERS" type="add">47198 - Fixed formula evaluator comparison of -0.0 and 0.0</action>
            <action dev="POI-DEVELOPERS" type="add">47229 - Fixed ExternalNameRecord to handle DDE links</action>
            <action dev="POI-DEVELOPERS" type="add">46287 - Control of header and footer extraction in ExcelExtractor / XSSFExcelExtractor</action>

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

@@ -19,6 +19,7 @@ package org.apache.poi.hssf.record;
 
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
+import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndianOutput;
 import org.apache.poi.util.StringUtil;
 
@@ -60,7 +61,7 @@ public final class FontRecord extends StandardRecord {
 	private byte                  field_8_charset;            // ?? defined by windows api logfont structure?
 	private byte                  field_9_zero = 0;           // must be 0
 	/** possibly empty string never <code>null</code> */
-    private String                field_11_font_name;         // whoa...the font name
+	private String                field_11_font_name;
 
 	public FontRecord() {
 	}
@@ -76,8 +77,10 @@ public final class FontRecord extends StandardRecord {
 		field_8_charset             = in.readByte();
 		field_9_zero                = in.readByte();
 		int field_10_font_name_len  = in.readUByte();
+		int unicodeFlags            = in.readUByte(); // options byte present always (even if no character data)
+
 		if (field_10_font_name_len > 0) {
-            if (in.readByte() == 0) {   // is compressed unicode
+			if (unicodeFlags == 0) {   // is compressed unicode
 				field_11_font_name = in.readCompressedUnicode(field_10_font_name_len);
 			} else {   // is not compressed unicode
 				field_11_font_name = in.readUnicodeLEString(field_10_font_name_len);
@@ -92,9 +95,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @param height  fontheight (in points/20)
 	 */
-
+	public void setFontHeight(short height) {
-    public void setFontHeight(short height)
-    {
 		field_1_font_height = height;
 	}
 
@@ -103,9 +104,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @param attributes    the bitmask to set
 	 */
-
+	public void setAttributes(short attributes) {
-    public void setAttributes(short attributes)
-    {
 		field_2_attributes = attributes;
 	}
 
@@ -117,9 +116,7 @@ public final class FontRecord extends StandardRecord {
 	 * @param italics - whether the font is italics or not
 	 * @see #setAttributes(short)
 	 */
-
+	public void setItalic(boolean italics) {
-    public void setItalic(boolean italics)
-    {
 		field_2_attributes = italic.setShortBoolean(field_2_attributes, italics);
 	}
 
@@ -129,9 +126,7 @@ public final class FontRecord extends StandardRecord {
 	 * @param strike - whether the font is stricken out or not
 	 * @see #setAttributes(short)
 	 */
-
+	public void setStrikeout(boolean strike) {
-    public void setStrikeout(boolean strike)
-    {
 		field_2_attributes = strikeout.setShortBoolean(field_2_attributes, strike);
 	}
 
@@ -142,9 +137,7 @@ public final class FontRecord extends StandardRecord {
 	 * @param mac - whether to do that mac font outline thing or not
 	 * @see #setAttributes(short)
 	 */
-
+	public void setMacoutline(boolean mac) {
-    public void setMacoutline(boolean mac)
-    {
 		field_2_attributes = macoutline.setShortBoolean(field_2_attributes, mac);
 	}
 
@@ -155,9 +148,7 @@ public final class FontRecord extends StandardRecord {
 	 * @param mac - whether to do that mac font shadow thing or not
 	 * @see #setAttributes(short)
 	 */
-
+	public void setMacshadow(boolean mac) {
-    public void setMacshadow(boolean mac)
-    {
 		field_2_attributes = macshadow.setShortBoolean(field_2_attributes, mac);
 	}
 
@@ -166,9 +157,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @param cpi - font color index
 	 */
-
+	public void setColorPaletteIndex(short cpi) {
-    public void setColorPaletteIndex(short cpi)
-    {
 		field_3_color_palette_index = cpi;
 	}
 
@@ -178,9 +167,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @param bw - a number between 100-1000 for the fonts "boldness"
 	 */
-
+	public void setBoldWeight(short bw) {
-    public void setBoldWeight(short bw)
-    {
 		field_4_bold_weight = bw;
 	}
 
@@ -192,9 +179,7 @@ public final class FontRecord extends StandardRecord {
 	 * @see #SS_SUPER
 	 * @see #SS_SUB
 	 */
-
+	public void setSuperSubScript(short sss) {
-    public void setSuperSubScript(short sss)
-    {
 		field_5_super_sub_script = sss;
 	}
 
@@ -209,9 +194,7 @@ public final class FontRecord extends StandardRecord {
 	 * @see #U_SINGLE_ACCOUNTING
 	 * @see #U_DOUBLE_ACCOUNTING
 	 */
-
+	public void setUnderline(byte u) {
-    public void setUnderline(byte u)
-    {
 		field_6_underline = u;
 	}
 
@@ -220,20 +203,16 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @param f family
 	 */
-
+	public void setFamily(byte f) {
-    public void setFamily(byte f)
-    {
 		field_7_family = f;
 	}
 
 	/**
 	 * set the character set
 	 *
-     * @param charset - characterset
+	 * @param charset - character set
 	 */
-
+	public void setCharset(byte charset) {
-    public void setCharset(byte charset)
-    {
 		field_8_charset = charset;
 	}
 
@@ -243,9 +222,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @param fn - name of the font (i.e. "Arial")
 	 */
-
+	public void setFontName(String fn) {
-    public void setFontName(String fn)
-    {
 		field_11_font_name = fn;
 	}
 
@@ -254,9 +231,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @return fontheight (in points/20)
 	 */
-
+	public short getFontHeight() {
-    public short getFontHeight()
-    {
 		return field_1_font_height;
 	}
 
@@ -265,9 +240,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @return attribute - the bitmask
 	 */
-
+	public short getAttributes() {
-    public short getAttributes()
-    {
 		return field_2_attributes;
 	}
 
@@ -277,9 +250,7 @@ public final class FontRecord extends StandardRecord {
 	 * @return italics - whether the font is italics or not
 	 * @see #getAttributes()
 	 */
-
+	public boolean isItalic() {
-    public boolean isItalic()
-    {
 		return italic.isSet(field_2_attributes);
 	}
 
@@ -289,9 +260,7 @@ public final class FontRecord extends StandardRecord {
 	 * @return strike - whether the font is stricken out or not
 	 * @see #getAttributes()
 	 */
-
+	public boolean isStruckout(){
-    public boolean isStruckout()
-    {
 		return strikeout.isSet(field_2_attributes);
 	}
 
@@ -302,9 +271,7 @@ public final class FontRecord extends StandardRecord {
 	 * @return mac - whether to do that mac font outline thing or not
 	 * @see #getAttributes()
 	 */
-
+	public boolean isMacoutlined(){
-    public boolean isMacoutlined()
-    {
 		return macoutline.isSet(field_2_attributes);
 	}
 
@@ -315,9 +282,7 @@ public final class FontRecord extends StandardRecord {
 	 * @return mac - whether to do that mac font shadow thing or not
 	 * @see #getAttributes()
 	 */
-
+	public boolean isMacshadowed(){
-    public boolean isMacshadowed()
-    {
 		return macshadow.isSet(field_2_attributes);
 	}
 
@@ -326,9 +291,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @return cpi - font color index
 	 */
-
+	public short getColorPaletteIndex(){
-    public short getColorPaletteIndex()
-    {
 		return field_3_color_palette_index;
 	}
 
@@ -338,9 +301,7 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @return bw - a number between 100-1000 for the fonts "boldness"
 	 */
-
+	public short getBoldWeight(){
-    public short getBoldWeight()
-    {
 		return field_4_bold_weight;
 	}
 
@@ -352,9 +313,7 @@ public final class FontRecord extends StandardRecord {
 	 * @see #SS_SUPER
 	 * @see #SS_SUB
 	 */
-
+	public short getSuperSubScript(){
-    public short getSuperSubScript()
-    {
 		return field_5_super_sub_script;
 	}
 
@@ -369,9 +328,7 @@ public final class FontRecord extends StandardRecord {
 	 * @see #U_SINGLE_ACCOUNTING
 	 * @see #U_DOUBLE_ACCOUNTING
 	 */
-
+	public byte getUnderline() {
-    public byte getUnderline()
-    {
 		return field_6_underline;
 	}
 
@@ -380,20 +337,16 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @return family
 	 */
-
+	public byte getFamily() {
-    public byte getFamily()
-    {
 		return field_7_family;
 	}
 
 	/**
 	 * get the character set
 	 *
-     * @return charset - characterset
+	 * @return charset - character set
 	 */
-
+	public byte getCharset() {
-    public byte getCharset()
-    {
 		return field_8_charset;
 	}
 
@@ -402,45 +355,29 @@ public final class FontRecord extends StandardRecord {
 	 *
 	 * @return fn - name of the font (i.e. "Arial")
 	 */
-
+	public String getFontName() {
-    public String getFontName()
-    {
 		return field_11_font_name;
 	}
 
-    public String toString()
+	public String toString() {
-    {
+		StringBuffer sb = new StringBuffer();
-        StringBuffer buffer = new StringBuffer();
 
-        buffer.append("[FONT]\n");
+		sb.append("[FONT]\n");
-        buffer.append("    .fontheight      = ")
+		sb.append("    .fontheight    = ").append(HexDump.shortToHex(getFontHeight())).append("\n");
-            .append(Integer.toHexString(getFontHeight())).append("\n");
+		sb.append("    .attributes    = ").append(HexDump.shortToHex(getAttributes())).append("\n");
-        buffer.append("    .attributes      = ")
+		sb.append("       .italic     = ").append(isItalic()).append("\n");
-            .append(Integer.toHexString(getAttributes())).append("\n");
+		sb.append("       .strikout   = ").append(isStruckout()).append("\n");
-        buffer.append("         .italic     = ").append(isItalic())
+		sb.append("       .macoutlined= ").append(isMacoutlined()).append("\n");
-            .append("\n");
+		sb.append("       .macshadowed= ").append(isMacshadowed()).append("\n");
-        buffer.append("         .strikout   = ").append(isStruckout())
+		sb.append("    .colorpalette  = ").append(HexDump.shortToHex(getColorPaletteIndex())).append("\n");
-            .append("\n");
+		sb.append("    .boldweight    = ").append(HexDump.shortToHex(getBoldWeight())).append("\n");
-        buffer.append("         .macoutlined= ").append(isMacoutlined())
+		sb.append("    .supersubscript= ").append(HexDump.shortToHex(getSuperSubScript())).append("\n");
-            .append("\n");
+		sb.append("    .underline     = ").append(HexDump.byteToHex(getUnderline())).append("\n");
-        buffer.append("         .macshadowed= ").append(isMacshadowed())
+		sb.append("    .family        = ").append(HexDump.byteToHex(getFamily())).append("\n");
-            .append("\n");
+		sb.append("    .charset       = ").append(HexDump.byteToHex(getCharset())).append("\n");
-        buffer.append("    .colorpalette    = ")
+		sb.append("    .fontname      = ").append(getFontName()).append("\n");
-            .append(Integer.toHexString(getColorPaletteIndex())).append("\n");
+		sb.append("[/FONT]\n");
-        buffer.append("    .boldweight      = ")
+		return sb.toString();
-            .append(Integer.toHexString(getBoldWeight())).append("\n");
-        buffer.append("    .supersubscript  = ")
-            .append(Integer.toHexString(getSuperSubScript())).append("\n");
-        buffer.append("    .underline       = ")
-            .append(Integer.toHexString(getUnderline())).append("\n");
-        buffer.append("    .family          = ")
-            .append(Integer.toHexString(getFamily())).append("\n");
-        buffer.append("    .charset         = ")
-            .append(Integer.toHexString(getCharset())).append("\n");
-        buffer.append("    .fontname        = ").append(getFontName())
-            .append("\n");
-        buffer.append("[/FONT]\n");
-        return buffer.toString();
 	}
 
 	public void serialize(LittleEndianOutput out) {
@@ -456,9 +393,9 @@ public final class FontRecord extends StandardRecord {
 		out.writeByte(field_9_zero);
 		int fontNameLen = field_11_font_name.length();
 		out.writeByte(fontNameLen);
-		if (fontNameLen > 0) {
 		boolean hasMultibyte = StringUtil.hasMultibyte(field_11_font_name);
 		out.writeByte(hasMultibyte ? 0x01 : 0x00);
+		if (fontNameLen > 0) {
 			if (hasMultibyte) {
 			   StringUtil.putUnicodeLE(field_11_font_name, out);
 			} else {
@@ -467,20 +404,17 @@ public final class FontRecord extends StandardRecord {
 		}
 	}
 	protected int getDataSize() {
-    	int size = 15; // 5 shorts + 5 bytes
+		int size = 16; // 5 shorts + 6 bytes
 		int fontNameLen = field_11_font_name.length();
 		if (fontNameLen < 1) {
-    		// options byte is not encoded if no character data
 			return size;
 		}
-    	size ++; // options byte
 
 		boolean hasMultibyte = StringUtil.hasMultibyte(field_11_font_name);
 		return size + fontNameLen * (hasMultibyte ? 2 : 1);
 	}
 
-    public short getSid()
+	public short getSid() {
-    {
 		return sid;
 	}
 
@@ -544,17 +478,4 @@ public final class FontRecord extends StandardRecord {
 		field_11_font_name.equals(other.field_11_font_name)
 		;
 	}
-
-	/**
-	 * Only returns two for the same exact object -
-	 *  creating a second FontRecord with the same
-	 *  properties won't be considered equal, as 
-	 *  the record's position in the record stream
-	 *  matters.
-	 */
-	public boolean equals(Object obj) {
-		if (this == obj)
-			return true;
-		return false;
-	}
 }

src/testcases/org/apache/poi/hssf/record/TestFontRecord.java

@@ -18,14 +18,18 @@
 package org.apache.poi.hssf.record;
 
 
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
+import org.apache.poi.util.HexRead;
+
 /**
  * Tests the serialization and deserialization of the {@link FontRecord}
  * class works correctly.  Test data taken directly from a real Excel file.
  */
 public final class TestFontRecord extends TestCase {
 
+    private static final int SID = 0x31;
     private static final byte[] data = {
             0xC8-256, 00,       // font height = xc8
             00, 00,             // attrs = 0
@@ -87,9 +91,7 @@ public final class TestFontRecord extends TestCase {
         record.setFontName("Arial");
 
         byte [] recordBytes = record.serialize();
-        assertEquals(recordBytes.length - 4, data.length);
+        TestcaseRecordInputStream.confirmRecordEncoding(0x31, data, recordBytes);
-        for (int i = 0; i < data.length; i++)
-            assertEquals("At offset " + i, data[i], recordBytes[i+4]);
     }
 
     public void testCloneOnto() {
@@ -104,7 +106,7 @@ public final class TestFontRecord extends TestCase {
             assertEquals("At offset " + i, data[i], recordBytes[i+4]);
     }
 
-    public void testSameProperties() throws Exception {
+    public void testSameProperties() {
         FontRecord f1 = new FontRecord(TestcaseRecordInputStream.create(0x31, data));
         FontRecord f2 = new FontRecord(TestcaseRecordInputStream.create(0x31, data));
 
@@ -120,4 +122,28 @@ public final class TestFontRecord extends TestCase {
         f2.setFontHeight((short)0xc8);
         assertTrue(f1.sameProperties(f2));
     }
+
+    /**
+     * Bugzilla 47250 suggests that the unicode options byte should be present even when the name
+     * length is zero.  The OOO documentation seems to agree with this and POI had no test data
+     * samples to say otherwise.
+     */
+    public void testEmptyName_bug47250() {
+        byte[] emptyNameData = HexRead.readFromString(
+                "C8 00 00 00 FF 7F 90 01 00 00 00 00 00 00 "
+                + "00" // zero length
+                + "00" // unicode options byte
+                );
+
+        RecordInputStream in = TestcaseRecordInputStream.create(SID, emptyNameData);
+        FontRecord fr = new FontRecord(in);
+        if (in.available() == 1) {
+            throw new AssertionFailedError("Identified bug 47250");
+        }
+        assertEquals(0, in.available());
+
+        assertEquals(0, fr.getFontName().length());
+        byte[] recordBytes = fr.serialize();
+        TestcaseRecordInputStream.confirmRecordEncoding(SID, emptyNameData, recordBytes);
+    }
 }