From b2535a5b586a5b5a440b6d3c822ee7c45f8907d6 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Tue, 12 Dec 2006 18:21:41 +0000 Subject: [PATCH] When sorting the contents of a directory, do so in a way that doesn't upset Excel when it comes to Macros. Correct logic from Bill, and fix+test from Yegor. See bug #39234 git-svn-id: https://svn.apache.org/repos/asf/jakarta/poi/trunk@486265 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/poifs/property/DirectoryProperty.java | 33 +++- .../org/apache/poi/hssf/data/39234.xls | Bin 0 -> 24064 bytes .../poifs/filesystem/TestPropertySorter.java | 152 ++++++++++++++++++ 3 files changed, 182 insertions(+), 3 deletions(-) create mode 100644 src/testcases/org/apache/poi/hssf/data/39234.xls create mode 100644 src/testcases/org/apache/poi/poifs/filesystem/TestPropertySorter.java diff --git a/src/java/org/apache/poi/poifs/property/DirectoryProperty.java b/src/java/org/apache/poi/poifs/property/DirectoryProperty.java index 5681ad5f0..e0d817cf0 100644 --- a/src/java/org/apache/poi/poifs/property/DirectoryProperty.java +++ b/src/java/org/apache/poi/poifs/property/DirectoryProperty.java @@ -127,7 +127,7 @@ public class DirectoryProperty return result; } - private class PropertyComparator + public static class PropertyComparator implements Comparator { @@ -162,13 +162,40 @@ public class DirectoryProperty public int compare(Object o1, Object o2) { + String VBA_PROJECT = "_VBA_PROJECT"; String name1 = (( Property ) o1).getName(); String name2 = (( Property ) o2).getName(); - int result = name1.length() - name2.length(); + int result = name1.length() - name2.length(); if (result == 0) { - result = name1.compareTo(name2); + // _VBA_PROJECT, it seems, will always come last + if (name1.compareTo(VBA_PROJECT) == 0) + result = 1; + else if (name2.compareTo(VBA_PROJECT) == 0) + result = -1; + else + { + if (name1.startsWith("__") && name2.startsWith("__")) + { + // Betweeen __SRP_0 and __SRP_1 just sort as normal + result = name1.compareToIgnoreCase(name2); + } + else if (name1.startsWith("__")) + { + // If only name1 is __XXX then this will be placed after name2 + result = 1; + } + else if (name2.startsWith("__")) + { + // If only name2 is __XXX then this will be placed after name1 + result = -1; + } + else + // result = name1.compareTo(name2); + // The default case is to sort names ignoring case + result = name1.compareToIgnoreCase(name2); + } } return result; } diff --git a/src/testcases/org/apache/poi/hssf/data/39234.xls b/src/testcases/org/apache/poi/hssf/data/39234.xls new file mode 100644 index 0000000000000000000000000000000000000000..20fc9a0011870caf1cd439324fbd16d665e13208 GIT binary patch literal 24064 zcmeHP3qV}Oxt?=&frUU^2#HBZ;tAwI62N@`0S$&_Ss)Pt4dl^^mn^VJFbj+9qA}*N zNwl@C*1TG6wQa1`YIChoTdnpX=Gvy#rar2@Y9A&>TkWl_di7R&t$V+J<_vq5EU;i} zwKoIbnK}Qw{`qI-pXb@fAIm&`(_Pa~up#C=W@7I}QdzQruE0&;#>$uqhY5Hu5{U>R zV{o1}A0Y|+4ZK=VdD(y&NCK$NQ-D<93?L1d0HgyMz(fGm&!z&IKo&3wm<;3tQ-Cvp zX+Rz@9moM@05gGEz*&F>%m#9SIlw|-E-(+64=ezvZ7tI60@#JR`y(Xc4>hYRSpa__ z)P;xj!0uz0LBfffDQGdWfTRyJ3q2Vd;7?XV)@zF z$i0tkV(pxdO(ND9o>@#)sI5sJ5{;@-ZX3gh+>Mm6xh>Ghvh384;~AO%z@gA7`4jI_j}Ptt zlpcvGUjHY(KsvzK|Ec^W2igBgKadVcr2mtiARR#cUG{&{C8PsLFOUw9{l5tAvjH1m z2ONMCC zXat&oj{($5nt>Kz6|fpu15i32hy4lQ0-zPR5coHs4OjYz4Lf+kqWG7)Z6C7L$|MA15D6 zd1L@i6JsaPA`Y7$;_c!EglR=i>5P5cw;|A{ogdiZ4_u=CqfHg(!K+Z)96%$jP?FKo zRm?Yj0m=i)8N^Y`#;)L+K;NeJKw#62@SKR|MiHPC@SMb-ck(2@&z}|W*^Ixl&TW}P zjf18$Yt>2~?i&8g<*5kZ%K*^g6~2F`q2pgQNFXx zGoWCh;Bx2W<`v}S+1eJI+nO(5S{KYR&&F_#fmM>=f5A#$=Y_F>axtFIj0P&Q#ioOI zN@_l{u-u`chNPwkU5rkNZr4VvEYu`t`fF=GC$ug&S105a=7O)b{K_`uQiMK@Ufq^& zjJHrTMp~%F#8`@+k3MY7Oq3utKhByV%cV=BnU+Ff z#;aXC1~j5mL*Vo`rPs)GYLoH84@aj|Lti)y zJ@pj&e$~vZVN-PYS1S8~T&m+2N$fI_L_8T?q~JW12yuPd0D#`9gN0 zJeO>sIEeD66a&S1H74E){vJ{+0Mv4s!rnz!lqLcVWMLha$$+GZftKD>8l8m}x5jAk z2`V(Z!E6T`8Ce@ zwi7-NnX}=haep@teFcBsM4AL4s9*{T7&XV23OCuS5Ef-pX+>c)=c!rjUM{vu19>KR z#_&W&C}GC_3`vW@{sDzAR)yFTmB~I-#79adyzrI$DIN*S#F3~>ZbgOsKaq)(Wb*!u zGDSfXM04y!8-EdLm7zb~5BE`E6W0ZzwH~eAyB{4Ql83NYA>6%iE4&$FmKGrdOTZqPF=gnz7 zROe#2?AZQ@8_aP^u;+#TqYR5BN8_~L`zcNEa(7xV5Yl84W~arRh9*u;%nUj$+ouve zoJ!9@5h)5a_Ai)8Y>eG!)|jT|vwS^~0K!Ruu7yAGib{ClFmr`MeO>MSA?DLoSGBG1 zcEeGj=`Y-?p{ChhJ4k&zOGA5YSzB0{$ME3-Br59Ty1hRg{OvRXw8=XcJ0c& zZH>LY%AWiM8EG1AEj82^1w-CYe{L|j-qX{$^URDihW|uO->R@4lx*pq1-B5jv`uI% z!iX6;V+RfI)V6O=5`-so`yJgTvG3H~7VPI!bD6D=-d-L>VaxP)3M-AmcGJv*nbxK9 zwnjSQ!($Zob9ScA7wrqu_>Fd&@@9l~%lL5y8a2`9=<6PuW$59!E7pyoPNE%1G}Ils zcS2VD`m-XgsK2PxD7i|&435Oak@|^g9k#(tG;Rq45;xQWV6as@toAe9WR4!q}5JPUFOEdI6 z5xOys(JkXjTj6{DV!V8w0*=?vm?i~d(OonYQ$|phw+%Z6VgI2L`#v=Iyj*4W2GtNB zdKuLSN$=ug&nOazu0O`J`l7R7RiQIs*^WlZj(gK!F4rH?{KvEU68X#87nL(Q z2OACfV*uvoI-|#m9|Dsbg_M`%A%3V>DmA3Xbk%BH|Bh!OM(a zzc!DGd0PJ)#r;0qq$f}7|EK_|K}O%AF1?`+`VAM3VtiQizvCmL|Hsh(Uu`+v|D(Hj++-d(_M;0oYM;FG|o0NT;}v~GV^x37WyIpA7g53m>5 z2jFaj9RNNLd;z!)xE}Z-@Fm~|fYSRi?5_Y{1r7o?0bc`d2EGp50^ACG1Go*i9rz}2 z2%z|P!2T9+CvX_J3%DD&2lzJd9pGNzKH$5+{lEjj_kiyMKLCCRJO~T`4*^Gj{{S8a zu*PLS295$h0UiN<3Oow@3^)e-9C!?P9C!kF68Hu16!0|gOW;3&XMksc=YU@UzXqrb zzlBY!ljFb%;053$@FMUM@H^o50EK-8_8)-%0{#fR3j7K9Gw>SlI`9VY-@u!|TfqMS ze*xYG-U0p!{4ek>@HgN+0@9{UO@4kw34L1?1vm^M{30*l{Sp&AU_iq9|Hy3e(MfU7 zugvi05yt$mA_k?L1f&4O0h!qIDzG%N5!olQ{wG%dxrX#VP9zNH5VZSFG<6B>P6!wA zNa7Ide^C`AEJfLjrv^|6c`hJA3)(vP{UMyMpA$qP6ZAAet^{~I8D8D3@o>a}g*f44BK~Dw92s=rXmhX)Udhcjkj3U@D&(Zcjqy3Z;AW73@8DVH4c-Q{^Jd8q|`bf~Vf?WF zz`qSJ!#~mDe+FTz_4HQZ*6gzPu>+Ha{f{)HB?k>X*%7U);rmOyYU^>6){EOvK7NaK z#AtCn>8dD|Tbj6mh7fPW`?Grf!`(k05@py|+`kxe{{N2D=B0}yCEp*soeu`E{}DZ< zPbVH|TxlLWuODvzy9TGUHMqC!L_2ce4tF771BG06xJt1POti%aK`uJ4r?YnlkK+X0 z4p#}k4K3QOqLdLoVVV~uehyDz>$o5V=R&3;Me-vp<+e>raL~cfEaQrkf63gTBc#89#KzzPH8?U8l@-_gC^W*DVt~iA!J% z`u|CEnSVL`FWb-uZ=de}!|MfVzp_S*Zb^%^MQeXvSmS$+(*F+++yCnW-pWp2U(9|7 zGypoe&;YE)B{TqYEfyMp*&IRxm`mj8SCxP8^vmj%r(c>^o_~kzP{6tV=w;voa)70 z#4gZ#5Xxw*3SB>nl>^;^bG|P**uiHBcqCa!IrJg-4dBa*ubqg;E^y`pH?5Eb@lF0g zU_Ch1APc$>MRXrRG$MQrmy!p(krX|+li|j*9yyWQgPh`}%!J z<%199RgZkyIcGz^ll*SnR^mh&#;cE4 z)jpjcdy$$C<&IbT-Mpm~!Pk$!_%&~ALF1|W6le+YhcDF3su?#ev3N{Y6-GC=(_mZq z?uwiX${T%=i1>Ck`UNL#uNmj_(NQ3}4vs{U^|3)QR-NuXTAPBacEjE3&PE-adSx&Y znXC>UClaBH>!c0W=}TjbizfZE}B#_ zUC~4Di7lGEY#E7Ystgf{9GNYmlbqOAUMLGbaS;pL*aC?JI*_MRjfROtXpe!+W=dkN zE*Ik(m6C}ThLR~pFO5V>P#9ylNQ9=VWcEapqG^Mo90q}0R7$er8Cliw{KiA0CNrLR zA55J&R=d-+j+_k4&fyJ*Lx76P7yU*Fb@Y1d? zOZxt#ReR6A_|57EzqaMju_ZJ2O8O_R>sxZ$!xlkgHOt?pO13_vuAUM&mUf7WhV3LD%W3Lu!6RxtH)mY z(01MFbfoMc?*?zjJ$GA~nZlLck%w>H%uRiPi+mj+d)k>+mTV4&Is<;+{&iNCMqzZ% zA3jKajqBHUb@(RtTUol<__^`clU9}rt|BIdwRtY_`unjGo%$?A5uc#FJblpZcqQs~ z{sHb3b3>rB-|w@xXA*RfQ-98W+KsDi@LfoR<(`I`n~4Xv-y00tcOaD6jOBX$wY*@MxMSYlcLAc1 zQrH9Wicu;{HKDmohS23mGz}w|%5qc{^WmBBmB08fTQ%d}7gbSW23vIqjb!k2s&(4f8y8+Tg@4Of%sOjEQsAR%Tk*T8o?QVO(u5cH(lOWv1Ee#(ZO2C~PIKe{15o2Pu*^BU_tnhVQh!1k^FC$W-Jsd+ zMOK^E*uHZ>tM6LZ-skPR3_rx7oRPWf7b$GgF4NK4Y09o|=xUkqpMj`lQ!~SpF#io- zgpV3s;k)^d%Y;pVRRhZOpHFZ+H(|y=7N%r%Rf_${r5A^}1Zhc~9jykL5T&{|L8U%z3rt(HvK}l0n1Q?pm?@Fij=H)67W@OVTxs&aIWH4UI=; z&;Dk~P56=;s(E07<+!?z&luJ8msH1U_KF%vpWB=6oii`rv@qjDSZM9ZC+F24TA{I7u6r_l0-O`AxpbbCklBC!!ppEeL0GXmAx$q`G6A9qL)ud4 zPohaz5_;dzKm01mDl8fJz#T>`0>W=cx7!YK43PTvF8wTTGQx?IMy#ITCR0w?!aVsx zY0@qAuT)<1PU(wGkmiUrb@UKjYI=QQ6*P;YYnxVO@i7;%*4}(I@mn z`|t@7DXt&gsUQ$YV-fMUq~i$3UYxg^O48nDP7+I`V=s zvv>3q#C!CaO3~sNl2|;mK-Y;w{SX|hi`buAL7idj#_!(npE-zmbHuzk!5rocjQRj9 zlG)&R*bzd~ra#ka8D@Y#Sq-vM&xRYLR@2JH+Lf;D_EvXeLqp>VI2x9@S2i}ofiaa( ziifAPsJgyh<^v+~Nc*@h6slM%FEy&2I_(wRUI$v#5HFKx#SSyTpR5K4;3@`e&P!il zL!Z=Vtx}7HG%-VzI?UecS?l)H7kSotNDL%VK2s%@J|@!UwXEBxf z#FA}@c+e|Uy6>S6>T*oSkS?YRG_1E%-c&t;1i9}Z2E2NMAerd-u}|x?Cv-ckXDuvZ zp#Jf<3;%jSj`asG;U-q~Z&yNDs7v8Z^%El#q2h(YHwZ{*`KW+|j`vFzh5R^sR8KI1 zJ;BeXby}w0Hus1i`jgmuZb+38ZRmH*L>u}UJ<*1KHczyyY9X2dkq5rKWLTo(i*@~n zh7d;0Fue2WIRa&*<72T(5eZL8kxC4)@K1Fj=Hv8+q)6@8OCO=#QMo$?lNrr%>irG0 z?d{w2Bp_8?3!SQ4bS;bEbmocG1f#8Qawo%10W1K0yO#z`0MdaBU?M=_tguPnW$EwP zuqOj_rGmb#qcGES8_v__Lnr}S>qU)d6LNjWFZ-p+QAB?8DH^oGps5mXro%R4<&kQ2@S4CK$Rdhz_Lky{7n z!H2vkDF$h>^Y*tHpM|&*`Yhb(T9F z4hLE=0=OG$D{{-r%3bBfE=PHp5!3;isu0}fbU5A48fUct=(X;OTz8q<@FcV z%0SLYyPz%*4B`4?$k&72Ds5TM`oM)5X@fn+UU9jz%;~5uEw&Z9YszhfcDuW#u*`#A zXtTL&?sB``W-D=RI~SA0a|dJDc*-=M2l2VT4+$FkV+D#*pz+6IXGwupTDrJE%SaO) zvI1U%|8AY?h)Z=2rJ@5OZA!X8D{&&x;?iw`PVT~gN$O1?$(tQ64fMz;0=c4{i_tq$DirectoryProperty . + *

