From 324a36e170d9cfadb02f33a0667f9049ec7a9e60 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Wed, 21 Aug 2013 22:08:20 +0000 Subject: [PATCH] Bug 50298: Fix corruption of Workbook when setting sheet order. The boundssheets themselves were adjusted, but not the corresponding records. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1516313 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/model/InternalWorkbook.java | 84 ++++-------------- .../poi/hssf/usermodel/TestHSSFWorkbook.java | 77 ++++++++++++---- test-data/spreadsheet/50298.xls | Bin 0 -> 20480 bytes 3 files changed, 79 insertions(+), 82 deletions(-) create mode 100644 test-data/spreadsheet/50298.xls diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java index a989c32b98..0a489d29cf 100644 --- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java @@ -26,61 +26,8 @@ import java.util.Locale; import java.util.Map; import java.util.Map.Entry; -import org.apache.poi.ddf.EscherBSERecord; -import org.apache.poi.ddf.EscherBoolProperty; -import org.apache.poi.ddf.EscherContainerRecord; -import org.apache.poi.ddf.EscherDgRecord; -import org.apache.poi.ddf.EscherDggRecord; -import org.apache.poi.ddf.EscherOptRecord; -import org.apache.poi.ddf.EscherProperties; -import org.apache.poi.ddf.EscherRGBProperty; -import org.apache.poi.ddf.EscherRecord; -import org.apache.poi.ddf.EscherSimpleProperty; -import org.apache.poi.ddf.EscherSpRecord; -import org.apache.poi.ddf.EscherSplitMenuColorsRecord; -import org.apache.poi.hssf.record.BOFRecord; -import org.apache.poi.hssf.record.BackupRecord; -import org.apache.poi.hssf.record.BookBoolRecord; -import org.apache.poi.hssf.record.BoundSheetRecord; -import org.apache.poi.hssf.record.CodepageRecord; -import org.apache.poi.hssf.record.CountryRecord; -import org.apache.poi.hssf.record.DSFRecord; -import org.apache.poi.hssf.record.DateWindow1904Record; -import org.apache.poi.hssf.record.DrawingGroupRecord; -import org.apache.poi.hssf.record.EOFRecord; -import org.apache.poi.hssf.record.EscherAggregate; -import org.apache.poi.hssf.record.ExtSSTRecord; -import org.apache.poi.hssf.record.ExtendedFormatRecord; -import org.apache.poi.hssf.record.ExternSheetRecord; -import org.apache.poi.hssf.record.FileSharingRecord; -import org.apache.poi.hssf.record.FnGroupCountRecord; -import org.apache.poi.hssf.record.FontRecord; -import org.apache.poi.hssf.record.FormatRecord; -import org.apache.poi.hssf.record.HideObjRecord; -import org.apache.poi.hssf.record.HyperlinkRecord; -import org.apache.poi.hssf.record.InterfaceEndRecord; -import org.apache.poi.hssf.record.InterfaceHdrRecord; -import org.apache.poi.hssf.record.MMSRecord; -import org.apache.poi.hssf.record.NameCommentRecord; -import org.apache.poi.hssf.record.NameRecord; -import org.apache.poi.hssf.record.PaletteRecord; -import org.apache.poi.hssf.record.PasswordRecord; -import org.apache.poi.hssf.record.PasswordRev4Record; -import org.apache.poi.hssf.record.PrecisionRecord; -import org.apache.poi.hssf.record.ProtectRecord; -import org.apache.poi.hssf.record.ProtectionRev4Record; -import org.apache.poi.hssf.record.RecalcIdRecord; -import org.apache.poi.hssf.record.Record; -import org.apache.poi.hssf.record.RefreshAllRecord; -import org.apache.poi.hssf.record.SSTRecord; -import org.apache.poi.hssf.record.StyleRecord; -import org.apache.poi.hssf.record.SupBookRecord; -import org.apache.poi.hssf.record.TabIdRecord; -import org.apache.poi.hssf.record.UseSelFSRecord; -import org.apache.poi.hssf.record.WindowOneRecord; -import org.apache.poi.hssf.record.WindowProtectRecord; -import org.apache.poi.hssf.record.WriteAccessRecord; -import org.apache.poi.hssf.record.WriteProtectRecord; +import org.apache.poi.ddf.*; +import org.apache.poi.hssf.record.*; import org.apache.poi.hssf.record.common.UnicodeString; import org.apache.poi.hssf.util.HSSFColor; import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalName; @@ -405,14 +352,14 @@ public final class InternalWorkbook { } for (int k = 0; k < 21; k++) { - records.add(retval.createExtendedFormat(k)); + records.add(InternalWorkbook.createExtendedFormat(k)); retval.numxfs++; } retval.records.setXfpos( records.size() - 1 ); for (int k = 0; k < 6; k++) { - records.add(retval.createStyle(k)); + records.add(InternalWorkbook.createStyle(k)); } - records.add(retval.createUseSelFS()); + records.add(InternalWorkbook.createUseSelFS()); int nBoundSheets = 1; // now just do 1 for (int k = 0; k < nBoundSheets; k++) { @@ -422,13 +369,13 @@ public final class InternalWorkbook { retval.boundsheets.add(bsr); retval.records.setBspos(records.size() - 1); } - records.add( retval.createCountry() ); + records.add( InternalWorkbook.createCountry() ); for ( int k = 0; k < nBoundSheets; k++ ) { retval.getOrCreateLinkTable().checkExternSheet(k); } retval.sst = new SSTRecord(); records.add(retval.sst); - records.add(retval.createExtendedSST()); + records.add(InternalWorkbook.createExtendedSST()); records.add(EOFRecord.instance); if (log.check( POILogger.DEBUG )) @@ -628,9 +575,15 @@ public final class InternalWorkbook { */ public void setSheetOrder(String sheetname, int pos ) { - int sheetNumber = getSheetIndex(sheetname); - //remove the sheet that needs to be reordered and place it in the spot we want - boundsheets.add(pos, boundsheets.remove(sheetNumber)); + int sheetNumber = getSheetIndex(sheetname); + //remove the sheet that needs to be reordered and place it in the spot we want + boundsheets.add(pos, boundsheets.remove(sheetNumber)); + + // also adjust order of Records, calculate the position of the Boundsheets via getBspos()... + int pos0 = records.getBspos() - (boundsheets.size() - 1); + Record removed = records.get(pos0 + sheetNumber); + records.remove(pos0 + sheetNumber); + records.add(pos0 + pos, removed); } /** @@ -1087,11 +1040,13 @@ public final class InternalWorkbook { Record record = records.get( k ); if (record instanceof SSTRecord) sst = (SSTRecord)record; + if (record.getSid() == ExtSSTRecord.sid && sst != null) retval += sst.calcExtSSTRecordSize(); else retval += record.getRecordSize(); } + return retval; } @@ -2320,10 +2275,9 @@ public final class InternalWorkbook { * @param password to set */ public void writeProtectWorkbook( String password, String username ) { - int protIdx = -1; FileSharingRecord frec = getFileSharing(); WriteAccessRecord waccess = getWriteAccess(); - WriteProtectRecord wprotect = getWriteProtect(); + /* WriteProtectRecord wprotect =*/ getWriteProtect(); frec.setReadOnly((short)1); frec.setPassword(FileSharingRecord.hashPassword(password)); frec.setUsername(username); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java index eab7b57dac..c1b226b87f 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java @@ -17,29 +17,40 @@ package org.apache.poi.hssf.usermodel; -import java.io.*; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.util.List; import junit.framework.AssertionFailedError; -import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.POIDataSamples; +import org.apache.poi.ddf.EscherBSERecord; +import org.apache.poi.hpsf.ClassID; import org.apache.poi.hssf.HSSFITestDataProvider; +import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.model.HSSFFormulaParser; -import org.apache.poi.hssf.model.InternalWorkbook; import org.apache.poi.hssf.model.InternalSheet; -import org.apache.poi.hssf.record.*; -import org.apache.poi.ss.formula.ptg.Area3DPtg; -import org.apache.poi.util.LittleEndian; -import org.apache.poi.util.TempFile; -import org.apache.poi.ss.usermodel.BaseTestWorkbook; -import org.apache.poi.ss.util.CellRangeAddress; +import org.apache.poi.hssf.model.InternalWorkbook; +import org.apache.poi.hssf.record.CFRuleRecord; +import org.apache.poi.hssf.record.NameRecord; +import org.apache.poi.hssf.record.Record; +import org.apache.poi.hssf.record.RecordBase; +import org.apache.poi.hssf.record.RecordFormatException; +import org.apache.poi.hssf.record.WindowOneRecord; import org.apache.poi.poifs.filesystem.DirectoryEntry; import org.apache.poi.poifs.filesystem.DirectoryNode; import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; import org.apache.poi.poifs.filesystem.POIFSFileSystem; -import org.apache.poi.POIDataSamples; -import org.apache.poi.ddf.EscherBSERecord; -import org.apache.poi.hpsf.ClassID; +import org.apache.poi.ss.formula.ptg.Area3DPtg; +import org.apache.poi.ss.usermodel.BaseTestWorkbook; +import org.apache.poi.ss.util.CellRangeAddress; +import org.apache.poi.util.LittleEndian; +import org.apache.poi.util.TempFile; /** * Tests for {@link HSSFWorkbook} @@ -455,13 +466,16 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { public BadlyBehavedRecord() { // } - public short getSid() { + @Override + public short getSid() { return 0x777; } - public int serialize(int offset, byte[] data) { + @Override + public int serialize(int offset, byte[] data) { return 4; } - public int getRecordSize() { + @Override + public int getRecordSize() { return 8; } } @@ -598,6 +612,8 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { wb = HSSFTestDataSamples.writeOutAndReadBack(wb); assertEquals(3, wb.getNumberOfSheets()); assertEquals("Root xls", wb.getSheetAt(0).getRow(0).getCell(0).getStringCellValue()); + + fs.close(); } public void testCellStylesLimit() { @@ -606,12 +622,12 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { int MAX_STYLES = 4030; int limit = MAX_STYLES - numBuiltInStyles; for(int i=0; i < limit; i++){ - HSSFCellStyle style = wb.createCellStyle(); + /* HSSFCellStyle style =*/ wb.createCellStyle(); } assertEquals(MAX_STYLES, wb.getNumCellStyles()); try { - HSSFCellStyle style = wb.createCellStyle(); + /*HSSFCellStyle style =*/ wb.createCellStyle(); fail("expected exception"); } catch (IllegalStateException e){ assertEquals("The maximum number of cell styles was exceeded. " + @@ -877,4 +893,31 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { wb.unwriteProtectWorkbook(); assertFalse(wb.isWriteProtected()); } + + public void testBug50298() throws Exception { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("50298.xls"); + + + HSSFSheet sheet = wb.cloneSheet(0); + + wb.setSheetName(wb.getSheetIndex(sheet), "copy"); + + wb.setSheetOrder("copy", 0); + + wb.removeSheetAt(0); + + // check that the overall workbook serializes with its correct size + int expected = wb.getWorkbook().getSize(); + int written = wb.getWorkbook().serialize(0, new byte[expected*2]); + + assertEquals("Did not have the expected size when writing the workbook: written: " + written + ", but expected: " + expected, + expected, written); + + FileOutputStream fileOut = new FileOutputStream("/tmp/workbook.xls"); + try { + wb.write(fileOut); + } finally { + fileOut.close(); + } + } } diff --git a/test-data/spreadsheet/50298.xls b/test-data/spreadsheet/50298.xls new file mode 100644 index 0000000000000000000000000000000000000000..99eb7643e60c85ae92cac08cebe76ae7bbbacecf GIT binary patch literal 20480 zcmeHP4RBo5bw2NDrCnLr*4n~0HkQ`%pDasaEy;fvvof*M(#qJ_q0qqz7;D!ye{LPc z#Zz1agkfU4sbeR0W4nzZFii=CVB8P}TI_a)kcnuMw3H!*j>&{H37rI-pC%q)`+fJm zyZheT_nzJcm#I5?n!EendFOuT+;h%7_h;YJxBfYO`hh2=zAnDwa;cH`iuF?GqFXp; zQAeZ5Oq}rgy<)Mbi6S^1JeiA}7_Se3)P9D{njdi~GN6?R))~oxjBQ zEYMDBHn|qkx63VZlhWfBU8{=E31VA}TcbmqsucarKFwJx>*W)28~!$`wqFS=6qHV+ z49ZrV-yQ+l2Kmf^>_5~Yg4J{DTh3Ljtb~M3N{77iKy~^ofqc{UxbkVQHY$yyK80Rf zS1XUSyqNq3vUweDzAmhp2M|E&64jyu=c&2qLO_e@gZ#*A88gvO0@B#f)T&C$djv|1tY`_JiiR1Ns#2iq=CG+rys_uI28F!`jjHgCUI^O_CcuR821Ygam@I7W`8)oT7&Re#j%{kvNuQ|~Idt;7PzN?JoVadSan8G8hICF?N9pIz4z#YNrudGXZJ(V_IP}C9y!J2A|0aa~NC^G05c-i2 z`r{$=M?>g;5klV+LJ!j?4F5@m!$p&d!;&5TGK8-6spQxCRMH;`!FeKtej~N@bs)JMkkruDHll^v@Ow&$T?)B8jIIKNp@}dwyBblbBj4`mek= zNyC3Mou1S%sX-El6+aiRivM5bMw}wwX}Mg`dhvDKa*?a(pG~I^H5>xYL50JGucG_( zPs$ukuh6G0kj@O|=(855SPyG`xS)2?(R@wkBHN*7a0)eibe}$4kURVvF|3#Hn+zT8 z(stJE;=*0=`*4y{r|A{#N(Iun_*Xb3yXk()4FN?zffF7Hod+<~>pa7N=Yc$PXR1l= zG}nrllSk7FPwz@~$}SIGRluf1lUjgrw(bla?c*@Tuq!K&#jd6TXSh{W;7YKn3S5p= zRe^gnRaM}IPE{4yiK?nVr&LveX`reK%nwynVB)B%0y9Ka6_`4zs=%C5RRt!S$O3~>%vUi$7@FZAOky!W7_Q+U%y=NVIpDbz^`;dxv|wF+ifd!_DCsAkp68?s5!} zXzyI?0l|fLEYW?ZyC>Q^+}|#15(*OS9d3}v0EzYvci3ZqM0+O_4$D<=c8)b6aiCGzbQIUp=^qZ3dR)0;&4f-F~#xY$16j@_-zbx)6JQ{hwSQe)C)c5N#ehr9u;9 zo-=38lt7Gm?z-!)3iG&W#yq3}TOGzcZd#?2v*6WErc$WvZCPan?At|0g}+B(V=}se zf7aH63gvw7?}9cxMq>2v)f)$-n2QEk7ze~Zkow!d2q5KdRMDJWscU>0WwB~p=u(g| z;cvg&CMKg)rm7_XE(aj9PheIE3cz!hs{H0zS~3pm!>f|%sKw#W|1Wk_f3H{$rOJ5>Kj{W5_54rWDadc^%cD(kz;3}t=~%J4 zB`dQfBb_J@;zz<~vOmkdkj-Gzet&8;3SgYWN;DV8;>U+DekyI@fs14!U-<*CG8X=SU#*tZ@FPwSp)nJ(uqGa|2$ea@)Q>QG|&boZz$8WzD zEOTO%%-#T*?GZ9{hEp<=`NA8g|0`H#QG-ogUwHRNuLR4SM47#zaab9*Esz<`Uy_*^?*H2G_k(3lj*=M^w=*MTy5n}Z z{|8ThEm-E1D49WVJ2yh6J8p;jU;h5H!7?w3k{J}Y^CM)s<94`z_~dtkWloKf85Fmd zM96f*z^%y9m4+;;bT?#Y+6 zOeR|#rF=U*N@h^p&We!fj@#~@R}OtHSY~sS%%He!i;(G#+wPu!__xEsGFzf#2E}bh zgiLqbcJ~~A;OoIMGf^^w;x-#0(;c_nJqKQN#udLmXGF;iird}@neMpl?m2sMUr;+I z;>YczSpK=e3gmVu=NwjIdNJvnKa}HBWX;`w7mPjAkIk6$W6$tmyW4X81rM#^I^N=N@Qr@!C>q;er(2)AG_0s?T#Oj-+lD? zVC=bmY{rZqdw~zz9Va6HeelCU*kh#)3pMMx>_eD8zM0*hx&+83KJa^4&G=$M%w5jw z)%+04o0cngseEM}uqWaH6~x-9>|dz|)-v*pLfYv?vL{17FArz&oU7DgSsMNuTq|?de0SgYA5*C?8fY;vYWH_WN*%{MC*Juvo3~CULF(w6wB=w z>csP-*rRdsb3*11)fV@sGTsK()`I!l*>{}DIE{F~uE)bTcQqCz*pNqVq!|}`8)8jELwLtyJnXd?RBp&P#;cc0h?ClV=#TfOws~>v zNxoDiF$xp@b|Ls)O^Jy%lTg&`(xuxpaMU-EMB#z+wsM9;yLnhmW(S*ihok$o0^t z3neQbWi`|rFFlNhJ)b&z^|j8DF6V`)X@o_}_l^IOu5M}5e@X0~=p0VeUNF=)YCq}(7phLvA*gyjWGmD~Zd4{|E^L*3)Q43w(S^zc zU747$aIKfcjJ4+vrgra7{gw~mBE%Z+aHC3TiV7w|B&SZA+3vaT|Q zF9a^{gvBs(nd%EBhDj*Lu)Z=etglQA>nq0aQpWJ3yT=uRVt659Sl=^tVGOealoHsb zLrFyDRIf>DlTdx?zAx>ijTyhoh7#av^Km8cvJSa|Vz(ZT*upaTK+7}%o_E}q;U10Y z-MQQjuVr+fHJM0X8O!J^V;Ox#on=U%^L($)mO%7HHy_0F^-iF;oy|a5PPMn)6O`Ph zqAu^alN)z&jN07lQHK3=>N@$fD@ymljA{IR1ROWXt&+zZ0E2i8k!Pz>2Jp;3-w0SL zYjL&%Pv;N1IQl``EL%WZfaS!E7I)w7!_znqsPvMQD{tSiZR@7|eb^A%$li)4Q618P z`w+b|*efs=tlYXe|M5X`?c`1#xK`#1`R&{D1EhVM8&06D%WukW-H{(SoSGtsT}{wt z(tEjI!~^_ablqdkz34nw#YR|?XvEh> zj(By*ALek=YhCmrep5+FJ(|tuCN`xep_n@Q6(C;OauE6*blth)4NH!IcEm-aZhJx7 z>!b0R5B-8dqvr*M2D^#iXq$dn=_%~4zl}1M;R1bZIB~o>Pves>Dy8I|zkd5Wt8ZGH zyFt<06@9+?yhl-nz+;Ztj93|G-Hkh^5|niz%DsyFlj?I0xC`p|L2j<=ysd=t!`wXA z`2(2v6VZZxu&B@qJ?i+EocXroXlf1$7(>6P4 z)t18zy{Q45B`v-ZVo8VUb1N{X%V2MR0gy9thdN&hIT`hu*~n6YIjmrEiF%CPU*&*i z=%F>CR=ugi4f<1%6$hX-^N$;?4zG|I815L`sy@GK@aH~Q(|uSK_gTNagr0D64&hn6 zXl|s@^J*-xvcLbTjko1{x|gq7yC&Pac*TdhKq@1sP$*xCn3xig%&8Ml(rDE6D5H6mju_3WrdN*URmQ`) zKSuMah+bI)W!|1%;b}LnwSlYA?z}ls`vdtUZKaYsNS0T(f@Fx&`<)*Xo~)IMg3LJtd>p#}+pJ;VRMfB1}l*?(&a1#FCKH)guC*`AC?66X|GY%LU)VAQ})4V~S! zeoH<-*tJA{zV!GXhH5Z+-G>_4A~!x)pN OTKO?yW3!#l^8W|Ncgx8D literal 0 HcmV?d00001 -- 2.39.5