From e76448a83ca7e015d18f0b3cf12473d487b35899 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 15 May 2014 21:14:52 +0000 Subject: [PATCH] Bug 56325: fix Exception when removing sheets with named ranges in the workbook git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1595048 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/model/InternalWorkbook.java | 10 ++++-- .../org/apache/poi/hssf/model/LinkTable.java | 18 ++++++++--- .../poi/hssf/record/ExternSheetRecord.java | 28 ++++++++++++++-- .../apache/poi/hssf/usermodel/TestBugs.java | 30 ++++++++++++++++++ test-data/spreadsheet/56325.xls | Bin 0 -> 24064 bytes 5 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 test-data/spreadsheet/56325.xls diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java index 4b4a11f9c9..e54d6c5d80 100644 --- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java @@ -716,11 +716,15 @@ public final class InternalWorkbook { // Bump down by one, so still points // at the same sheet nr.setSheetNumber(nr.getSheetNumber()-1); - - // also update the link-table as otherwise references might point at invalid sheets - linkTable.updateIndexToInternalSheet(i, -1); } } + + // also tell the LinkTable about the removed sheet + // +1 because we already removed it from the count of sheets! + for(int i = sheetIndex+1;i < getNumSheets()+1;i++) { + // also update the link-table as otherwise references might point at invalid sheets + linkTable.removeSheet(i); + } } /** diff --git a/src/java/org/apache/poi/hssf/model/LinkTable.java b/src/java/org/apache/poi/hssf/model/LinkTable.java index 3304ae2351..6e203ef1d1 100644 --- a/src/java/org/apache/poi/hssf/model/LinkTable.java +++ b/src/java/org/apache/poi/hssf/model/LinkTable.java @@ -414,9 +414,17 @@ final class LinkTable { public int getIndexToInternalSheet(int extRefIndex) { return _externSheetRecord.getFirstSheetIndexFromRefIndex(extRefIndex); } - - public void updateIndexToInternalSheet(int extRefIndex, int offset) { - _externSheetRecord.adjustIndex(extRefIndex, offset); + + /** + * @deprecated Was prevously used for removing sheets, which we now do differently + */ + @Deprecated + public void updateIndexToInternalSheet(int extRefIndex, int offset) { + _externSheetRecord.adjustIndex(extRefIndex, offset); + } + + public void removeSheet(int sheetIdx) { + _externSheetRecord.removeSheet(sheetIdx); } public int getSheetIndexFromExternSheetIndex(int extRefIndex) { @@ -453,8 +461,8 @@ final class LinkTable { */ private int findFirstRecordLocBySid(short sid) { int index = 0; - for (Iterator iterator = _workbookRecordList.iterator(); iterator.hasNext(); ) { - Record record = ( Record ) iterator.next(); + for (Iterator iterator = _workbookRecordList.iterator(); iterator.hasNext(); ) { + Record record = iterator.next(); if (record.getSid() == sid) { return index; diff --git a/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java b/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java index 4de0f60b02..0ff062b088 100644 --- a/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java +++ b/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java @@ -164,8 +164,32 @@ public class ExternSheetRecord extends StandardRecord { return _list.get(i); } - public void adjustIndex(int extRefIndex, int offset) { - getRef(extRefIndex).adjustIndex(offset); + /** + * @deprecated Was prevously used for removing sheets, which we now do differently + */ + @Deprecated + public void adjustIndex(int extRefIndex, int offset) { + getRef(extRefIndex).adjustIndex(offset); + } + + public void removeSheet(int sheetIdx) { + int nItems = _list.size(); + int toRemove = -1; + for (int i = 0; i < nItems; i++) { + RefSubRecord refSubRecord = _list.get(i); + if(refSubRecord.getFirstSheetIndex() == sheetIdx && + refSubRecord.getLastSheetIndex() == sheetIdx) { + toRemove = i; + } else if (refSubRecord.getFirstSheetIndex() > sheetIdx && + refSubRecord.getLastSheetIndex() > sheetIdx) { + _list.set(i, new RefSubRecord(refSubRecord.getExtBookIndex(), refSubRecord.getFirstSheetIndex()-1, refSubRecord.getLastSheetIndex()-1)); + } + } + + // finally remove entries for sheet indexes that we remove + if(toRemove != -1) { + _list.remove(toRemove); + } } /** diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 3bd97827cc..a6894a1a1b 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -27,9 +27,11 @@ import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -2582,4 +2584,32 @@ public final class TestBugs extends BaseTestBugzillaIssues { assertEquals(5, cf.getNumConditionalFormattings()); } + + @Test + public void bug56325() throws IOException { + HSSFWorkbook wb; + + File file = HSSFTestDataSamples.getSampleFile("56325.xls"); + InputStream stream = new FileInputStream(file); + try { + POIFSFileSystem fs = new POIFSFileSystem(stream); + wb = new HSSFWorkbook(fs); + } finally { + stream.close(); + } + + assertEquals(3, wb.getNumberOfSheets()); + wb.removeSheetAt(0); + assertEquals(2, wb.getNumberOfSheets()); + + wb = HSSFTestDataSamples.writeOutAndReadBack(wb); + assertEquals(2, wb.getNumberOfSheets()); + wb.removeSheetAt(0); + assertEquals(1, wb.getNumberOfSheets()); + wb.removeSheetAt(0); + assertEquals(0, wb.getNumberOfSheets()); + + wb = HSSFTestDataSamples.writeOutAndReadBack(wb); + assertEquals(0, wb.getNumberOfSheets()); + } } diff --git a/test-data/spreadsheet/56325.xls b/test-data/spreadsheet/56325.xls new file mode 100644 index 0000000000000000000000000000000000000000..38b0e4abb5ec97e5d57d08bf8e0228b12c7df6d0 GIT binary patch literal 24064 zcmeHP4R96J6+XM!m-j;WNkT~Yk>n8wKLJAWUIHP3ynuo-JPM;_YDXPF%tHnh2hnM# zGQ`%-bSnN4ic?9eR-LwLwJH`nwFPE6+L=DFV`Z$!ShQ&AC_2z~T7O@^bNB7O-Me=; z?@ei^c6Kv&-|o5Re&^hC&pms8cJsTJs!u%dKKNDz!BW~8dVI7F=7}Do$80-(BOscww3Eb(8EvD24fyYXdG!%X%0=rUmG=1pV5kQ z54!YRP-J?8u@C`&qPoasbTReQRdj`K;U0?8Bxm7P$n=-$&TTN3p}O&);(i*)KwGGV zlBBIe9b16tttaQ0HVDeM$~S?#$+n$Ymd|~}=LTa7%~Fe72Xn8ZR>7tc=gxPF^X6IN zzJ_M&QPs|E22)RNXLlV{TyD`RbRo-AOcXvB#Lo^A@7CK-yqn7_-kWcVKS8yKG(>;T z#l^3AIuvI*higIqo0Szz=b)+L_oFkaivRzauA%$bhi=+40)nu^{(O#cpRm7|{u3Yi zBR=%Uedzmq=ui02AM>Gq&xd}S58Y26Km4Z!&Nz>KzVAaf_3`qX`grLN`QSY1L*MU1 z|CJB@+dlN4`_N0(=NW-B%CgU%>gszc@2MoiY{$_K`oYG7jb!W<{9_z+li#FA5C?@H z{u2!1u>7+{Nsod@@V`Q9aM_4)rNEE5>6I3}qCk)15^9+IHeHG{9GUX?8;E*s{A!E7 zCzCPjbwJ4FnU&y|a7J2myMH|@_@S7|Z_~~GHP)g_ICg&=s;;i9tb?8J6*v)$8B}(| zJiyX_JuW9&^{uh!9{kx#oCqF=Dtyc!OuDWABk`MKwRAH#fPM7RqaWV>%#PSx+To%z zgHEFm=gV-#vEF*Lj>afQ_ZDE5k_vF#FR1`4Qc?k<)B@auB}rhPDycxU)B@ZnB}w4! zQc?l-ladN>$0(@)_pp)*aL6jD0LQYD3UH7ssX$Mu1s0WBV0Nhmc=@NKI&_smY}Qqsf%mMLG|7I~28rgYxO3;u98daquEEXD08C4P!n3pr2hE&o5Z1g3d|(&H zj+m^}EYO`wrPNBDJ$u%pR4SG7D8)2cDW=IvF)gnXtRySd;!^6P9X{q(DRu5FCHKaK zP|EC$Y<-VXR+fc=gnR?Jj~8*sSp447pZ+^v10wlqke)+;|&V2KIvRfVKeQ(T;6gM*SIg$d!Gz zt{9MPIvC_U7YJ@t)sch0tjW}3KnR23g0a0e;mJr5HVFo)F9sx=4hEU*0>O>iCcz-n ziUGm?2G}eI^xA|cI7Qeb7-W1gAlY;L8cc2!ka!pHmP-i_{Ni95KOBb)$swzrh`G^E)d+PZ4wMJqZkn0 zU<@r&g3F|&8>_fK{GE@u*~WpnF)o=rU02d z9?Hv%^k>ct5Edesk352k1v_Et5 zkMF5v)&|M!RLE=%kZI0*gv?lf=GgmxRm&U~By+w(W+Ff)55n@ct?18u`j^+#GV54o zM+tu02$}x;woD_Nel~kjEwesIrpj-p2FP^!Z8rVRvj@~N#|Oz&`R$AVnNGjWrvGsA zIkn6QK{8c-J3Bz8({Hos?4e_7nG=I#s{D3dfJ~>~X441WIHi_3DM+TuZ<7Hsoqn55 ze{yucTIP8ybAAba8;4AP{(|2olHYmyRZ}JhTO4-0og5@n<+sxUWIFvekvz6{k6LC! zkW7`|&J2+0^xH)8*RSqV%WMpisq))70WzI_n@B!+{|joF@gSKhzikVU>Gaz~a`%hJ z)iS39$yE7mM}SPH-zJhD9lTQ|b1XK*qom1WgC&UVaJKxR8IyBSY5Y*|PrN^M%P-Z~ zQ)O)SNf~>JgzfZ0Y}Kq;rN*8nW3%tc*v%5Q)8B~p{o>1N>}DC8eN4ulC1E@LifG`f zAE>dX%h>EoGWJ{v+vz_pTdj9#O| zhy(XVB+Emv(xca+Tp^$5VLZNw@R|{pEqT1i*SxkV;#WCge+vX&jnTmDyoFBsG(VVY4%TD{R`B;h4JNbD`$@D`qL ztaJmGRply4&C9Vx%rCGM*0^GaX#@6jpL3it;R>18Gn4FnGbqhNNe+x#*t{TWXZ8U(l{dNUP}(@qtGA*Y2z^y>_uC7 z^`DS4$jZfh)5LJj_PF<2tzq^U>#iL7&%#8qhO~)5=hVoExD4KIodtD?sC2+4{t+*ix(2xHzkdv!UM0?g=R<_+rO@Y-99 zw8_wK0erX(nwvd(fm_$IG3Z7zm&)cEdr$|r>10uJwn3Vk5x=bXp>-7Cx=Th?3vk$VPh??SUpC(6^z>pM_2;0@^XY}+J$a(y%X=~d-*Km zT2GEV?gU!EVYm@y?J^bt^+HFZT_+)rKp0kW@)EQC$in`F&9^7Gl*#n*xtp4ZmZrQ}>v)ZdiSMu8M{$WyMrB zoc&y}p1Qm-#r<|Jbz|>9Yk~p0(L77hS5q))C!YA{8|Kw~T$_!y8ZNO~(xft1OU0IU zHYJp@T*RQBfii{d%3!-`Xk9kRVkpNC7w1!J(Lgy$ap-q=e55=#T3O2dxj>K@2;*G;-1=qWidcoG#CM&6dmahO}_aJ7Sm89S)7|f^yaP`yx25#Jl(XbO~*z zuZpWx;w|LObR%7h__+pe!X1wdpX>6MI_-tE_BwKwzOB(d(DpfKY@J=8?Q+lzgekr zeX~-j^LAh1cphVjQP6`Um5#U8$B64{oC(n*MxXrn%*(5;T$5TS=q-XiTb#cxC-D!*M;+Mmd$5<+y$T^GQb3pq~amVGAV1 z@%xnZyPbwG9vzbEk7D+*h{s9dSPLz{sPlVA&ZAeap`QstpM!iG?0TCESEtFaP+3ek zAb)m%tFle8dfMb*gE4u~Pam;;nAxDSTAu zPvgkGYV*czTW{E!*&bi^m5u$^2-yRfw;p$K|C|pW=lTD7O#JyB zTAu%}LE;Y=FGb>S0tNEQP zp4ao7o#*fTmJ!eUd5+KjP=NRRd7f{*8O8HlUJu}J;`tkJ{^ptAapV6J&7b}AT(}V_ zjx+^{=lWBTIKLUc(~m V(4k+nKxz8mzgw4F|7i~S{{lljsN4Vm literal 0 HcmV?d00001 -- 2.39.5