+ * In particular it is important to serialize ROOT._VBA_PROJECT_CUR.VBA node. + * See bug 39234 in bugzilla. Thanks to Bill Seddon for providing the solution. + *

+ * + * @author Yegor Kozlov + */ +public class TestPropertySorter extends TestCase { + + //the correct order of entries in the test file + protected static final String[] _entries = { + "dir", "JML", "UTIL", "Loader", "Sheet1", "Sheet2", "Sheet3", + "__SRP_0", "__SRP_1", "__SRP_2", "__SRP_3", "__SRP_4", "__SRP_5", + "ThisWorkbook", "_VBA_PROJECT", + }; + + protected File testFile; + + public void setUp(){ + String home = System.getProperty("HSSF.testdata.path"); + testFile = new File(home + "/39234.xls"); + } + + /** + * Test sorting of properties in DirectoryProperty + */ + public void testSortProperties() throws IOException { + InputStream is = new FileInputStream(testFile); + POIFSFileSystem fs = new POIFSFileSystem(is); + is.close(); + Property[] props = getVBAProperties(fs); + + assertEquals(_entries.length, props.length); + + // (1). See that there is a problem with the old case-sensitive property comparartor + Arrays.sort(props, new CaseSensitivePropertyComparator()); + try { + for (int i = 0; i < props.length; i++) { + assertEquals(_entries[i], props[i].getName()); + } + fail("case-sensitive property comparator returns properties in wrong order"); + } catch (ComparisonFailure e){ + ; // as expected + } + + // (2) Verify that the fixed proeprty comparator works right + Arrays.sort(props, new DirectoryProperty.PropertyComparator()); + for (int i = 0; i < props.length; i++) { + assertEquals(_entries[i], props[i].getName()); + } + } + + /** + * Serialize file system and verify that the order of properties is the same as in the original file. + */ + public void testSerialization() throws IOException { + InputStream is = new FileInputStream(testFile); + POIFSFileSystem fs = new POIFSFileSystem(is); + is.close(); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + fs.writeFilesystem(out); + out.close(); + is = new ByteArrayInputStream(out.toByteArray()); + fs = new POIFSFileSystem(is); + is.close(); + Property[] props = getVBAProperties(fs); + Arrays.sort(props, new DirectoryProperty.PropertyComparator()); + + assertEquals(_entries.length, props.length); + for (int i = 0; i < props.length; i++) { + assertEquals(_entries[i], props[i].getName()); + } + } + + /** + * @return array of properties read from ROOT._VBA_PROJECT_CUR.VBA node + */ + protected Property[] getVBAProperties(POIFSFileSystem fs) throws IOException { + String _VBA_PROJECT_CUR = "_VBA_PROJECT_CUR"; + String VBA = "VBA"; + + DirectoryEntry root = fs.getRoot(); + DirectoryEntry vba_project = (DirectoryEntry)root.getEntry(_VBA_PROJECT_CUR); + + DirectoryNode vba = (DirectoryNode)vba_project.getEntry(VBA); + DirectoryProperty p = (DirectoryProperty)vba.getProperty(); + + ArrayList lst = new ArrayList(); + for (Iterator it = p.getChildren(); it.hasNext();){ + Property ch = (Property)it.next(); + lst.add(ch); + } + return (Property [])lst.toArray(new Property[ 0 ]); + } + + /** + * Old version of case-sensitive PropertyComparator to demonstrate the problem + */ + private class CaseSensitivePropertyComparator implements Comparator + { + + public boolean equals(Object o) + { + return this == o; + } + + public int compare(Object o1, Object o2) + { + String name1 = (( Property ) o1).getName(); + String name2 = (( Property ) o2).getName(); + int result = name1.length() - name2.length(); + + if (result == 0) + { + result = name1.compareTo(name2); + } + return result; + } + } + +}