From 942562c3822b87ca73bd07b022b501104f67ad0e Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 16 Jul 2020 17:00:29 +0200 Subject: [PATCH 1/2] Clean up Zip Util to be more strict about what is a valid package archive, fixes #8931 --- src/Composer/Util/Zip.php | 50 ++++++++++-------- .../artifacts/composer-1.0.0-alpha6.zip | Bin 5227 -> 4046 bytes .../Fixtures/artifacts/jsonInFirstLevel.zip | Bin 542 -> 519 bytes .../jsonInFirstLevelWithExtraFirstLevel.zip | Bin 0 -> 595 bytes .../Fixtures/artifacts/not-an-artifact.zip | Bin 4140 -> 3264 bytes .../Test/Util/Fixtures/Zip/multiple.zip | Bin 2569 -> 936 bytes .../Test/Util/Fixtures/Zip/single-sub.zip | Bin 0 -> 306 bytes .../Test/Util/Fixtures/Zip/subfolder.zip | Bin 1328 -> 0 bytes .../Test/Util/Fixtures/Zip/subfolders.zip | Bin 0 -> 454 bytes tests/Composer/Test/Util/ZipTest.php | 18 +++++-- 10 files changed, 42 insertions(+), 26 deletions(-) create mode 100644 tests/Composer/Test/Repository/Fixtures/artifacts/jsonInFirstLevelWithExtraFirstLevel.zip create mode 100644 tests/Composer/Test/Util/Fixtures/Zip/single-sub.zip delete mode 100644 tests/Composer/Test/Util/Fixtures/Zip/subfolder.zip create mode 100644 tests/Composer/Test/Util/Fixtures/Zip/subfolders.zip diff --git a/src/Composer/Util/Zip.php b/src/Composer/Util/Zip.php index ab10d5bbf..3ebdcdd85 100644 --- a/src/Composer/Util/Zip.php +++ b/src/Composer/Util/Zip.php @@ -71,37 +71,41 @@ class Zip */ private static function locateFile(\ZipArchive $zip, $filename) { - $indexOfShortestMatch = false; - $lengthOfShortestMatch = -1; + // return root composer.json if it is there and is a file + if (false !== ($index = $zip->locateName($filename)) && $zip->getFromIndex($index) !== false) { + return $index; + } + $topLevelPaths = array(); for ($i = 0; $i < $zip->numFiles; $i++) { - $stat = $zip->statIndex($i); - if (strcmp(basename($stat['name']), $filename) === 0) { - $directoryName = dirname($stat['name']); - if ($directoryName === '.') { - //if composer.json is in root directory - //it has to be the one to use. - return $i; - } + $name = $zip->getNameIndex($i); + $dirname = dirname($name); - if (strpos($directoryName, '\\') !== false || - strpos($directoryName, '/') !== false) { - //composer.json files below first directory are rejected - continue; + // handle archives with proper TOC + if ($dirname === '.') { + $topLevelPaths[$name] = true; + if (\count($topLevelPaths) > 1) { + // archive can only contain one top level directory + return false; } + continue; + } - $length = strlen($stat['name']); - if ($indexOfShortestMatch === false || $length < $lengthOfShortestMatch) { - //Check it's not a directory. - $contents = $zip->getFromIndex($i); - if ($contents !== false) { - $indexOfShortestMatch = $i; - $lengthOfShortestMatch = $length; - } + // handle archives which do not have a TOC record for the directory itself + if (false === strpos('\\', $dirname) && false === strpos('/', $dirname)) { + $topLevelPaths[$dirname.'/'] = true; + if (\count($topLevelPaths) > 1) { + // archive can only contain one top level directory + return false; } } } - return $indexOfShortestMatch; + if ($topLevelPaths && false !== ($index = $zip->locateName(key($topLevelPaths).$filename)) && $zip->getFromIndex($index) !== false) { + return $index; + } + + // no composer.json found either at the top level or within the topmost directory + return false; } } diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/composer-1.0.0-alpha6.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/composer-1.0.0-alpha6.zip index e94843eb614f84703773840a0b898a10ee8e1874..585b4f7ea0075fa9935849ea299e15eb4c11af2e 100644 GIT binary patch delta 98 zcmaE@aZY~2LdMP8nY>vh?`0C$e4Bk0`{Z(gwUbqZRG8*6PW~t$IXPL#p2?31%-k#F sz+}ukd7*&B+&jYJBCKo-K)?osB1{YnOL##%0Eqk>jsO4v delta 979 zcmX>n|5{_iLdGaA1`wFIq}~b4fD)Vx0u1r-zK+iR!4dkQ5j+frO%Z`0{FPCJA;6oN z1-mwRWNniJnZ-EadMDZ{@@k<=2kXV_xdg`tm*f|vf>i!!j0pS<#2{Oe6GW5K6Os}> z`1*u>;1BBvXkeNk!K^MI&A5?Cp!tU*j{=`hD+6;F1JFI}9Q&NR6tsYj4`P`7fK_Jm zc1BZ{$+pY_yn5(XGU>%nPG^?Q#%+9|D$w}Vk{wKaii(Z_Y97aB6rQ;UAL2Qq@`v{NY09mngMZ9fmH8-SfGnmGi-jq zdW?PY6IKb#0I1iC4{~*M@paY9O#uafX28F(yS zse4-IoX;r}PhO@h1}%dtrY1&Pj7$vHt~0w3Xt3gH;0iO74O@UrQ{y#jS-~N^_|d`G zE}(y2F-%^>yRja70^nrefur68_1d{9@WYz`TKoOa=x$6tjP@VKLi{-)Ba=M?F2AY(wS#~Hn8faLE~qRcg9L-=x*K`T(bpJZbmf$W z91tJb7EFI4+mZ#e2^zj|UnAS_?1pwG$V3=D?dpnjm<^EF!5#=u4*;q4Xf}X@B*2@M R4Wxt_2-z7K7+!%m3;=?4hAsdA literal 542 zcmWIWW@Zs#U|`^2n7%B?CH~jPFdiUJ9EdrAIJKgrC{eGZqJ-O1SMTH*?{E#rrHlb6 zTKbq?p0)rg1!0gu#idCpnML}^`MCx8#i>PlS;hHztHS~UPI;g44c!zJpanGP;(4ue z=Rcn*KCPpr_t8_=`)uH)zyK|8U9EFx&NHtxykdO8I3Q>RD+8)c$c__0*hGXIk#!29 z=`_Oh9wQUPE7-#Xs2>TyZ4dB9)rKA}2tCF?Cbm#QHv!qzApau3Tp$x_0#5spb%Xqj c0M~&`WZmEp2=HcQ11Vtv!f!xYh!MmC05v{@hyVZp diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInFirstLevelWithExtraFirstLevel.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInFirstLevelWithExtraFirstLevel.zip new file mode 100644 index 0000000000000000000000000000000000000000..b400918b9c7ce2bde31d94f1d931c2a0e8123b8e GIT binary patch literal 595 zcmWIWW@Zs#U|`^2n7%B?CH~jPFdiUJ9EdrAIJKgrC{eGZqJ-O1SMTH*?{E#rrHlb^ zEkT_h0=irmO!EXP^#)>)LB*v>DVat3$@#ej`NgS4dRfK!d9J4oxf%?3STC$@vG-oP zdUw-;99=Vd!$!s=@+v+RrrC^>FYrY$`!F#~Oq7`H%gxkh2be8wm)1`wFKxZVlOfD)Vx0u1r-zK+iR!4dkQ5j+f04H1DL{FPCJA;6oN z1-mwRWNnibxh2@4Iw#sH@@k<-7IKRR#_PER#|M|>7o~!%@BkVgRtK>nIYBfzJs~OK zgRf862mY{*fCi=s63prX(u^CK1e$+1@+k26v@$SE05|gcY9GKl%fK2hpQ+dQqK~9F)afUD45fL2dc5niN0G?1m dcIY6-F$X-31H4(;K!L>xgwo6m48Ora2LOIBzxn_G diff --git a/tests/Composer/Test/Util/Fixtures/Zip/multiple.zip b/tests/Composer/Test/Util/Fixtures/Zip/multiple.zip index db8c50302d78e7512c043c4598fd62f518af151c..96c0959bbd5a61358708d7c4eb32e5e99e37caa6 100644 GIT binary patch literal 936 zcmWIWW@Zs#0D&V3DSluEl;8l;Y56%RsYQnR0Z>&O410m9vKTsYCjezxBp4V3kyPoq z1jh%LA#iljshH9~?7e zFH3`*&jt1t$dlLRi!;jsc_1u`PP5P;IK z$m3#A0E;j(0OL#J{-eT6R~Ub8ZqBWbzQ!ndE-42@2YACw#umjelN*5Mr~xs|WO#(5 zSmFNi4#*0I`qrNy8r2G1(SU5lQzR=;LI=efs843z=F0>5K<6vEPq0M}%oL0-|B%Dzdp)wrxFQs0as$xh7$lQ%dH`n4i2#1E2Y8mltYKvXn!&&dginD6 Jtp#ER1^_Zv^%?*G literal 2569 zcmWIWW@h1H0D9J_iOYJ9Y%ed5Eum*72cZaN; zaO1hyoCT}rHJO$_(7EzMR7prBVnNskyOY0^Ds%kJwjF24`F0_sf&IaR+?UBr$ITWk zIBoJHw=et7;;j7DKSRxJuOFOPWPgm;YD-4jPyO)b75;*!*R2!J-G1ZSj^|&WJ^lFQ zWsTOVpKe!P@t|$qy1u1*_P$Z zvM=ps&z`pNhrzl%+Hq%qa`wMc760+NN7~K(FCKsWzf*^O&(yDb<~`i2ATjaWtqr-; zcQZc7?E9#?;6#6G`i~d*(ws90894P&B}z0J}C&6u`A1#~!j)?6D_-u2nA{XGA6^h$g2eBqefhbjzQbzgzyynQ}!*S#@2M9@x2TPq(|s0i6oM2s&*) zb0=Up7d^*_qBz%3A0@y+84i(itoaTZ2)Ly8a(kQTY2Gbh$V+fN$+?4pHGIMMi;A}d zH;DuVs5Vtj`Wn_%pDE+daO1;{_y!(@gT2OflWvH$tTQ{O*dpN6QJcK3h17he(UnQoQ2wV@9g5>*eL1`PU;LD zxtoANhd-%<0s$q>LZTfJj)c>$0xl!a(lBz6QY{TbQX+byKuw8;`WPt@3CQR8j6inDU&pP;Mj(3#VGP1U$c32%iibX+8-tP#vHK0ZDC0pg@vUPu z6C{8UMIyv|pt1}(ph2b}ms*lYrYvdPie?I0I^lp9kH{g0p0ZILrp1ixFi@t2I1H4u zksSsq`m&J915p%*rJ$LDk`iFK7THngDFNBIzmAzKNGSmnjff%zJyl?jP6ZU#t-)m+ zTC}1%6k;+c^pG>h~n5SP$Mynf|UUQ-mGk({K^Z2 L*MQ+J3+4d;JZ$R> diff --git a/tests/Composer/Test/Util/Fixtures/Zip/single-sub.zip b/tests/Composer/Test/Util/Fixtures/Zip/single-sub.zip new file mode 100644 index 0000000000000000000000000000000000000000..b8ccd4c76300549eb937b85666e320a8f5aeffde GIT binary patch literal 306 zcmWIWW@h1H0D17q?=T&nl0D)3oVs5IEm4Z@Qe!hNEVv!P8EmwdyBaj+{(oWcQj3P`s-14K8bLsX!;6@au6R5yyVxEMfE3=E765)2Hhl^wD# kU14mwZs<}UeU0%X$2BJq9pKH%22#TWgbRUmHHgCi0GA^^2LJ#7 literal 0 HcmV?d00001 diff --git a/tests/Composer/Test/Util/Fixtures/Zip/subfolder.zip b/tests/Composer/Test/Util/Fixtures/Zip/subfolder.zip deleted file mode 100644 index 93060bea23bf3efe8fec023cb6c2dd42ee44a80e..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1328 zcmWIWW@h1H0D9<*fOmYO^o~FaN4*AxTAik* z2{)dL%~_!BcW{<YzU)LgyQ-2i#pKZKtBI5c|F|oE_%ktS7iS*FE=|M50!NPqi(^6jtqz$w;lm-d`6t`T9;Jrwua zFSm-5p}cqtKcnZLNQ*=1`#}c0c612b^@tNyd=1r@oS$2eUz}Q`msOmf2TR3y zZy|xW`dJVI)6zXh?u4jnvo788r{?dHBTB04qV&KHtPXAa zNl7h&Yji-;D8j%2guy|mR!N|0OwP|O$S+PU(#tB&&x2W?_ZDLP>SsX=OiTA1xf7zQ z&AN2UpPIi*jwq?Fi_!x+jGd!CuXwu@&;uYG;LXS+%8bi%JTSjHymbUIAztNxdlgwX zrbm%=8$or$y$vx0markTestSkipped('The PHP zip extension is loaded.'); @@ -74,7 +74,7 @@ class ZipTest extends TestCase return; } - $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/subfolder.zip'); + $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/subfolders.zip'); $this->assertNull($result); } @@ -103,7 +103,7 @@ class ZipTest extends TestCase $this->assertEquals("{\n \"name\": \"foo/bar\"\n}\n", $result); } - public function testReturnsRootComposerJsonAndSkipsSubfolders() + public function testMultipleTopLevelDirsIsInvalid() { if (!extension_loaded('zip')) { $this->markTestSkipped('The PHP zip extension is not loaded.'); @@ -112,6 +112,18 @@ class ZipTest extends TestCase $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/multiple.zip'); + $this->assertNull($result); + } + + public function testReturnsComposerJsonFromFirstSubfolder() + { + if (!extension_loaded('zip')) { + $this->markTestSkipped('The PHP zip extension is not loaded.'); + return; + } + + $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/single-sub.zip'); + $this->assertEquals("{\n \"name\": \"foo/bar\"\n}\n", $result); } } From c353ac835c8e09600b91b9c20778390ee0ff1099 Mon Sep 17 00:00:00 2001 From: Wissem Riahi Date: Tue, 21 Jul 2020 17:10:26 +0200 Subject: [PATCH 2/2] Add exception for multiple composer.json files (#3) --- .../Repository/ArtifactRepository.php | 7 +++- src/Composer/Util/Zip.php | 12 +++--- .../Util/Fixtures/Zip/multiple_subfolders.zip | Bin 0 -> 3724 bytes tests/Composer/Test/Util/ZipTest.php | 39 ++++++++++++------ 4 files changed, 38 insertions(+), 20 deletions(-) create mode 100644 tests/Composer/Test/Util/Fixtures/Zip/multiple_subfolders.zip diff --git a/src/Composer/Repository/ArtifactRepository.php b/src/Composer/Repository/ArtifactRepository.php index a0acb7a61..f8b068218 100644 --- a/src/Composer/Repository/ArtifactRepository.php +++ b/src/Composer/Repository/ArtifactRepository.php @@ -88,7 +88,12 @@ class ArtifactRepository extends ArrayRepository implements ConfigurableReposito private function getComposerInformation(\SplFileInfo $file) { - $json = Zip::getComposerJson($file->getPathname()); + $json = null; + try { + $json = Zip::getComposerJson($file->getPathname()); + } catch (\Exception $exception) { + $this->io->write('Failed loading package '.$file->getPathname().': '.$exception->getMessage(), false, IOInterface::VERBOSE); + } if (null === $json) { return false; diff --git a/src/Composer/Util/Zip.php b/src/Composer/Util/Zip.php index 3ebdcdd85..ad6617edf 100644 --- a/src/Composer/Util/Zip.php +++ b/src/Composer/Util/Zip.php @@ -66,8 +66,9 @@ class Zip * * @param \ZipArchive $zip * @param string $filename + * @throws \RuntimeException * - * @return bool|int + * @return int */ private static function locateFile(\ZipArchive $zip, $filename) { @@ -85,8 +86,7 @@ class Zip if ($dirname === '.') { $topLevelPaths[$name] = true; if (\count($topLevelPaths) > 1) { - // archive can only contain one top level directory - return false; + throw new \RuntimeException('Archive has more than one top level directories, and no composer.json was found on the top level, so it\'s an invalid archive. Top level paths found were: '.implode(',', array_keys($topLevelPaths))); } continue; } @@ -95,8 +95,7 @@ class Zip if (false === strpos('\\', $dirname) && false === strpos('/', $dirname)) { $topLevelPaths[$dirname.'/'] = true; if (\count($topLevelPaths) > 1) { - // archive can only contain one top level directory - return false; + throw new \RuntimeException('Archive has more than one top level directories, and no composer.json was found on the top level, so it\'s an invalid archive. Top level paths found were: '.implode(',', array_keys($topLevelPaths))); } } } @@ -105,7 +104,6 @@ class Zip return $index; } - // no composer.json found either at the top level or within the topmost directory - return false; + throw new \RuntimeException('No composer.json found either at the top level or within the topmost directory'); } } diff --git a/tests/Composer/Test/Util/Fixtures/Zip/multiple_subfolders.zip b/tests/Composer/Test/Util/Fixtures/Zip/multiple_subfolders.zip new file mode 100644 index 0000000000000000000000000000000000000000..5c5bc5adf86e747df57a7e1d2b7218612e3134ec GIT binary patch literal 3724 zcmWIWW@h1H0D<-iHv+&6D8b4gz>t=oZ>%30!Nc&j=u2E}<>$Dsj3NvHa4nZ&Z=z`7 zglS1kN``Bz0cwS56Jg*0!bvfx+Jum`>A3{Q2bbg*rGl(gDEtzqQUKBacE(1oLk0qE z?{D$3PVecfWWBH8RVp6u5E9_Hvb);s{^{_7kA)`B+_|Gk-Wu=`ir#<5)C13G)@W#$$jo}R4(|;d+)#7cr6LM|S#J72|r`=QH#f~P0>&dmo z+<9VU%fI^K)!8o+Kf8vu2sn9^UtF>0dY)v<1bLM?{9~bq^qF(Rgl#(*1H9Qe4lj+2dku^V76}HV5a*9W4LVSW z$H)6RI{ODlz(X7qS+Ed?#4WluK4fhukxW=GD7N5Mg7qTDHnM)~u`P>Lzg|4fh)+%s zO-@fpO8DUG6ZU~WtRtX-X@Uf^x_~s}MrncOAEFLt7=j-v9B5#+Xlr3{oGEG$`vMeX z>>S)32P{m1L0AFAphOFCPjFC*A2f-BDKVgX;ND2i&n?I=PA$^QD$dUXrNA;^SmwR0 zi2;S@>SsX=OiTA1xf7zQ&AN2UpPIi*jwq?Fi_!zzR-aeAT?%L-2;;U*5r^;bT8ujc z3T1&IAT80s)TgNE7@+2HoJm2-z;v&3=dXI}Xq@o8dfHP*PgB>=*V9wSGn9{m zO_N9Cmk|4+)>~^U9ZkSa7s=T7I|k_N4Jb~ZgjOJc5)C+@Fk)&7v}AZY!`APxfkfMV z{j&m{N0k?^zaTwBLx$li(}cG93qyC6sHHy^nbX|M8?(RFqj18EsDr;ft_i;^{bSi( z&vf8b?RFNK2CkE0@XyV&cqk5m5nY^%MzW%j<_ zzw__ke|3HK^wov8{!Vx|&uDSkj^obT?kUb(d%R@XJ;k$Egq=7Pk3|HP7T(I_P&_wb zhh2r$1?NqM`Y(#U^ewiPeW7^s#`*=`yB5j5ed)k|GGNX!F}^56=tf;%NO4PK1YBNZbM zFHFHzaD%jAqzd)Xf*Vm)qL;v!r6_XHW0s=mL62FAg5vFCQ)Ww4J#gCISU33ZyiB2v; zFvJK@a6^oM7Cpdxi`b0V4b*KsMZ*n>9?LL(r~ z0i|YqM)08+aR=E5l#n504zjoYI!Z7hIR#$9W4Z|uM9A$0StK_tX^g;X7D{=I9$JWO zg4{3=LoxOR&{(+3(9$jkyy<})lc2^6a&4`MVqO?CHZSYN6J<20x<{_LkYi;@qaX{^ zsTc_aR7D{>5o8W>NJ^qOF@Y7`iC97rVI;2V3Xy)_I=%rK2@XX_2?|Mbpvnu`RiN?{ zcNS1)Lox!xhoB-9U;0H&cX90KK16mGA`L?l2*_Q?p@o|6=3zApONb$yNKCrZ<3RE* rM&f`K*2uw!Ki$m%ng`GI@TkC&?hr<^0z;gEK@bS#fzcPv3E}|&bq`N_ literal 0 HcmV?d00001 diff --git a/tests/Composer/Test/Util/ZipTest.php b/tests/Composer/Test/Util/ZipTest.php index 78c7e9657..ba61e8bf6 100644 --- a/tests/Composer/Test/Util/ZipTest.php +++ b/tests/Composer/Test/Util/ZipTest.php @@ -55,28 +55,30 @@ class ZipTest extends TestCase $this->assertNull($result); } - public function testReturnsNullIfTheZipHasNoComposerJson() + /** + * @expectedException \RuntimeException + */ + public function testThrowsExceptionIfTheZipHasNoComposerJson() { if (!extension_loaded('zip')) { $this->markTestSkipped('The PHP zip extension is not loaded.'); return; } - $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/nojson.zip'); - - $this->assertNull($result); + Zip::getComposerJson(__DIR__.'/Fixtures/Zip/nojson.zip'); } - public function testReturnsNullIfTheComposerJsonIsInASubSubfolder() + /** + * @expectedException \RuntimeException + */ + public function testThrowsExceptionIfTheComposerJsonIsInASubSubfolder() { if (!extension_loaded('zip')) { $this->markTestSkipped('The PHP zip extension is not loaded.'); return; } - $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/subfolders.zip'); - - $this->assertNull($result); + Zip::getComposerJson(__DIR__.'/Fixtures/Zip/subfolders.zip'); } public function testReturnsComposerJsonInZipRoot() @@ -99,10 +101,12 @@ class ZipTest extends TestCase } $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/folder.zip'); - $this->assertEquals("{\n \"name\": \"foo/bar\"\n}\n", $result); } + /** + * @expectedException \RuntimeException + */ public function testMultipleTopLevelDirsIsInvalid() { if (!extension_loaded('zip')) { @@ -110,9 +114,7 @@ class ZipTest extends TestCase return; } - $result = Zip::getComposerJson(__DIR__.'/Fixtures/Zip/multiple.zip'); - - $this->assertNull($result); + Zip::getComposerJson(__DIR__.'/Fixtures/Zip/multiple.zip'); } public function testReturnsComposerJsonFromFirstSubfolder() @@ -126,4 +128,17 @@ class ZipTest extends TestCase $this->assertEquals("{\n \"name\": \"foo/bar\"\n}\n", $result); } + + /** + * @expectedException \RuntimeException + */ + public function testThrowsExceptionIfMultipleComposerInSubFoldersWereFound() + { + if (!extension_loaded('zip')) { + $this->markTestSkipped('The PHP zip extension is not loaded.'); + return; + } + + Zip::getComposerJson(__DIR__.'/Fixtures/Zip/multiple_subfolders.zip'); + } }