From b00c47a0471f64b379e2c1c5ece6a75875f42ec2 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Sun, 20 Sep 2020 14:41:24 +0100 Subject: [PATCH 1/7] Add a failing test for reading local extra fields. --- test/data/local_extra_field.zip | Bin 0 -> 9466 bytes test/extra_field_test.rb | 12 ++++++++++++ 2 files changed, 12 insertions(+) create mode 100644 test/data/local_extra_field.zip diff --git a/test/data/local_extra_field.zip b/test/data/local_extra_field.zip new file mode 100644 index 0000000000000000000000000000000000000000..5a936e4c351f0676c63eaeaf8bd39453ba5c2022 GIT binary patch literal 9466 zcmZ{~RZtvY)25BPTae%m0}M`Z*Wm6tI0Sch_uvk}2KV3|U~qSL*C0XmeQW=!|J$wF zrw^X)uDbi^?y5exR1{$0@Svcekf5UcvsAh+gUc77p`h5{p`eie)3da(2eGrddAVtN z!9ktCp1_7{%sS=(@%*>mW0<)yb9kPEA2hDk+^RzPw&8? zp=K4d|2h78CB!58&}AqhEu!_}(-gm~OtiHZYY(N>=|Fr4;ak@m0y+$cbRo2_UtXo2 zP<9UQz>6}mwv44d`x5`Ut-?p`X_KsVlZEPI!4#lc@?}~o`TA$vG$ed zTVK3{e1D5r&=xzrcWZ#sb;r{Mvi~M8Xe_}r+~cQ4bkY(x1~`-n^BLL8xn(G~YEWz} zVSDL<7u2#Ve>UX~qj_&+0Hm9|eP5VRdCGGOed(U^aUZfZ_;*BI@!FeFCMlmm_@l%# zc%4w77nw_hI^!2bM2j2!!1{RXAw682W2Gk>V!c0N-$|9ScYr+EBD!z1AdPFWzWsPp67|MTDn~8{7e4z(WVAtv z27>!!g^-&T#VjEWCBlGeH5Y3#i68YVYRX6jtT*Vnj9Ar8p0&~%Wl60%&7|L!4wdfI zmrssPR~mBo&Y>N)o z^BWvy3+h$*KP+Iwl7TKr1qFpu^#5Cc<9}M| z`J*@bLC|{6deR_vdYrpCO+F1hCHnUJs+0bphnLZ*G44Ccf1Z$IjrNe?B>t?=pq@?!2?pFdx8)V~?R)%yNI4 zyz!buQqHx`fn&b=HfaX{hW%nRy_#D90HrlhHO-rkHS+YaOOWNBuZvjh+rRw0HxI6_ zmsfl9Vmd=CBRqaH)tSal8i!nfRrmaEh2nM-%Fwwe!{I}&*g}psUgF$@sX$UPbfuy% zb-FhSf;pfI|McnNc#&QoHxf7O80hf%QWIpeI9=?-CN8rkn)vhJ!uJ%>5Z=bT`Ps3X z6Dw=kwzYrS>7^=DW5WqAuM#V?*Md+QWW%^Ya^p0)wB-99NK$e>t3@0m0dOf0J{DAK zBfK#ji*W^tk~mj}jnZXj;ubtDIP1i6Q=_;>+-Lp0iT%4Lr`B?nULIBVEYuVbq8b!Epz8C<;GUta-2x9CCulB6LI;1%a79Hg7!+Xl~`S&d7ochsPPtx>&4)H+*c}=ZNKm>jaFzyztI-20k<>UR_}ii%=fptB zG$}dAnJ+(oU!#QXCb8sYQ7ZS^VO;E-5$kU`R+#ehgPh=kiy}4b-`&tOf5RK6AF*4T zJ`57r5~D}h$9Wp{J*?y61LlxfDL*+TNsmPskUb6^CjnYQ0p^_`ME7l%pDe=}ui`~L z?wCFGa0-rsqumkar;se;Rr zpCHlKxCG@7WLjLp#T8<1EVCJ$d-jRccdplOtYV9JA;VF|>c#GnJ0@){t*vD?9AxmH zalAt?Bm)-3x-Kp|!RpoQfWXH~A&UW4LU6)sS9 zpk@P<>IGI*b}nYDWpO>=^D#DMq9=;2Gb=L#Ai33i zPK_Q4ePx?4S4N=wW{ZQc8?69Iiod6+52G|twUqYxQzk$6uJtzJBodKht;YBq)el+O zcxQz+n$C5m(hqNkeyqz850iHQa=!OLS5cyT7l9U?zAkr0Aw_g1M*H~Z=j6sv{T*v3 zLzl8La1^PfeB{*RV5tID==pL89^ap@-alG(2?ET(qz)y zdgB7HEx~KvHBV0UG|JmiWWY+`tG%zQ8oL;Fx(0I&%`HHDxutiQ)iG$_KE3j096_3e zbs%;XeQ-u1vlL!pbD~nZ1^9=UB2C^LoUN)RI?T7$5>2JuZFbBb_rT&EFI}tNYx#f- zpAabKjIs>nk(317#nVMGEVrd+cIdp~jXvLWWO1DqGfHUGXud-7hT4mB_j!?0p`Od< zlPhtn6JKU1DBs6=b57-_IQ1qoDt@?z@8@W0{4pU<@sS!0BQ#HjnffSjK`>++K@*hm zJr+C4L8nnSZg>1rk+qipNr$=hP*Fn13drP?4`CF&#$a9{u6 zpP$ySWMtI4#*vpcv^%(n4vSb7wy1|d74|M!v|$>|9uBS4V9Gys2d)qMhMRP{0-Eme zeLZO4^6+nRkdda19#M^uSi-POAiFZ8iQ*unHtoULDo?oDiaxN?79qb`n{hqhkzdNw zJ4aFvy^!-9#%eWOE*hwEg=%VQh@n`Bi8neP6Uc-$)5+221whU=4zLm|5gBx?qN743 zlM}NZOH`jFu!_lyvHTuwsc|r7<(ajk9MR;F02R^iZp-kDlEWdcAq$4gppl`J;?Szg|Qk8w^)niCy9IJUE(g^mT zBJ!@-dlHA~RdL2!HwT&p^cs@OJZb<*8RES6YkJ*``CF>S2AMCwfX0N#&MwVa^5*2L z=9&%om9*ftEczl@{rA?^#Ti|{7Pz_iX{V~auvVI$D`tP8k60iSL{ax>b%0^*9;Np) zp5NfEXqiTR9AV|`=Zv7JnZT^6j+tPhMXWXsoK`%mp?-8x3Y?7*a&*%(T6m+j8D|2M4%n^TVI^vS9jU6pO@^Dk)agY>${ zY*#}|KD?@Eft%E^)gEPWrc0UXso}p!){Auh^71FlYftJ|{=BS7@XMk5J)7l(F4x|2@}(n)-`5}QNo_w z%%yvaOe^e2Xr|ZB_AV9i$xV0CH;=(eUn?i>$AjiAJZ*Jx$^Jl?jJM5|s>VqpnknF> ze|MxvHbeqWK!;pYrVQMZ#7jJ2IxeqVyszE(BT9!$7KOs!LW(C`uoh{C$2;VR)!1dZ zYGb7r$!6r6leCZLHPGo4=hXfnSJ!jnQB7;+t_E=yv!?-zj_a7}&8hShn4lySw|v|P zrIZb5Pc@~eri%DAPca2E$9TC~Ev>4Y<*k>?0vUTYSvAt|<|w>5RtigHm3OeZYVb?K*;{|oR7D#Zr>V?kPAVvtWn@6Ge61`y zEcz=#<#=Ktg!_EMR-k;e_21e3=jjVvg+H~;E%aJdPq;1aAu0_mKw%|ERRi3+zm*fk z_aNEJPZ^;7KUR%VBDnd@8`=hTd*tB9jnPYRBQAcMQe zR$u7q3=J~~N7zbe==qdN?=qE$Xa%%i<{p84_l`N;Vw+6XTr z$gUl~px`eX3+kBP&$MjLwCW#c$Pv-oSzB^aer^#q>kxsDJ;?5PNkJXsyw)+TOG{ZE z63V`EuWZ1rwM#dI6k)As44!0m0vYiPQkw|2DG@qJ&u zps?E|^T5G(W}+`eC;dVQ))VR7f0=*jyuYi?T~|scenWFD?bB#SFI9RUL# zt}rw$b6c%7%G?pVAdsCRJO*gM;~E8}LI7qU7)Rwdi18UeKU_Zxn4BAB)dOT-T3)h9 z(B<4hkRY6$Fe+YrDLVTg@3MLkBg>@(6z)4El0K~>#u7kgGNdl;p zFCFwD%@68`I^UbR3=pvH`D9~dcj!cgGhT$5)W;PMNKLsY6LhlJ`yEo`PjE&MnZEY; zV~u{TThen@PrJnjF#*W|oa%&UdXxL$3qt^ahsdeJs&iYqr)<(@GW6KTKqa&Yrn4q| zQG(iZ^I5O{bH0Cf$GdO|AT}tASWJQ=NW)W-5+lwOC+(mQL!)SR`ENn*zGAbc-#msx zKEz$!GH%KgTHFcRCwfUP-{;@Jx9BW1Y3(XauuxTB{g1yZN|y|Ld(~g^%L0A~yw|oe zev!u{(aNHG#~;!h)MJa6{gU5jCt`xh-zFwQOhs}brSqLgX7Z$h5WoZKH*5L* zX8@TjH}=mwngPBC=}{2=w6`cd9uL!@DQ>m+-k+ZLQJ-!j!_PqmH3*Vm?G$41+d8`d zYK41KcAqa8`Ib@$l}kd4DE|^a!fKsYa6LA{3g6a4tI+a~%%ln2Z zQorzDm8setfRI89tJ$6fK;$~&RBBsXKa<%D8D?l2qL$5+W)yk&06<@nh#o&>U*i$t z$*s?^pDG6Ut2EjmjSDlLPV#*xa*Ud61d4M_h{CPR6FQJ*{`yMP_C;J&HGR1=kqkf8 zh&UrL2>9R_BjeTjCF&f0jZ9S`l#hh*5@*eGkqHbj>OCdg@h5-Az=U#o?(P0e)kVpy z{1sLi9vxa(ZZg z)3qBf+uCgL|7sHddy#oMgKQcy46W<1oV~L#;HreK4<*QBP15(pbz=>vJS(rRjWn6~ zRs08{Zlt5waGNTb00vhVJ#KC<%ie0~+5ET8=`69Toi;fk5E06#;BUboN(|+FyP&r! zuyHK$+j|zt5|>9z@0nRjSafu+aB3Ysow9veiZhZ?*{S}*y+xh}*0{Yy52ZyOy^58Y zVPS1(ET>y#6_r*Q1`MFA{tsRP7?7AzJX6yI)93kz2Zn}L8lGM6ug#`9U?3s^RVoHJ46*t9CEJXv`zE5S9lU04`{ayu`X*I+#Dsfp- zxH>;W&&Ps{BQU_9WP9gC^8Qx;?aACPIWl1F+YNJ=pOgt|r2uEjtyXz$?T*K{lbjr7HZ41UqXd74!E=xL>*M=?{px!PpSzXB za^&Pr9;olTf37?6DRi78CKM#};b}fTdJ2BQxu@ey*wz4&7!?O~t19>uNRCd;m~0nz zaMHT|Cf^+iH~T5^@B9F&vE`(bN?}0V%< z&uRqMLjB?hax#%$1o7F{_#3XFCbhPBbZ}%J*b2eqJ5oY690|WbDrQ?09fB&(3Bs6s zESUu&V|VU<6-LV1Auv5kYB#o1Y;iNgs)_gJI6d+GNujN0X!ede+GvWjiB!H^$HBTy z^at=!2>d(%o!*)3NJtKOzJrkO(spiKCyE{!dqt_$=)q{{VXDX>LncaPQfn9XP4tnr zHov5H=h)8VqF_BIHz)?YmuWSLOyVG+ODJUKi7C}{>{mWW_uZkEBBv`de1dr?$gdU_;ZZ3 z3^jPmsmQJN5{8NXZrX1=vbV{|Bp@GM6$QT9s}0OomSCIWDqr$j=M;pkh?-O{{q-oO z!U5@8&EAk{OpD4g2%t8)HRZ(zCmTv-WR}M){+ieOBjTq%HzVUR#56y%AQADU7H=wy zb?k1uFk}};wunwWUsHoD&4y!eht9Acwj;|wm-Y*u3~0M&CD8pNu^^&kzK_1l1@C&) zfm|DITpp%p?B^U_k%1Kn+qY<%HX-%n!~>toc11<1yf}_kF*;5gOF@X*{S+-xn`HQC zIa#mE*hR?rI7E^zt8=8q(4G`|dp;)_MfzHP95x|$fv1lIU+SnEkC_*&UVKk9#kZ!h z_)C+V9T?4x1L63o(+YR%ez3ijY(Io>Z<0o8vQ14*@W&ZF@f2_lbmAYCm`SDPBgEkoFh6SthZV}Rb-18qa~w}A;ru!M@|@PC)E0bzs^NtyofX6?h$`0 zFELZ}=+^9iE{d_{R;WeXUq-labFMfY+KBp7>T!z5T_3-=fgiN&e(8 zprPQH@<^5+m=;A1$Y%JUsAa0Q&}9_90kCIT(;CGBNIJZft{i58&G=n%!**3~BN{(A zlq0Pyc3Cnl!}Fc+H(aeNP+qVV-HBk)r6*Upt(2MxInqq-VV-$zSD3l~nxzmZt7yghglG~J#ac?u|Nn;$M&;t9u-ebQDhd_i^xX5naKI9u@moY3qAc=AW~ z+b;GZW-q%c@;#x|-n(L54@f&v`eUIPp32#-Qg1cn_WAb4NZdF_+Nu#;5rPMfXx>-r|iq_jq;g3ITviRD}0JS)#9>c{zO-w-<6WPEW;n$smye?=AWf47X)G;PY z(g7tS@qdPy^wucRf}k7%_Zlr&!rhduJXG!T@%>ZUY{Hf&Hbq-D<2yozvuEo|cLlHQ zu*{Ntfeep)LQhsjVpanjBhlX{RkY@N+xOYD0>!&}eox$gv!|mqFNB>n)TD}&D}9AK zd_2H+PGxiH%=z*-!EE-^o@_+jPwjB1w;m@*;WvaLl>&0l&rko31e`6{nwu94jU(bn%dQ7>ab6U?sH=Sw+0)D>gKZ-<)byA)&mo&s9#l7h zV(t{DX`BWdl_BzG5*Ai6nL_fmZXw#7#4#m*_Uym12W0V;vEzX_a@f5oTtSHCvtsUg*|Kl6>8iNe)j@`A7e+k|CzY1}(2+Z&0<;h0D6y{7d$ITH zE`t2wCu8i7KFT7c!v{PE70V$)g>0+X{-G#L{r(bT<|tjv5f+}fv^=n1&kQvbc?YnP zKG(=_!eVpOS63%B71DcSpGP3(H zpK=qM&PkX>#K3Czz;;Eoa+j?Q#^p!s&w-l|w$-BS5mNw$FowW4DleK?v;$|dba5hC z-r(7%T86W_ULPqea{8$OQ_pIM1}}yZ!awTS`9xgmhW-hwKA5$1d}V7dUY;Gp36Mq5 z@{iYn`7)T4rYQ2go5@Mw9;#e&qD?abJ$W<}>DK7prwMiuKT z5&>BeSS$9p-popU^zV;GPJk<;rgLlek#i!~t!^k4w*^5#`i4a#C22WoWV-k$2(kC` z80QJ>S86T|7Lu}*B8nO<7`9w{__3q4i-Hzqa{2;2J2R}^0w$7<>THkE+{;g?(t@Go zCZ`Esr>tPTyvb)!dePg2%oJt&5|7IY!C%o@O5E=LALe%+RkSj(uMV2duc)w=YH_@a zZ(rVdbPDM*LM}Fvy;w}2XHf~0Sna54ZLrds++@_?e%Q{ZbUYF%l*u*v44&26+FelT zY>m(=k%Ml{MBCCMBKu2%HrKwPIpAX&-K7!|nl8NJi*&~P=(r$Z2!Y0-nv!tNAIe?o zUe@k4`%6z6BTOtvjoELp;-H+#A<|q!=APDgy#`zbo@$&lA z^J;0s;?wM~9IgDD>QonYIXZ-^*Bwe%{pqD!&pvKH&F$nf?Sf|Z<-v$>Ct3tH@NMql z=*z@Z%^6UC!~BwSQoX^Rr}nB_Rx)fsT$T`OrRK+XR|e!j`E={I_OdtBn$rNfNupr z%0Fr_VXMQ$Js#3;)o&nPGl`tF1fc8J*;zhL~%Q z!h5*TQDu=%*0{X-;*XJ)k9MCrG!ft0sM2F*_K#2sHav}8lDbJjk&@Wk$~E$l#v8P} zK)^0lrNlXM#X`%ES`$fl?}2#q?mo2AMLoL;HdbqHLdC(JhJ%CE`Y_NS7%lGpJBe*K z&e)K{c8WMICYw;^wB`#MBREC{B|F?h*5Oz}BomYpL>A z^dmwq3s|9!C*R0?lNP?Atk&v6n{0u2VX+@<#ah=8)Q@3RBN};pQufQ})mTM=Fw{7B z2`%;|;v*ZELo?)BUcXw1F2mDOC59&{&`=d!&A+@8!q-I}`cjn|lnz=TD zzjO@7)B=7I_cxJZD2Cfj;B2L^5{^{4|4JuEsUFg$ZMRwWclp3sHC%6Q^o@zUvQt}Q zv^_D0iyrxy%-gZ@jwpwC(wZ|leS?68Y=<|SwB6Zins849=>TqrFV|(m95#R)1=dY`BFRZ>_-AP=oKkjIOge^rDK9pe&>$QgkY zy{~^3DWhcH6?zS{or`94fteT~BT`_#o79uI$Eo1j^=H}72Y7J-g9dH^{sXxqj_YS>jiFjCASN3^Fcg-iG~K+o!vF;X+2 z`NLxG4VDFam7+p6N~naH7T^CCtVnLinwVU0U8;Q@Av7&YG;pHq84Q5mCn1O<(EXO? zm^&bl=7Kd@9Sm3d%;+T{04R??oUrrYU>&*+Ov|V)Zy-(5g#5lsUF^~)-cHTdX!CY_RG-` zgp0Z4;%=QG+T@#T@pnW2w(+HCS~njNNjFH)taS2E;%3J>p2mDVb6aiQryJ`@7lk;! z4+l5L!(y`Oh8}rhDaMHG%euh~Z=TjTG#_(#&P$t91;f?ZN@n5`O7wUc_XzZRH1E{0_H#T9Dt5rSG%>;}Si!>_|9qf-hmZj2a_NZ-!zm(X zeUSih^46B7!OYP|f9Jd}fRY`2E$#j4t1O;Qx$n*4dcER?u&M`o{%Lq7nCNf1jKbVI z-$K`YLAz=sb1l^w=Van3YlAt54mgxfR6l-?N=?+Z4kLc zqt8qzt-&pnJq`n82lrfzYV+Z`rtTeBLW2)|?Wa~Moorr8rDYkw2zq<+z z!v*{Q;*H?`!y!RI{RcmSY7YKy_CM$&D5(Ei`hRjN|5FMJ^S`D4AGPvdDLle|=fV6Z MJpS`L()_pjUtRzq-T(jq literal 0 HcmV?d00001 diff --git a/test/extra_field_test.rb b/test/extra_field_test.rb index fa6e212d..69704890 100644 --- a/test/extra_field_test.rb +++ b/test/extra_field_test.rb @@ -73,4 +73,16 @@ def test_equality extra1.create('IUnix') assert_equal(extra1, extra3) end + + def test_read_local_extra_field + ::Zip::File.open('test/data/local_extra_field.zip') do |zf| + ['file1.txt', 'file2.txt'].each do |file| + entry = zf.get_entry(file) + + assert_instance_of(::Zip::ExtraField, entry.extra) + assert_equal(1_000, entry.extra['IUnix'].uid) + assert_equal(1_000, entry.extra['IUnix'].gid) + end + end + end end From fe1d3c8da0591a4a802f711b4a9770f34899132f Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Sun, 20 Sep 2020 15:06:04 +0100 Subject: [PATCH 2/7] Fix reading Ux extra field. As previously implemented the `uid` and `gid` fields could only ever be read as 0, because they were being initialized to zero and then memoization (`@uid ||= uid`) was used to 'save' the new value. Using `nil` as the initial value for either of these fields breaks so many tests, so I have fixed this by not using memoization instead. This is safe because it is only the local extra field that holds these values for this type of extra field. --- lib/zip/extra_field/unix.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/zip/extra_field/unix.rb b/lib/zip/extra_field/unix.rb index 9a66c81d..d83087e4 100644 --- a/lib/zip/extra_field/unix.rb +++ b/lib/zip/extra_field/unix.rb @@ -20,8 +20,8 @@ def merge(binstr) return if !size || size == 0 uid, gid = content.unpack('vv') - @uid ||= uid - @gid ||= gid # rubocop:disable Naming/MemoizedInstanceVariableName + @uid = uid + @gid = gid end def ==(other) From f742994cf25bd4c3fb66f69b9ecc012f3f51c214 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Sun, 20 Sep 2020 15:18:34 +0100 Subject: [PATCH 3/7] Abstract out reading extra fields in Entry. Remove some (almost) duplicated code and get ready for the real fix. --- lib/zip/entry.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/zip/entry.rb b/lib/zip/entry.rb index a67c6568..bf47ad6d 100644 --- a/lib/zip/entry.rb +++ b/lib/zip/entry.rb @@ -271,12 +271,7 @@ def read_local_entry(io) #:nodoc:all raise ::Zip::Error, 'Truncated local zip entry header' end - if @extra.kind_of?(::Zip::ExtraField) - @extra.merge(extra) if extra - else - @extra = ::Zip::ExtraField.new(extra) - end - + read_extra_field(extra) parse_zip64_extra(true) @local_header_size = calculate_local_header_size end @@ -379,11 +374,11 @@ def check_c_dir_entry_comment_size raise ::Zip::Error, 'Truncated cdir zip entry header' end - def read_c_dir_extra_field(io) + def read_extra_field(buf) if @extra.kind_of?(::Zip::ExtraField) - @extra.merge(io.read(@extra_length)) + @extra.merge(buf) if buf else - @extra = ::Zip::ExtraField.new(io.read(@extra_length)) + @extra = ::Zip::ExtraField.new(buf) end end @@ -397,7 +392,7 @@ def read_c_dir_entry(io) #:nodoc:all if ::Zip.force_entry_names_encoding @name.force_encoding(::Zip.force_entry_names_encoding) end - read_c_dir_extra_field(io) + read_extra_field(io.read(@extra_length)) @comment = io.read(@comment_length) check_c_dir_entry_comment_size set_ftype_from_c_dir_entry From c2b9aa2893c03537e9a985b8410d9bb1eeeba8e2 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Sun, 20 Sep 2020 18:11:49 +0100 Subject: [PATCH 4/7] Correctly read extra fields when opening a zip file. Previously, only the extra fields stored in the central directory were being read in. In reality it is often the case that the extra field in the central directory is just a marker, and the full data is in the local header. So we need to read both in and merge the two into the final correct extra field. This merging infrastructure was already implemented in the extra field code but we never actually read the local extra fields in until now. Reading the central directory headers and local entry headers seems rather fragile, so we can't just read one over the other and hope to end up with a correctly merged set of extra fields because this breaks other things. So we need to specifically read the local extra field data and merge just those bits. This commit also fixes a couple of tests that were 'broken' by us now reading extra fields in correctly! --- lib/zip/central_directory.rb | 29 +++++++++++++++++++++++- test/filesystem/file_nonmutating_test.rb | 6 +++-- test/filesystem/file_stat_test.rb | 4 ++-- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/zip/central_directory.rb b/lib/zip/central_directory.rb index 9975884c..5b64c7f5 100644 --- a/lib/zip/central_directory.rb +++ b/lib/zip/central_directory.rb @@ -124,10 +124,37 @@ def read_central_directory_entries(io) #:nodoc: end @entry_set = EntrySet.new @size.times do - @entry_set << Entry.read_c_dir_entry(io) + entry = Entry.read_c_dir_entry(io) + next unless entry + + offset = if entry.extra['Zip64'] + entry.extra['Zip64'].relative_header_offset + else + entry.local_header_offset + end + + unless offset.nil? + io_save = io.tell + io.seek(offset, IO::SEEK_SET) + entry.read_extra_field(read_local_extra_field(io)) + io.seek(io_save, IO::SEEK_SET) + end + + @entry_set << entry end end + def read_local_extra_field(io) + buf = io.read(::Zip::LOCAL_ENTRY_STATIC_HEADER_LENGTH) || '' + return '' unless buf.bytesize == ::Zip::LOCAL_ENTRY_STATIC_HEADER_LENGTH + + head, _, _, _, _, _, _, _, _, _, n_len, e_len = buf.unpack('VCCvvvvVVVvv') + return '' unless head == ::Zip::LOCAL_ENTRY_SIGNATURE + + io.seek(n_len, IO::SEEK_CUR) # Skip over the entry name. + io.read(e_len) + end + def read_from_stream(io) #:nodoc: buf = start_buf(io) if zip64_file?(buf) diff --git a/test/filesystem/file_nonmutating_test.rb b/test/filesystem/file_nonmutating_test.rb index 346d5a76..485298fa 100644 --- a/test/filesystem/file_nonmutating_test.rb +++ b/test/filesystem/file_nonmutating_test.rb @@ -295,8 +295,10 @@ def test_ctime end def test_atime - assert_nil(@zip_file.file.atime('file1')) - assert_nil(@zip_file.file.stat('file1').atime) + assert_equal(::Zip::DOSTime.at(1_027_694_306), + @zip_file.file.atime('file1')) + assert_equal(::Zip::DOSTime.at(1_027_694_306), + @zip_file.file.stat('file1').atime) end def test_ntfs_time diff --git a/test/filesystem/file_stat_test.rb b/test/filesystem/file_stat_test.rb index 05d7fff8..b8efe754 100644 --- a/test/filesystem/file_stat_test.rb +++ b/test/filesystem/file_stat_test.rb @@ -19,11 +19,11 @@ def test_ino end def test_uid - assert_equal(0, @zip_file.file.stat('file1').uid) + assert_equal(500, @zip_file.file.stat('file1').uid) end def test_gid - assert_equal(0, @zip_file.file.stat('file1').gid) + assert_equal(500, @zip_file.file.stat('file1').gid) end def test_ftype From a668fd14d2047b27ba9fa5d39e7e7f91971a4084 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Mon, 21 Sep 2020 09:34:14 +0100 Subject: [PATCH 5/7] Test reading an extra field with a bad header ID. --- test/extra_field_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/extra_field_test.rb b/test/extra_field_test.rb index 69704890..6f2ec65b 100644 --- a/test/extra_field_test.rb +++ b/test/extra_field_test.rb @@ -17,6 +17,16 @@ def test_unknownfield assert_equal(extra.to_s, 'fooabarbaz') end + def test_bad_header_id + str = "ut\x5\0\x3\250$\r@" + ut = nil + assert_output('', /WARNING/) do + ut = ::Zip::ExtraField::UniversalTime.new(str) + end + assert_instance_of(::Zip::ExtraField::UniversalTime, ut) + assert_nil(ut.mtime) + end + def test_ntfs str = "\x0A\x00 \x00\x00\x00\x00\x00\x01\x00\x18\x00\xC0\x81\x17\xE8B\xCE\xCF\x01\xC0\x81\x17\xE8B\xCE\xCF\x01\xC0\x81\x17\xE8B\xCE\xCF\x01" extra = ::Zip::ExtraField.new(str) From 8bafcbbc4db64ba7e9f6d7e2fc59cb3ac87dc685 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Mon, 21 Sep 2020 09:51:47 +0100 Subject: [PATCH 6/7] Remove dead code in extra_field/generic.rb (`==`). From what I can tell this was erroneously copied out of extra_field.rb during a refactor. It attempts to compare a non-existent Hash that used to be inherited before the refactor. If this code had been left within ExtraField it would make more sense, but as it's not needed there either let's just remove it. See 20d79feb995728c6b791580cc82f62f2be82606e for the refactor. --- lib/zip/extra_field/generic.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/zip/extra_field/generic.rb b/lib/zip/extra_field/generic.rb index 5eb702d6..9237a1db 100644 --- a/lib/zip/extra_field/generic.rb +++ b/lib/zip/extra_field/generic.rb @@ -22,15 +22,6 @@ def initial_parse(binstr) [binstr[2, 2].unpack1('v'), binstr[4..-1]] end - def ==(other) - return false if self.class != other.class - - each do |k, v| - return false if v != other[k] - end - true - end - def to_local_bin s = pack_for_local self.class.const_get(:HEADER_ID) + [s.bytesize].pack('v') << s From 0235e76baecd1d952e44319db906775156675664 Mon Sep 17 00:00:00 2001 From: Robert Haines Date: Mon, 21 Sep 2020 10:50:22 +0100 Subject: [PATCH 7/7] Test packing the NTFS extra field. --- test/extra_field_test.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extra_field_test.rb b/test/extra_field_test.rb index 6f2ec65b..52140a2d 100644 --- a/test/extra_field_test.rb +++ b/test/extra_field_test.rb @@ -35,6 +35,8 @@ def test_ntfs assert_equal(t, extra['NTFS'].mtime) assert_equal(t, extra['NTFS'].atime) assert_equal(t, extra['NTFS'].ctime) + + assert_equal(str.force_encoding('BINARY'), extra.to_local_bin) end def test_merge