From af21f0ff8358bbf74c9d0f829b64a6824c541e84 Mon Sep 17 00:00:00 2001 From: "ismail.ozgen" Date: Tue, 26 Mar 2019 12:07:53 +0100 Subject: [PATCH 1/2] fix for the Issue #186 - when sst cache size is set, IndexOutOfBounds exception is thrown while processing the empty cells for some files --- .../monitorjbl/xlsx/sst/FileBackedList.java | 9 ++++++++- .../monitorjbl/xlsx/StreamingReaderTest.java | 19 ++++++++++++++++-- .../blank_cell_to_test_sst_size.xlsx | Bin 0 -> 8694 bytes 3 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 src/test/resources/blank_cell_to_test_sst_size.xlsx diff --git a/src/main/java/com/monitorjbl/xlsx/sst/FileBackedList.java b/src/main/java/com/monitorjbl/xlsx/sst/FileBackedList.java index 423fac0a..d4d61b56 100644 --- a/src/main/java/com/monitorjbl/xlsx/sst/FileBackedList.java +++ b/src/main/java/com/monitorjbl/xlsx/sst/FileBackedList.java @@ -29,10 +29,12 @@ */ public class FileBackedList implements AutoCloseable { + private static final String EMPTY_STRING = ""; private final List pointers = new ArrayList<>(); private final RandomAccessFile raf; private final FileChannel channel; private final Map cache; + private final int cacheSize; private long filesize; @@ -40,6 +42,7 @@ public FileBackedList(File file, final int cacheSize) throws IOException { this.raf = new RandomAccessFile(file, "rw"); this.channel = raf.getChannel(); this.filesize = raf.length(); + this.cacheSize = cacheSize; this.cache = new LinkedHashMap(cacheSize, 0.75f, true) { public boolean removeEldestEntry(Map.Entry eldest) { return size() > cacheSize; @@ -49,9 +52,10 @@ public boolean removeEldestEntry(Map.Entry eldest) { public void add(String str) { try { - if (str!=null && str.length()>0) { + if (cacheSize > 0 || (str!=null && str.length()>0)) { writeToFile(str); } + } catch(IOException e) { throw new RuntimeException(e); } @@ -73,6 +77,9 @@ public String getAt(int index) { private void writeToFile(String str) throws IOException { synchronized (channel) { + if (str == null) { + str = EMPTY_STRING; + } ByteBuffer bytes = ByteBuffer.wrap(str.getBytes(StandardCharsets.UTF_8)); ByteBuffer length = ByteBuffer.allocate(4).putInt(bytes.array().length); diff --git a/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java b/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java index d0420794..1e1278ad 100644 --- a/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java +++ b/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java @@ -501,7 +501,7 @@ public void testSpecialStyles() throws Exception { @Test public void testBlankNumerics() throws Exception { File f = new File("src/test/resources/blank_cells.xlsx"); - try(Workbook wb = StreamingReader.builder().open(f)) { + try(Workbook wb = StreamingReader.builder().sstCacheSize(2).open(f)) { Row row = wb.getSheetAt(0).iterator().next(); assertThat(row.getCell(1).getStringCellValue(), equalTo("")); assertThat(row.getCell(1).getRichStringCellValue().getString(), equalTo("")); @@ -510,10 +510,25 @@ public void testBlankNumerics() throws Exception { } } + @Test + public void testBlankCellWithSstCacheSize() throws Exception { + File f = new File("src/test/resources/blank_cell_to_test_sst_size.xlsx"); + Map> contents = new HashMap<>(); + try(Workbook wb = StreamingReader.builder().sstCacheSize(8).open(f)) { + for(Row row : wb.getSheetAt(0)) { + contents.put(row.getRowNum(), new ArrayList<>()); + for(Cell c : row) { + contents.get(row.getRowNum()).add(c); + } + } + } + assertThat(contents.get(1).get(2).getStringCellValue(), equalTo("")); + } + @Test public void testFirstRowNumIs0() throws Exception { File f = new File("src/test/resources/data_types.xlsx"); - try(Workbook wb = StreamingReader.builder().open(f)) { + try(Workbook wb = StreamingReader.builder().sstCacheSize(4).open(f)) { Row row = wb.getSheetAt(0).iterator().next(); assertThat(row.getRowNum(), equalTo(0)); } diff --git a/src/test/resources/blank_cell_to_test_sst_size.xlsx b/src/test/resources/blank_cell_to_test_sst_size.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..3d5906a79008e810a4f31e98f789415be5e5f7a5 GIT binary patch literal 8694 zcmeHsg;!kJ@^xbYf=hs)!6CQ>4em~G3)*;acXtWU!Gn8ncZcBa?(R;oUuS0CH#3>} z{(|>TuXWGrb#K+W=bqYC`|OgJf`Y~bya2!f003fuAUAKHIs^dl85#h<0Kh?N3Rzj& z11;@!lw7QVcG~pLpDf;FLPJue10ccA|8M*ke}N*9jBF<(YQT;7BJnvExuyCW|0Kuu z5%jkT$hzwi22v3-ppYX^tfokrr<=QXwkTjBeq4<8TXbpP{dM^1>S&^y)WROJDDTl1};sBcausv>;b(X2W%xsXTS z90p!dF$&A$;sMM)MNelc_dF8w5r`(noh1r1+(`96;diaz88)Oweztf48SRL5x(JlZ z73zx!P1m75H%uW|&2u7xy&2;9;msp}_M{U~s&X>vqDvZVm|i>AisAAgOV4e8Q7k~z z7q{od{0PR}^D`7c{%?qscb9Fy0Ed}07;p$+MC#ZAE$kTRe_sEG#Q$RW{blJzF(Z;t zOsGL8z9*>L%U?gipqbWSqq$-fpfFNYddpX3}3EjATu7BT)v1LnrB_v)b?>;`dgKvoz zyEms$NLI%1RV6b^E3&+k-zYoMMsa2ar|mHd znp{!amN2p6A%}SarF$#~Z&vkrUzXiL^jL(r8uUIFLS}WQh)!%AgQ+ zUQ~#kpz8B$C z*SE4V|5>;G0~rW#@dA7K?;hZgG3#VR>^bpk_8arC8INER{ra&deTL2rU_fJIqK_wa zA6uK&moAfGAFqX&=HGfknQ?S%jNgS#vG1oU|B4*$l45q*=_PN{Od};~*SzL?=%|+z zWV2JX>Kg3BIoj z;oLf+r;gh)XPio6Tp=dtB|bML_>SIJuc+-eR|MaAr+W&`EXCO>_QJ){=86Hl)Bg;a z%r#B1NpQH7!vFv%;3vQ#^VeX>R?@UeX2kJGt9tf!&fC_91BWkqFqUajVPU-ScaHYZ z^2AR`_KMHqkOWlMsPMx&NHxlF($yb>tR2icVL6`HdOg6ERY6z`B&7gYhYD5+0 z!UWS{FY+R$mrRWA88LjJ(1ZY{`8o>+uHo$lDi(*&&mnjV8pl}eBe`Qj@5Jm>5?~)* z(>4mtU(=ZWQ1m~5)3;E!87N$o(JhQT1nRAGHJHZ}V3*5Sn6ez`iV|Db1fC5Kg zD3m@~pLAD5^jtFUJ`MX2SDEE9R+q+=$4yV;FsN-3A7m4bkw6d>!ToWr!HMcBZHAYn zrIhYc%~w$kI%|~XHiK?8VeP1w7+Ff75HMeQl7NJRZ18@IaWvVpSq#T3OIY5z)eA|d zr{$!RQb}Uo6bW9feV8BnNuTn*dv*;;^lb5wsc)KTm?8ulGiXfEMd(K~PB6=WAp4e8gi5S#@4hX%UqrRJigHvpc7I&6S+Yfwjwv3)(n@BQoD~5m1zg#+NDS@~MA=iT} za+8B1o1sv6C-<)azzkWy>IdJU0Ovx?-vMA}qGt;wKTT-nKBKd+QC@@Rrn;} z$#2$;gdA2eH2X8CsutB(YNTVi3(|bcbtdQW{w-N^-FFw3f#KO9r~N7HNPHGfH991! zY>L`wdgSGUnQjcKIQ~z&EE#@8hO@WdVFzdn7ty(Qg#(R?>-QL8K5j!4aP08pz7dJU z9_kFv*g{tLP&(72#1$Z(p2QQ)kPw+>`W`}9Q0suX4ex7oE1HZiq z@Dq^Xw=2`nr5~yfh2gPdwqpO_L$q^xVB3fympwJE>9a(ykd=(o(7dMl_WHHNas<3j zOo}Tt=J>c&MlGSFX}V9~#~#-G zA+v&Xd@;5)%Cw_?L)MV3GB{Du{$jjzk(^wR%}jJEk~tboqGV^y(B0|RxRPWB zS_Q&SIXUON0k6@iC)W2wNk;}3T2J1*^wJ#g<)F^9S?6Fz zA+*=()@@5eXuceO%S<)^ryeU#}_KLP9}lQlM~ z?jZBqvWde>i$`40y1DtSYrzu2ew-&#D3t{A^k`0*$`QS4qpvj1l4D1*9%H^L^A_jJ z3cWAdW})y>o4HzHWAA%l0rLEzWD-2}32X4U0$5IHqGJ#(1`JGa|8-r zK2K4OuGkBYVTlG-RUpu!nG5w3%7t?z_QiRr03^>qT z2yo&k3rBDUdX7{$Fs$22;Irt2=_Aw);2bd?+qbc?#jmZc@gz1-?$}{CA!G>6ENX+) zAdW|j`AfTUD)96M29q&&ab;|LbMib5U~#xroZ?XjN*fKk5_K{5mdHQCqUA{i8m>rY zYndc>p~eRc@WWMaB`jiEu z`$kD?>4*nkVk&N5eOH5vA4EbvVDt4v!s#Kn)@}&?L>9lyj_(@ z79H{^jn5xSazYVHe@kEK6zm$vk#1;EcY zkZuhn>Uq5cBt@X5PQ{w`)TM6tb@g;IQTN#G15^hZhc%Ghhdm@NE=Hdjc#zj;2HCtt z)bvjA<0pz@c2dKy+WYEt7Y^;kx6MO@ejY9-RrlzIrv*Uo*_C;AA>m^9bN|;QRE%2o z2qY@Ec4!VIX65yYmikunJc#l*>Q}PTG3;1;l>MovE7Wd_FoUl#c`Un66!$GMK}Q%c z!xW2NWkq`#F18q*5*5ShJIX^Y%1~C%&liP86CZg?fy3sB7aBcR*H;2&iB0Z4{S+tX zyQy+b?ykoB zljY*c1J7M0C-wLjRC5zJdeTOC3Jy35I>>^UAC3J#&Lygi*{hAo@5g6oh}vwuWc>8% z6-Nq>vx3!~-e^NtcvrEVWWKw3E!N z&`by5NS7=~!&$>uMK}gzE4Mu3$}P8vs|OWJ)Q$re!(BIox-_DD;Vsic`Y>6~A~igV zaXX6p;f6Gxpt*WZ*-{0L;jX3wb}>t|MPu^tp0pEyJpqTrXgSC;*|8M@(q!9&{G+DpC`S9g8{tDQAv32+pNc2xv5SK8Cd71aK{P3?W@k>YG!pNE ztWQKXNUhR_@93bZv8D(5eGEue*V9v6?W9*JEJO@lp1yJV}Zp3d}o@(@0Twj8$%X+EkgZ3OA0VRyP zB#l)w8|K(IXXkO-9l_lqxutVq^VS|VIwfk@H+yVN>`LqxEn?pGgp+UmjH>4fUL{Rk zr=wnSMF>`E9$>mwTQ;-rI&_NKosQlhvvHWHHVPE4P)+tPSrX@e0o_FyWe_U8JxX7; zy!5;7EDHDel6M(>2f6s|sNwP_&HiUC&Ya~ufq()4puqwp(jPp;-o*m=8v)5y{z=0z zJ%7^h2U#RCLkValnGe~L3#34oJZ*}t+;nC~iBUiC^mR!?7NWzHm5S ziWZ0lby)8H+%81N8Tj}-VhFgCaor#N6>v>_9xqdrUh&QEed?*0S{+I+PWo^ybe}@g zcA_otartnc*EyC-d?+7qGw#)&WkAU(s&ix+z{Cp^d-R%Jn)iz;_=`IPy4jEXbxXxohq8$;NJRY}=HAuvnZNi?tLx$ubS6kXDbzRrSvXR_xE%CR$z%MVuB+OCk&9 zt%Md#xYTsdj$VKhraKYF3wps7yNQsU#);R7jTdud_7kHh?<)89nWa0??oK^6Jy7mF z@13d+J9G4WZ7J@}uGnxjGo;g7Fkz0(eS%h;&awXuE784beC*&u>pfU0#Q7a7KM$>T zCP1LQ9m8+aZ{#&xNhWIH4QjLMz8C-E42RAVS!nY-MQB5iq%z+KPLu&ABmL*7#~T9c zL(5n~+j59jmy7M8gsBO28e8atY*S_~$q$n0Xt|&v`YgT{E23=>$4GJv%>=)wPWxEi z!(-!A-)!?Y2p)mJ9$?jVi#GGNFj7)uSJmihe81Yzd&{H9`e{f8gU>1!DY--m+Z7oE zo#V@$dWDiNXK`#sM{gs$eAfxD8Z~N4j3?F!xrUCOMtR|~G_$F)W1iSzK4f;E17~ms z5CR<1#1!dWkL&6)W`qm< zFL`6kGGQj!17Bt_lJbVc>0OT6cpgh0pwq|nwP|}_XuhVRMuXjnI9BfVeSwu7Nqfty zZ77GKWMlQPi5_*r>**-Qa}XL*V_COz*Pn#Z?ju@XvbA=DLl=F~YLpHS@dLk%339-G zcE}*LWEPE^j6OK3Hh8Qx_=--S=M9b-u78Gr69U3)__`hHyLxoTu!N8BjUt03{tiNO z^)u%L?%R%(cw(4JHfDs>VP7_3dqJt)bHcb(N= zp8r);6Hu)TRt4X&0S6o!SX48#GLW~mvbJL|u(AdI(FppVmIg*)c#MKnCnILS3e2-m zpIdU3vQ5U3Za-mHhHlWhrnqITZZe2${%9rHQS)dNmdI|TFW!#6Z@V!Uxus35Ksjr-USa5bjE zK+ho+lSQd5U4t29E=7f(_X@U!tzD=1N;uM{R6}J=D&fsI!IrGhjZhh|d=n=RuIbrLd7OVr|LnJ8T}Yaco6#$6@`>`bV6tz|ebp0%%|N7}q#88*>ODn8 zyQ+RYm59%>|NIXR@?%Fn`j%i{9Csvo=b|n&39Y@GlSztz;1IeG(R4h@nZQUI>b|O3 z2s+z^9a)SIuqhK&@foH??FYK4T$i*A1Lu`pP+6trEy0vROq?aIp5IIf2HV7L8Vv^9 zG&~~4)S*&z3apAHd;~kY>2MA>;aO@?Afp6_B$9TvG#I)aX%1upscRCPe;92`i zwqQ;ob(FOr%?fK=HSLdCi^I(Ex%k9q-Xi%!sgb*iWAPZb3A`gWI3%$dMkaHqN4qX$UxZ!PS76ZPw>kdxXH^Kdx!l$+xDONU;6FxQh#^w_lDqq z41dlsU~K%QJ@~8P-@7D#HLQ8@n+Ep(>6!fM=U3_cPfvSb4*!dg{;Tn?vg4n|`v|`o z|3ifQ)x)pi-Jc$|5&u=n`_;j(CyqZI*n)*n@cY~Szw^hhroR%fKTT7?gzb-||AUVG z>gDg1^-mw*1}`oE@Q*tCtNGtk;a|-Y@&97}M`Dzhf&uR?0Pqt0^9Gl4U80}={tq1c B7Iy#u literal 0 HcmV?d00001 From bc0a861e370de68e8053bcee27240c0dddc57c76 Mon Sep 17 00:00:00 2001 From: "ismail.ozgen" Date: Tue, 26 Mar 2019 12:16:23 +0100 Subject: [PATCH 2/2] reverted the settings in existing tests --- src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java b/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java index 1e1278ad..b9b3b645 100644 --- a/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java +++ b/src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java @@ -501,7 +501,7 @@ public void testSpecialStyles() throws Exception { @Test public void testBlankNumerics() throws Exception { File f = new File("src/test/resources/blank_cells.xlsx"); - try(Workbook wb = StreamingReader.builder().sstCacheSize(2).open(f)) { + try(Workbook wb = StreamingReader.builder().open(f)) { Row row = wb.getSheetAt(0).iterator().next(); assertThat(row.getCell(1).getStringCellValue(), equalTo("")); assertThat(row.getCell(1).getRichStringCellValue().getString(), equalTo("")); @@ -528,7 +528,7 @@ public void testBlankCellWithSstCacheSize() throws Exception { @Test public void testFirstRowNumIs0() throws Exception { File f = new File("src/test/resources/data_types.xlsx"); - try(Workbook wb = StreamingReader.builder().sstCacheSize(4).open(f)) { + try(Workbook wb = StreamingReader.builder().open(f)) { Row row = wb.getSheetAt(0).iterator().next(); assertThat(row.getRowNum(), equalTo(0)); }