From 143128be30625dea2a3b04f5804cda90ce2aff4e Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Fri, 23 May 2008 05:28:54 +0000 Subject: [PATCH] Fix for bug 45046 - allowed DEFINEDNAME records without EXTERNALBOOK records git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@659429 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../org/apache/poi/hssf/model/LinkTable.java | 34 ++++++++------ .../org/apache/poi/hssf/model/Workbook.java | 3 +- .../apache/poi/hssf/data/ex45046-21984.xls | Bin 0 -> 22528 bytes .../poi/hssf/usermodel/AllUserModelTests.java | 3 +- .../poi/hssf/usermodel/TestLinkTable.java | 44 ++++++++++++++++++ 7 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 src/testcases/org/apache/poi/hssf/data/ex45046-21984.xls create mode 100644 src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 4fdeaefb0..cc52e793b 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 45046 - allowed EXTERNALBOOK(0x01AE) to be optional in the LinkTable 45066 - fixed sheet encoding size mismatch problems 45003 - Support embeded HDGF visio documents 45001 - Partial fix for HWPF Range.insertBefore() and Range.delete() with unicode characters diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index bc57530d2..85f5b8963 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 45046 - allowed EXTERNALBOOK(0x01AE) to be optional in the LinkTable 45066 - fixed sheet encoding size mismatch problems 45003 - Support embeded HDGF visio documents 45001 - Partial fix for HWPF Range.insertBefore() and Range.delete() with unicode characters diff --git a/src/java/org/apache/poi/hssf/model/LinkTable.java b/src/java/org/apache/poi/hssf/model/LinkTable.java index 88c94a61e..a19971b7d 100755 --- a/src/java/org/apache/poi/hssf/model/LinkTable.java +++ b/src/java/org/apache/poi/hssf/model/LinkTable.java @@ -41,7 +41,7 @@ import org.apache.poi.hssf.record.SupBookRecord; * * In BIFF8 the Link Table consists of *
    - *
  • one or more EXTERNALBOOK Blocks

    + *

  • zero or more EXTERNALBOOK Blocks

    * each consisting of *

      *
    • exactly one EXTERNALBOOK (0x01AE) record
      • @@ -55,7 +55,7 @@ import org.apache.poi.hssf.record.SupBookRecord; * *
    *
    • - *
  • exactly one EXTERNSHEET (0x0017) record
    • + *
  • zero or one EXTERNSHEET (0x0017) record
    • *
  • zero or more DEFINEDNAME (0x0018) records
    • *
