From 47ea1aa486645dcd0325fd50bfd6e69103cf0e9a Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Tue, 20 May 2014 14:01:22 +0000 Subject: [PATCH] Bug 53691: Fix a copy/paste error in CFRuleRecord.clone() also make CFRuleRecord.toString() print out more information which caused the bug to be much harder to find Add unit tests to verify/reproduce this git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1596251 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/record/CFRuleRecord.java | 37 ++++----- .../poi/hssf/record/TestCFRuleRecord.java | 20 ++++- .../TestHSSFConditionalFormatting.java | 72 ++++++++++++++++++ test-data/spreadsheet/53691.xls | Bin 0 -> 24576 bytes 4 files changed, 109 insertions(+), 20 deletions(-) create mode 100644 test-data/spreadsheet/53691.xls diff --git a/src/java/org/apache/poi/hssf/record/CFRuleRecord.java b/src/java/org/apache/poi/hssf/record/CFRuleRecord.java index 867ecec63..4cf83efce 100644 --- a/src/java/org/apache/poi/hssf/record/CFRuleRecord.java +++ b/src/java/org/apache/poi/hssf/record/CFRuleRecord.java @@ -17,14 +17,16 @@ package org.apache.poi.hssf.record; +import java.util.Arrays; + import org.apache.poi.hssf.model.HSSFFormulaParser; import org.apache.poi.hssf.record.cf.BorderFormatting; import org.apache.poi.hssf.record.cf.FontFormatting; import org.apache.poi.hssf.record.cf.PatternFormatting; -import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.ss.formula.Formula; import org.apache.poi.ss.formula.FormulaType; +import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.util.BitField; import org.apache.poi.util.BitFieldFactory; import org.apache.poi.util.LittleEndianOutput; @@ -460,12 +462,13 @@ public final class CFRuleRecord extends StandardRecord { } protected int getDataSize() { - return 12 + + int i = 12 + (containsFontFormattingBlock()?_fontFormatting.getRawRecord().length:0)+ (containsBorderFormattingBlock()?8:0)+ (containsPatternFormattingBlock()?4:0)+ getFormulaSize(field_17_formula1)+ - getFormulaSize(field_18_formula2) + getFormulaSize(field_18_formula2); + return i ; } @@ -473,20 +476,20 @@ public final class CFRuleRecord extends StandardRecord { public String toString() { StringBuffer buffer = new StringBuffer(); buffer.append("[CFRULE]\n"); - buffer.append(" .condition_type ="+field_1_condition_type); - buffer.append(" OPTION FLAGS=0x"+Integer.toHexString(getOptions())); - if (false) { - if (containsFontFormattingBlock()) { - buffer.append(_fontFormatting.toString()); - } - if (containsBorderFormattingBlock()) { - buffer.append(_borderFormatting.toString()); - } - if (containsPatternFormattingBlock()) { - buffer.append(_patternFormatting.toString()); - } - buffer.append("[/CFRULE]\n"); + buffer.append(" .condition_type =").append(field_1_condition_type).append("\n"); + buffer.append(" OPTION FLAGS=0x").append(Integer.toHexString(getOptions())).append("\n"); + if (containsFontFormattingBlock()) { + buffer.append(_fontFormatting.toString()).append("\n"); } + if (containsBorderFormattingBlock()) { + buffer.append(_borderFormatting.toString()).append("\n"); + } + if (containsPatternFormattingBlock()) { + buffer.append(_patternFormatting.toString()).append("\n"); + } + buffer.append(" Formula 1 =").append(Arrays.toString(field_17_formula1.getTokens())).append("\n"); + buffer.append(" Formula 2 =").append(Arrays.toString(field_18_formula2.getTokens())).append("\n"); + buffer.append("[/CFRULE]\n"); return buffer.toString(); } @@ -504,7 +507,7 @@ public final class CFRuleRecord extends StandardRecord { rec._patternFormatting = (PatternFormatting) _patternFormatting.clone(); } rec.field_17_formula1 = field_17_formula1.copy(); - rec.field_18_formula2 = field_17_formula1.copy(); + rec.field_18_formula2 = field_18_formula2.copy(); return rec; } diff --git a/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java b/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java index d7f43bd2a..c5da21875 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.record; +import static org.junit.Assert.assertArrayEquals; import junit.framework.AssertionFailedError; import junit.framework.TestCase; @@ -24,12 +25,12 @@ import org.apache.poi.hssf.record.CFRuleRecord.ComparisonOperator; import org.apache.poi.hssf.record.cf.BorderFormatting; import org.apache.poi.hssf.record.cf.FontFormatting; import org.apache.poi.hssf.record.cf.PatternFormatting; -import org.apache.poi.ss.formula.ptg.Ptg; -import org.apache.poi.ss.formula.ptg.RefNPtg; -import org.apache.poi.ss.formula.ptg.RefPtg; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.util.HSSFColor; +import org.apache.poi.ss.formula.ptg.Ptg; +import org.apache.poi.ss.formula.ptg.RefNPtg; +import org.apache.poi.ss.formula.ptg.RefPtg; import org.apache.poi.util.LittleEndian; /** @@ -351,4 +352,17 @@ public final class TestCFRuleRecord extends TestCase { byte[] data = rr.serialize(); TestcaseRecordInputStream.confirmRecordEncoding(CFRuleRecord.sid, DATA_REFN, data); } + + public void testBug53691() { + HSSFWorkbook workbook = new HSSFWorkbook(); + HSSFSheet sheet = workbook.createSheet(); + + CFRuleRecord record = CFRuleRecord.create(sheet, ComparisonOperator.BETWEEN, "2", "5"); + + CFRuleRecord clone = (CFRuleRecord) record.clone(); + + byte [] serializedRecord = record.serialize(); + byte [] serializedClone = clone.serialize(); + assertArrayEquals(serializedRecord, serializedClone); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java index 943ebd004..8cd3cbaac 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java @@ -18,8 +18,14 @@ package org.apache.poi.hssf.usermodel; +import java.io.FileNotFoundException; +import java.io.IOException; + import org.apache.poi.hssf.HSSFITestDataProvider; import org.apache.poi.ss.usermodel.BaseTestConditionalFormatting; +import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.ss.usermodel.SheetConditionalFormatting; +import org.apache.poi.ss.usermodel.Workbook; /** * @@ -33,4 +39,70 @@ public final class TestHSSFConditionalFormatting extends BaseTestConditionalForm public void testRead(){ testRead("WithConditionalFormatting.xls"); } + + public void test53691() throws IOException { + SheetConditionalFormatting cf; + final Workbook wb; + wb = HSSFITestDataProvider.instance.openSampleWorkbook("53691.xls"); + /* + FileInputStream s = new FileInputStream("C:\\temp\\53691bbadfixed.xls"); + try { + wb = new HSSFWorkbook(s); + } finally { + s.close(); + } + + wb.removeSheetAt(1);*/ + + // initially it is good + writeTemp53691(wb, "agood"); + + // clone sheet corrupts it + Sheet sheet = wb.cloneSheet(0); + writeTemp53691(wb, "bbad"); + + // removing the sheet makes it good again + wb.removeSheetAt(wb.getSheetIndex(sheet)); + writeTemp53691(wb, "cgood"); + + // cloning again and removing the conditional formatting makes it good again + sheet = wb.cloneSheet(0); + removeConditionalFormatting(sheet); + writeTemp53691(wb, "dgood"); + + // cloning the conditional formatting manually makes it bad again + cf = sheet.getSheetConditionalFormatting(); + SheetConditionalFormatting scf = wb.getSheetAt(0).getSheetConditionalFormatting(); + for (int j = 0; j < scf.getNumConditionalFormattings(); j++) { + cf.addConditionalFormatting(scf.getConditionalFormattingAt(j)); + } + writeTemp53691(wb, "ebad"); + + // remove all conditional formatting for comparing BIFF output + removeConditionalFormatting(sheet); + removeConditionalFormatting(wb.getSheetAt(0)); + writeTemp53691(wb, "fgood"); + } + + private void removeConditionalFormatting(Sheet sheet) { + SheetConditionalFormatting cf = sheet.getSheetConditionalFormatting(); + for (int j = 0; j < cf.getNumConditionalFormattings(); j++) { + cf.removeConditionalFormatting(j); + } + } + + private void writeTemp53691(Workbook wb, String suffix) throws FileNotFoundException, + IOException { + // assert that we can write/read it in memory + Workbook wbBack = HSSFITestDataProvider.instance.writeOutAndReadBack(wb); + assertNotNull(wbBack); + + /* Just necessary for local testing... */ + /*OutputStream out = new FileOutputStream("C:\\temp\\53691" + suffix + ".xls"); + try { + wb.write(out); + } finally { + out.close(); + }*/ + } } diff --git a/test-data/spreadsheet/53691.xls b/test-data/spreadsheet/53691.xls new file mode 100644 index 0000000000000000000000000000000000000000..5c20dc54e400319191ffdf595f4b25286919d3c7 GIT binary patch literal 24576 zcmeHP3vgUj8UF9x-Q=dvq>r>fn{Lx5Z3=A?N_n(tfr6EkcG?0}WN6adO*0Lfy4f&L z><|=YR1hJ>0&RWEI12Jsa72MoVH^WGDucz20#g_r86E1|DX!ml?%v(|&aN+`ig7}lf)27gs_c_{e|&LXcNd_eXHY`-P_Z#>RZGIVK_;c0-c z#+@dIL2Zzelw?!Xd8#@;sM0;D&Z}jLyrNw!A6dI*)+*!{|n zS9xAmloiq>N0Hu;+v)1M9IVyOmoDj-KIxVm&i_d%c4?Zdk|(L4lB8D`bifHR=Qa2`#abxh% zWyTmOG? zBszu7qZ#R&9s0EL^SoAgZc6T!Q>85o^ePy^%0e^vAQD-D>d4(xH3w{8;fx^0RK-68&o-^sk1{9|)lz3ZXw3Lcc$R{^=0Z_`s~WR*VuTJ$%u{BWW#z2y&VAyoRGVMr%y=3rc@ z_)oRze{}O1XVXjevi);uAnpWT`ui0BQu}kbp`menBji6=q}K)V8Smp`)fdjhl>8n! zYi;_^WHW9{qAmi6f;AY4=39DoUOz4K;OjzG5U_#dnXTrw0 z0w#3Va3<3#V?q}WXTol`0!irB;Y=DUVKUkoZJimFF^RU$%*vQVTj!+8m_%C#9m?2> z9qo*^&MB2KiMGxx7ZXI>eTh|fTHB(nGrKY-(bj3Ij7hY0FcNSm7LnlML|Z3~TxGW> z4_E(c>m=Gbb1P#KZJp*yn21DMXI^DYqOCK(GA7a1Sx_01XzMJjj7hY07FEW?ZJj$j z{mx=Asa5?B$4ea@9R;7C@WJup#|w7zSc|^MPRtb;PUj5=&024`3d>yObZb?rwKC2@ z16y_MGrU5b3VK&C(zZ6uR;^z>e2HYKhrXdy=^oH~2P=Fd zWwlVufVo|;dY=9L3nlgVQBOzCNTYaBm%C68n<1!$-yU|LdM&#GOf3JTUVM`mHm)pF zp)guvVpp0+9(ly)pQL&HlQge?k_NWw@T;N~^r}?Rh74r`9G0430~DT%GC;cvmFjS| zADVxRdPWJAiM<(gr?c4&j-ScON|+QnoL=l_a%Lq=yu$`>tH3u^Wv%;)<3~NKgi06j zwQY7oS{T+!zT>y`*)CL{1Nt+ZQ*DNGOBs|W!d~%onWtHH!ODfN%PhT(!6y-^;il`vr!ov-uSx#wHnYzC%a`Y-lb$Ek2iHh z(8xV$W{$q`RzV>tV~9f1iPYdjYV{*EM* zn(jkd=|^gfM6y~Xg;Zx|-u}&zV5CMOwU6O@bAZ%dk?)OmJ~4VM7-@zNDZuyUMIt$U zZ?yA`Cms(*n(0Fd@V!NmNKW4y?fm(%Cxekr@*xHI-qJ`Ur|*q+jz0ZzFw)6BqyXPL zJrc?3d!wC?|MJ(tNT>La0(`G663OX%qn#&Sd@LAg7Litt;d^Nyt*prRTH8ML=t~w7 z>NB^`u!U9a-fSOIfbY$ZL~{CGYun2Q?hHn1@*xHI-r`6kr|-44eeb11!ANs_NCCdr z5{cyWz1FseKlfZPQrd?U;Css>k(|EQ+BWk1(O{&xKBNHOYmY>7`d(|>-=F$q5Yhy^ z4#%Xp)Y-1{bhfh#wV~as$fg^=9n5rInJN6s%XF@XsnetIK4kO8V5alSOyNpirVBhw zoz5e2=v(g#X1buv6kg+Hda8%1(@#W(d%h6NbYYn(9K_3XnTM&L5#u_y_4VnB>5vRcFJ^+>7-DfW$*weoDGyg4 zIJn|H2YjlLse$9#1PzIuF~b$BbH(=Slko-;Zzaw%bLP%n*`aLjh;f5Qp2osBIS*3g z5O&Hg4ETl+dPAk+INl`{C%|Q?te7p86|<$XV*4>dUNT%$7%^JhqNHjHCUk;&0C8J9BaO%NIO=8LQltV9Is5) z=XUoW%bD38Gnd|GUYj2=E^_1PXB6rc-frv$Jf_T<#ZD+1HZ9w5y%r;RdzL_NqBV+r zn1Qz}m_l4M1h!|n*=i>j5^@oIf|;#hEHiVh!d501G>VLpa{6!rmQA~vq!LJcu?)bf zv?~u*b5<2rFzE&47-|f@uBR{R?K#2D`q>i6{AD94yRD;`EI;a3Psoq z;Ri}nDXZucvG)qTvvudLe$-1S{d{*u?G*UVR#hGQp#fG)xB9tKQ7S!_GfL{0VSyZH zdf16#mxmJR5?zp}2Z9tQj80eFuh(NNj@Ob$yUpC-&Rw~_45VA_W|x?ZdU{kfT{-wP zrQ^=ijFPd)6y7vv!#*XKwqKtjXJ8@D$u#YsgeK|>6GPahi&Ib*xz@p2XIp4jmK1zy z3Py7tApck_(GUS-^_w9;g^n*j#fc(;&h!HLhhm8-5kOX78UkdEHgyHG$_v!)t1F4A z5kTx;{Z_%qb-J9zmgO=tw>x`cPbP1s-*5K!qszL~ZEt-hT9(UHOR^gdbOL*=X=@Yh z%KLHd$B&&@LbYsc97&K^z%)RQFY$>nIY=ty?&!zLmr=M2{(y|<&N zg#)_NyE}@haxNT0F7L`^oh_!Pmg(a$*1KF)hb&4E-CCkpb}gGOtEo4otn>*~ls&l< z88bU#+~U#O4Ct+_p)@qd?mh?o<#3--n9k0hD@Mv8pv}?@!)}#08mMt~{xxVr%0qz~ zyoGSaZZo#aZH`5r+rhK}zilXRw=&sQ81XuFw-KWBpb8J6d2U6E)hE}eR}F|{+n3`8xaSXKdoCQvUEP)6W)9{L8Z+gf1KVn1to7qBVmtpA9nYBm zR&T&=A9B@22?{!TMfp*A2BlxYL-Rhu`0e5&<-y_t!zDF3ZPvc{C zP`E#0{J8_e#fhkS9NOtn?__n~fV04T3_0|+6{ zf=G*YBc42yB-V6r?`7HQ<)Z=hcZ)eV&_9^~huYkAL}KH1#WYi{k^ zYf7h?&zm_F-(vPM1xQ^zm`$zk=|vB|A-iWNzqNlLFPpOc*KNwOQ?Ky&JeEA_(8i(h z#!pAR->$c*x=g- zRu$1ED;gsj+Dq%OiUaRGh}26o@zt_WGnQ}MV3J;5z5 zxpjO&GFcL=q6A8D&LIzJjkqs2NtO^y@v!zW(Guvf(5-jeqz{v3$;La8a>1>5@VVUB za&W*jyz<8G+!(b{?W@g8lNm|XPhM|4C)XS9$BeP1PH6P7vXFLW|Ftqo5$u9D*faPWIIC@xP#ff50DV)PU1n8v5)jH-Gl{-!|>4 zzwZu1mM(hq2)lcFFsmA$uSH-_`yK@P$u@*32s;osO1u_j!sxaDLCZJLmD-C&Y<1=lcAs zD8BROeBa(V$vHpQ1!f}fFM{|FKm3Cp{=W|YE1VPcIS6Tlxd_b&^AP4E@cBYqx$$o? z0{_Be2?GBFqXmJ_-$vjyKHj+owqSQEKiNp*>lS{Ep2Mc)YK}%psa-*ognpDZc*Zwz z>Kwf3mA11UV;4ElbNCv*TgkE8CRcpLzkrwUDMO_h&t7GvOyNrqZp`MFGc5ZCXd$Cy zB88u_41f}TECxR75bIB+__N85l?O)(yW