* @@ -63,6 +63,7 @@ import org.apache.poi.hssf.record.SupBookRecord; * @author Josh Micich */ final class LinkTable { + // TODO make this class into a record aggregate private static final class CRNBlock { @@ -79,8 +80,8 @@ final class LinkTable { _crns = crns; } public CRNRecord[] getCrns() { - return (CRNRecord[]) _crns.clone(); - } + return (CRNRecord[]) _crns.clone(); + } } private static final class ExternalBookBlock { @@ -136,16 +137,19 @@ final class LinkTable { while(rs.peekNextClass() == SupBookRecord.class) { temp.add(new ExternalBookBlock(rs)); } - if(temp.size() < 1) { - throw new RuntimeException("Need at least one EXTERNALBOOK blocks"); - } + _externalBookBlocks = new ExternalBookBlock[temp.size()]; temp.toArray(_externalBookBlocks); temp.clear(); - - // If link table is present, there is always 1 of ExternSheetRecord - Record next = rs.getNext(); - _externSheetRecord = (ExternSheetRecord)next; + + if (_externalBookBlocks.length > 0) { + // If any ExternalBookBlock present, there is always 1 of ExternSheetRecord + Record next = rs.getNext(); + _externSheetRecord = (ExternSheetRecord) next; + } else { + _externSheetRecord = null; + } + _definedNames = new ArrayList(); // collect zero or more DEFINEDNAMEs id=0x18 while(rs.peekNextClass() == NameRecord.class) { @@ -222,7 +226,7 @@ final class LinkTable { public void addName(NameRecord name) { _definedNames.add(name); - // TODO - this is messy + // TODO - this is messy // Not the most efficient way but the other way was causing too many bugs int idx = findFirstRecordLocBySid(ExternSheetRecord.sid); if (idx == -1) idx = findFirstRecordLocBySid(SupBookRecord.sid); @@ -242,8 +246,8 @@ final class LinkTable { public int getSheetIndexFromExternSheetIndex(int externSheetNumber) { if (externSheetNumber >= _externSheetRecord.getNumOfREFStructures()) { - return -1; - } + return -1; + } return _externSheetRecord.getREFRecordAt(externSheetNumber).getIndexToFirstSupBook(); } @@ -265,7 +269,7 @@ final class LinkTable { ExternSheetSubRecord esr = _externSheetRecord.getREFRecordAt(i); if (esr.getIndexToFirstSupBook() == sheetNumber - && esr.getIndexToLastSupBook() == sheetNumber){ + && esr.getIndexToLastSupBook() == sheetNumber){ return i; } } diff --git a/src/java/org/apache/poi/hssf/model/Workbook.java b/src/java/org/apache/poi/hssf/model/Workbook.java index 08f226318..d87fc2c63 100644 --- a/src/java/org/apache/poi/hssf/model/Workbook.java +++ b/src/java/org/apache/poi/hssf/model/Workbook.java @@ -191,12 +191,11 @@ public class Workbook implements Model case ExternSheetRecord.sid : throw new RuntimeException("Extern sheet is part of LinkTable"); case NameRecord.sid : - throw new RuntimeException("DEFINEDNAME is part of LinkTable"); case SupBookRecord.sid : + // LinkTable can start with either of these if (log.check( POILogger.DEBUG )) log.log(DEBUG, "found SupBook record at " + k); retval.linkTable = new LinkTable(recs, k, retval.records); - // retval.records.supbookpos = k; k+=retval.linkTable.getRecordCount() - 1; continue; case FormatRecord.sid : diff --git a/src/testcases/org/apache/poi/hssf/data/ex45046-21984.xls b/src/testcases/org/apache/poi/hssf/data/ex45046-21984.xls new file mode 100644 index 0000000000000000000000000000000000000000..96ef3ee7cae95500101441f443ba94d7b3622d52 GIT binary patch literal 22528 zcmeHP3w&HvnLqbtCYek#%`{DGA8EZNp$%;&+<7ETXhY|jwvaTnDQ$&7J4vSPkVz(; zOiJ0lrVpgBfMAP)B2Z-uf=Hn&FHuua0T))p2cnCF*D5Z%qN0LJnf-s~-aB(=GI@b4 z;;$$9pL5UmoyT{+^FH_7dHRP_hK}4b>qVAj&SH7&-DDxl*U<$$6S%Q5CgLFvcsH3$ zDn!Qcc*=Z)G;k8KPD15P0}Ox>pgJ!AOh6%E28w`EzyeGLiUBkMHU%gFrUKIe38(@- z2Fw6v0<(bGKp8LxmMgf5@>t;W;$AoR#+MjBoo<;4RV_`k@JP6Z@#hpp~r=xtIdnAfJ z-K>Ml(X9|CV3#mapth&UFy5#=)QDj=u(iy~JaAXB)vT4B4L$hS8u)$eTu{a`8 zXbc#w|I_$D;|Prdybqg@6p2UY+S{~Wl@Knt)EXa!aQp9H9t zv;k{?bAh$MI$%BUDS+ai54Rn-0Jsp?0Bi(;KnKtXbO9k?6R;WB0)zpIPdy_7^Z>m; z6zBuC0x_T;hyw$_HsE4lJ3zge;_raF6W9gp2KE36pwNU?n{Q-KR6SMj1PJ*eV=tmb z95bBY?cybbX-7_I-`-HHcS|tVCAD>Kxj48v6q7zcy00S z@{mwU68XcS{*Wiu5emoqdxCv^p&JomGUgnLg^>aFRBqqR(|MTNjo`e&{y4j={h7Jc zLTF91UUc(tS8=|2umG4`jV z#M_Eu4N63(=ELc4YOK-e)RJ?BABj$><_iChOc$6$#Z!%@&xMK}-pZz*J_5aP1bX!d z^aVNTv_|Ig(3Hx}pmazyFl!<&N$!+HK6gtJM($!L=I+#lfxD%Ng5*X%@?45l#aa23 zgo&*Qc7!64kez6?G#RI94x)Y1l0kD`i50ya{bvSN)mZM(w5*wMUuWAuPfw_~FBa_@ z=#H=Gjbn5_A(+?+w9PeF3G5v#@u`^-P>MC7?eQdb0w#7Eq@m1dSD?%u){NQXLOxbb z5=+5XOd&Z1j6IJ|?&!0mU_la84K~FIsu*=b;a9`v{GN2qEx43eDIRbC$ewa`y{MHJ9dVkxC*$KQLTU zzP~Yvm=|FtBL!}RVn#GlM>dVp&Zf~mDw{^q>uKNOD~vq8mwICSwU z!0k0AF%?lXLn@lpEnpHeB(tI!Qqc^$XvS2u89Z8Eb~Ixuno*558+qlYqDk&T_zROp zCJ^~y%#8-h-oh-Z0M)4UL^VpFqy#V12~dzqsgetjpDjQ^N`L}gG*c>C6{4Z|+0jg? zXeOQ9g{f%OTyC=_cRu#b2DDF2_cCWs%4APUu^pUUl*ukiv0a>P$z)qn>^jcQOS7HO zSZ2K&nK4x&BlI^yy+hHdRH@z#tROqu@Oqz(-1PNc2wy>_F#7tXREG(ZpHgprO1(%W zB}^rXXk?~zs+6g|(ukdEHKcubQ?aRXj7+C}QjorKNG8*}S_@YdWKPLs*2$DvX-1Y& zY4D~prAjlhj7ozy#a1dUUn|i`$V1&wX)w=XxcMGNr&ZXxG-GN14dE=Ygx$xd6E{L> zW)8yVDPyaE==AhE^crOs^^h=$DB9#R*08S(n_9|$#7yz`HRKz!hI~y@Yzs*@-0Myg zvFwI!6h94jWJc1BV`91~#+>tE>4rCfVbkFUs2eBgCWjHGV}5=Ej3=>{{Rstti~VZy z$8hKC6kl@m(JJE5q;n9#?!&XdYlK2mL~_#y(L@=)p;k+o(X@?~ikcSXK%IforOh-w zk7|_cTX2X#Q8ckAiYDv}cp`t18bS&8Vc5_nbnVCPHH6cH5O!=aoJ54+kHg=CJ#8QU z=saRHQ;9V6(vKEaZ}2$atcMxc=D;#G*=5nt_$aMyZ9_ac?g0VQi%yVt4b!VC=+j= zLb^np0gFnAvk?BKfN0C{Os*TQVzhBO-71JrJaV(mXmmYK|D2tMfbt;q*N?9-u@@-d zdo}O=fP|;yGe;hxU@vyxG3JTKW8sd0I15Q@y&F~rd+^XGmE$OkX=h%rmc|SV+UFh`a^OD->*MykT>HVP@)afhH>HFjI*qvUd zmI?I#^*5aA|9^viuO)G+|0|lH#;EB;?`n-yqm2}f`2Sz?|JeQC@sHd8AH$OU)cmhC z$M@}?>i>!gN9xxf&HnHF2=@PcI{&H0IVGL{9K!~PD&{=+&9t+Ip*>N_(d&*9N8PvQ zJpVC(&m&CO8sGU3j-*C6O7`P9{~=L7Ttoez?z2SkCT@V=Ul_e?xw&*pg5Czwt4AtN z?ogT3e}eb8TD2Q+6Q&I}XjZ`;Wt*6pfX-*d`Hu;HI?x<%{?{Vxt!f(j{3m|;LU+C< ze*SV8dH-`N|J?V_RQ8z8ER>%PEYVxWUH;D_>|^|0F~j>)#`XdGfdjy$z^8%BfP=v0 zz!d<=dnIrca5eB5;2MC=Yd#D2kotTb+|Q}c*Tcn{oqYlLB5)({CEzCDX5bd!2=Ha# zD*)BWzX0gx>^9(b;H$tL0QUauYrtK=*MYl%W57MYy})tcUx9A`bclaH@Br{2@DT7# z;9=lfz_)?#0E55@;1S?a;Jd*0fbRp30gnSu06zf!4R{iG3ix+`%KbFl9|6w*KL%*{ z`YG@;;9213z;nPafL{Xt0Z_bO!F>T30$v1O0$v7Q0bT|E6Zkdo8{jqIx4?e^uLJ)L z{15Ou;0@sSz#o7=0&fD8{-5FgMSXr7?qAjCci^6+XMlH90e9uhh`>GK5))bwSWH|( zr_;)l!qv|Il%betg75}nPb#Srl@M$rg{NmS^i1Y0pgp;ex#21&-gLxngzrjvrOd+n z7x4IW-e*$J`Zj`_K-#n6hWA%4hW|K8v%+-#;to+D4F&xD*t-qnaRoYJv|0LGxTtu zmOf3OZi7-U5dn^YMF!J}~5l4umSf!a?p*{;TubRn&Hrn?l`hDXS5Wo%Oa7T^HNjpO77yhw23B|;aDq-)>*F|3h! z_ZDGZ5lJu%lqS4P#T^hl8t^QFGlS@fLb8<-=$=V*z20X>s_m%D z^+-7YP1Nw`0CLKeP5?K;DGkyQ#iSaPkX{G72-Hrl`(|jSgV$m9mmm-GVK=)SIgy6` z%wK!MXM;q_bDFF1$W`9{Rbb7pw4=0k*5Ur{O4MvC?$EcPbp5!)-VfPu*o>NRa7r94 zBg$JB#gF5u17&W71_HdU2SB41m#%4DHM6QzJv)$36Z*5COXNdIYSDU{&{}L6tCYZ{(j9n@sg%S4H(inssz(fO0WHu}D>R|?hg`L{vGJ86vBsiq-!t!Tq&)Q#?o3FyZ)Qdy6LUc=;c*< zeh=D?gwPXd>jE>UTF4^ETL;jH*}zLklv>_GTquzfK58Kj9>RvO#G_u}hfTfGhgeM` zl$BaJeP`5yaJ!*N8Y!2nEl*Y{%{+_;ZM#jStl@p3520!A7=TOlpDu3=_}kG#a*qP( za1+-H_A503%V&mWSud>-H(uE?30p3jW0XI-{${}RpD*GT8C)}5D_>WqcLd5BPMa-> zu6)&zri}EYKQR&X-C2DuIcmg>-?8trl%j52_|uk)G$Puu3Y#X5Cw4MP*O0iOg`o|b z7`m>IeT{`Osm+neOk5$(4x3EUn{6^PMo!X!uLfkauHkae9+sp7BGJn-GV;htI(d?d zawIutSdva8N$2lmmZy^}Ld#2^ACZK^7vjm}JTWs|GD$~yWHwS#R_q6HzY33nkbpf> z1DGhCTt^d~j&y!fnmhzPh1w=g;4M zkz}9_zw^!=hi@-=u=z7b<~tv}?AUV}eaFEY?>R5|#gz|z^SkWVuU>G2M*o9-j(p+# z#l8n;M<00krn?@{=vP$VeW>jC_Q27;=l!L1SN~R{lS_>-GHQB`Y%;)`ffRbtiVKk4{Iy9gG4f{_8-wwdu9awyWS=B~a%=X12_ea;s7t!(<&s&J(qM=RRKGvf=AlMySk7-;j! zzskc-G?sS<>4`Mr|`T6BCQbW{V;woH^j8KS8ECR7|Bv6UKbl`5CPk=cVu?Y6ew{rFSPI5Pcj! z9g+y_JlNT0_%!sMDAMp)**T5q7dnq8Xx=3eVdb92RIRw2(K+2tBpY85;7 zo{*Zu9kJk*u}j!FOqk9J5+YMa$FmNw$@nsvR^PWJVyL}-=1id1G4ZJ1RqF^O0(M(K zu5S`Hnmo38X0XW)?1bulF2C&cKH{3$*3{yVo9BDyG)aRSo}70ETg=MsO9z=acmG8z zwt5W9x61PCyDHrE?yVJrMG1CO?G~ARB~f_kAoI+?PNdH73N$p?*dl+e$L6ZXZp7ww zxB@n8Lp)B|@AL&szNV&wb^cc0xrvs*ZhO0L`QTPjW@2-z&(qw7ag=Uuwy*VmI*{lP z7uAHecOC33n6_l_6A5Ll&$PvU6;}IupDo_^jFIh|`iM|+bS9hPHmAP9X4*&C*Myd_ z^1VhD9{ke2eu#_Zp(9cZ=e0uyyzV3(0x5it~>gPf}#4*lHi$2SIKD~I%h#Yb~)fFeUBz4 z?MZnnM$`cL- zQ)jHN6)578B-f~t)3d;yQ0EDf=SONNBWpnMbD0%t=xknCEo(OYa&;CMM$gZtD1L~} zb4E5IX3ozHqp{N%S5!`RQJ$&M@WmRE!wjDjx4_ulIQ)7cGWP0+-6NhNc;>u zV_aRex^>xV5B2NTmX_9)cxYMSTix1*2M9x`gB7TwagC%$@+2jMHQ%6C1Sy*ILrsXj z{v~s|s^H8inPi!o^#d=McttEJxGb@#Hr`^mRYqAFWR-PoyW(J+4XpPCnri~<1IR?A z8sO_|66snsTcz%mN_$YH*-3O5H@7v5M3S@AgJvSqqAm9l74;^599(r$(&i*SB`Fi0 zqXx4Q$wsX}A(3!LQ~^jhRVLnd8xpSh>9=fuJ%6_KTd(3hw*OaH@-lHL z0#V&%=!;KJy);>6D34|+Oi(H1wJ39xCZ0kk@lL2BDB>vszNXT)s_lHEUOIXKD^jJW z7XBn}_5ly6t4xuX=I{)4(d0!9i-e)RsZNR{4JJiyG*?sD=_-Az>h>wYXy^k-fSXtL z>^g5(ZG zO#*csW`)18e1~lJ)_J`RUR%B0=d!tEFVy&k(m_Idmb{#|%)>W_8~;5QHA zjl)WP#=PbDK!zJ#Fl~$;mFpOl>&)P;3U+qmmvb6-czg{_j;03L7N}pe$Y!_u0=9;x zx_X-|dpxpSyU68nHSMyPxmrSvigqk!eC-ozEKg4|DAMv!q|X<{uQzOulPYA8)`nvJ z*hynN>BD-uBNB2t8q1vxPKU#R0wRE~Wm#jn-{F^?uDZO*Xoh&Jt+ebc zc24jAg1SQ;5R6zm4`AI)?`r6c(+a#H+=Pt=t)+9ny~#|n3i(rSDgl|?Om1$U5rfTd zRF)C#J&Jc!f*t`=+!aZ|Hy}10GL$28SW&Y2(YNMV%62keaii4YQTjJDaOh)Xqa5( zZ7lb=JasNdZNOLOwEGq*0B@PEv3yb8B3FZ4U+)F#o%IcbMLsYs=JS_FvAqv$kB54@ z@B?8hdN)Ncu$WVWtG#iDv)<|O*STbykA^>td`-6cfXhiEpX_U}+cElib}hz|b#W?| z%u}Yb%YJ;Q6heZ-#_vYCT1x%gpS+CFH?- z(7PCMjQp2+GJ7t%2OGNd=0CjMy2X0uwI;T3{_*E%G5r8QOAq3^0nGY%B|f_zpcTQb0Nr^x2GFocJGZ%TOMziO$_Cn4JS;(3dUehh-&__Jzj((!{7#`8 z{KN(O`ELc%*gl#Wt^DFopZe*IHM6aUK9BNOzx{P8Kiy%ZH4K%16+rU_Hvatg0wZuI z0dat2#4RFT?gMaXJ%1%Y`Zx@bejWw%`XJglI7Hemd6wak*0=2bS!SLl`6+EO*)C4C zd03K5YdBm;)#K26oU91JDO*xAiLd?uT}R}VIDSNlr!;>Wx8bEl{rh0zSidSS!8y)D^)U!7m?%k?oGK%|u7?@NE literal 0 HcmV?d00001 diff --git a/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java b/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java index 0e18e21e5..363e58c14 100755 --- a/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/AllUserModelTests.java @@ -28,7 +28,7 @@ import junit.framework.TestSuite; public class AllUserModelTests { public static Test suite() { - TestSuite result = new TestSuite("Tests for org.apache.poi.hssf.usermodel"); + TestSuite result = new TestSuite(AllUserModelTests.class.getName()); result.addTestSuite(TestBugs.class); result.addTestSuite(TestCellStyle.class); @@ -58,6 +58,7 @@ public class AllUserModelTests { result.addTestSuite(TestHSSFSheetSetOrder.class); result.addTestSuite(TestHSSFTextbox.class); result.addTestSuite(TestHSSFWorkbook.class); + result.addTestSuite(TestLinkTable.class); result.addTestSuite(TestNamedRange.class); result.addTestSuite(TestOLE2Embeding.class); result.addTestSuite(TestPOIFSProperties.class); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java b/src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java new file mode 100644 index 000000000..7d1082e86 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestLinkTable.java @@ -0,0 +1,44 @@ +package org.apache.poi.hssf.usermodel; + +import junit.framework.AssertionFailedError; +import junit.framework.TestCase; + +import org.apache.poi.hssf.HSSFTestDataSamples; +/** + * Tests for LinkTable + * + * @author Josh Micich + */ +public final class TestLinkTable extends TestCase { + + /** + * The example file attached to bugzilla 45046 is a clear example of Name records being present + * without an External Book (SupBook) record. Excel has no trouble reading this file.
+ * TODO get OOO documentation updated to reflect this (that EXTERNALBOOK is optional). + * + * It's not clear what exact steps need to be taken in Excel to create such a workbook + */ + public void testLinkTableWithoutExternalBookRecord_bug45046() { + HSSFWorkbook wb; + + try { + wb = HSSFTestDataSamples.openSampleWorkbook("ex45046-21984.xls"); + } catch (RuntimeException e) { + if ("DEFINEDNAME is part of LinkTable".equals(e.getMessage())) { + throw new AssertionFailedError("Identified bug 45046 b"); + } + throw e; + } + // some other sanity checks + assertEquals(3, wb.getNumberOfSheets()); + String formula = wb.getSheetAt(0).getRow(4).getCell(13).getCellFormula(); + + if ("ipcSummenproduktIntern($P5,N$6,$A$9,N$5)".equals(formula)) { + // The reported symptom of this bugzilla is an earlier bug (already fixed) + throw new AssertionFailedError("Identified bug 41726"); + // This is observable in version 3.0 + } + + assertEquals("ipcSummenproduktIntern($C5,N$2,$A$9,N$1)", formula); + } +}