From 74b4bcd22e68d0bdc446b37ad82d03d9929cb5cd Mon Sep 17 00:00:00 2001 From: Danack Date: Sun, 18 Aug 2013 22:37:18 +0100 Subject: [PATCH 1/6] Fix issue where none root composer.json could be used by ArtifactRepository http://www.php.net/manual/en/ziparchive.locatename.php#85512 --- .../Repository/ArtifactRepository.php | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Composer/Repository/ArtifactRepository.php b/src/Composer/Repository/ArtifactRepository.php index 869e4757f..e554cac8b 100644 --- a/src/Composer/Repository/ArtifactRepository.php +++ b/src/Composer/Repository/ArtifactRepository.php @@ -74,6 +74,39 @@ class ArtifactRepository extends ArrayRepository } } + /** + * Find a file by name, returning the one that has the shortest path. + * + * @param \ZipArchive $zip + * @param $filename + * @return bool|int + */ + private function locateFile(\ZipArchive $zip, $filename) { + $shortestIndex = -1; + $shortestIndexLength = -1; + + for ($i = 0; $i < $zip->numFiles; $i++ ){ + $stat = $zip->statIndex($i); + if (strcmp(basename($stat['name']), $filename) === 0){ + $length = strlen($stat['name']); + if ($shortestIndex == -1 || $length < $shortestIndexLength) { + //Check it's not a directory. + $contents = $zip->getFromIndex($i); + if ($contents !== false) { + $shortestIndex = $i; + $shortestIndexLength = $length; + } + } + } + } + + if ($shortestIndex == -1) { + return false; + } + + return $shortestIndex; + } + private function getComposerInformation(\SplFileInfo $file) { $zip = new \ZipArchive(); @@ -83,7 +116,7 @@ class ArtifactRepository extends ArrayRepository return false; } - $foundFileIndex = $zip->locateName('composer.json', \ZipArchive::FL_NODIR); + $foundFileIndex = $this->locateFile($zip, 'composer.json'); if (false === $foundFileIndex) { return false; } From abfefd1faae02a72dfa1259440fdf6d7305af736 Mon Sep 17 00:00:00 2001 From: Danack Date: Sun, 18 Aug 2013 22:57:26 +0100 Subject: [PATCH 2/6] Improved variable name. --- src/Composer/Repository/ArtifactRepository.php | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Composer/Repository/ArtifactRepository.php b/src/Composer/Repository/ArtifactRepository.php index e554cac8b..769e35b40 100644 --- a/src/Composer/Repository/ArtifactRepository.php +++ b/src/Composer/Repository/ArtifactRepository.php @@ -82,29 +82,25 @@ class ArtifactRepository extends ArrayRepository * @return bool|int */ private function locateFile(\ZipArchive $zip, $filename) { - $shortestIndex = -1; - $shortestIndexLength = -1; + $indexOfShortestMatch = false; + $lengthOfShortestMatch = -1; for ($i = 0; $i < $zip->numFiles; $i++ ){ $stat = $zip->statIndex($i); if (strcmp(basename($stat['name']), $filename) === 0){ $length = strlen($stat['name']); - if ($shortestIndex == -1 || $length < $shortestIndexLength) { + if ($indexOfShortestMatch == false || $length < $lengthOfShortestMatch) { //Check it's not a directory. $contents = $zip->getFromIndex($i); if ($contents !== false) { - $shortestIndex = $i; - $shortestIndexLength = $length; + $indexOfShortestMatch = $i; + $lengthOfShortestMatch = $length; } } } } - if ($shortestIndex == -1) { - return false; - } - - return $shortestIndex; + return $indexOfShortestMatch; } private function getComposerInformation(\SplFileInfo $file) From bc76e0014b59337d7b3f6b31e17daee3f4781797 Mon Sep 17 00:00:00 2001 From: Danack Date: Thu, 20 Feb 2014 17:30:51 +0000 Subject: [PATCH 3/6] Moved tests that are expected to work into their own directory. Added test for composer.json in incorrect directory. --- .../Repository/ArtifactRepositoryTest.php | 49 +++++++++++++++++- .../{ => correct}/composer-1.0.0-alpha6.zip | Bin .../{ => correct}/not-an-artifact.zip | Bin .../artifacts/{ => correct}/package0.zip | Bin .../artifacts/{ => correct}/package2.zip | Bin .../subfolder/not-an-artifact.zip | Bin .../{ => correct}/subfolder/package1.zip | Bin 7 files changed, 48 insertions(+), 1 deletion(-) rename tests/Composer/Test/Repository/Fixtures/artifacts/{ => correct}/composer-1.0.0-alpha6.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{ => correct}/not-an-artifact.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{ => correct}/package0.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{ => correct}/package2.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{ => correct}/subfolder/not-an-artifact.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{ => correct}/subfolder/package1.zip (100%) diff --git a/tests/Composer/Test/Repository/ArtifactRepositoryTest.php b/tests/Composer/Test/Repository/ArtifactRepositoryTest.php index 5ffae515a..f83ceee8c 100644 --- a/tests/Composer/Test/Repository/ArtifactRepositoryTest.php +++ b/tests/Composer/Test/Repository/ArtifactRepositoryTest.php @@ -26,9 +26,11 @@ class ArtifactRepositoryTest extends TestCase 'composer/composer-1.0.0-alpha6', 'vendor1/package2-4.3.2', 'vendor3/package1-5.4.3', + 'test/jsonInRoot-1.0.0', + 'test/jsonInFirstLevel-1.0.0' ); - $coordinates = array('type' => 'artifact', 'url' => __DIR__ . '/Fixtures/artifacts'); + $coordinates = array('type' => 'artifact', 'url' => __DIR__ . '/Fixtures/artifacts/correct'); $repo = new ArtifactRepository($coordinates, new NullIO(), new Config()); $foundPackages = array_map(function(BasePackage $package) { @@ -40,4 +42,49 @@ class ArtifactRepositoryTest extends TestCase $this->assertSame($expectedPackages, $foundPackages); } + + public function testExtractConfigFails() + { + $this->setExpectedException('RuntimeException', "Shouldn't have picked up composer.json from a location other than root or first level directory."); + + $coordinates = array('type' => 'artifact', 'url' => __DIR__ . '/Fixtures/artifacts/error/jsonWrongDirectory'); + new ArtifactRepository($coordinates, new NullIO(), new Config()); + } } + +//$archivesToCreate = array( +// 'jsonInRoot' => array( +// "extra.txt" => "Testing testing testing", +// "composer.json" => '{ "name": "test/jsonInRoot", "version": "1.0.0" }', +// "subdir/extra.txt" => "Testing testing testing", +// "subdir/extra2.txt" => "Testing testing testing", +// ), +// +// 'jsonInFirstLevel' => array( +// "extra.txt" => "Testing testing testing", +// "subdir/composer.json" => '{ "name": "test/jsonInFirstLevel", "version": "1.0.0" }', +// "subdir/extra.txt" => "Testing testing testing", +// "subdir/extra2.txt" => "Testing testing testing", +// ), +// +// 'jsonInSecondLevel' => array( +// "extra.txt" => "Testing testing testing", +// "subdir/extra1.txt" => "Testing testing testing", +// "subdir/foo/composer.json" => '{ "name": "test/jsonInSecondLevel", "version": "1.0.0" }', +// "subdir/foo/extra1.txt" => "Testing testing testing", +// "subdir/extra2.txt" => "Testing testing testing", +// "subdir/extra3.txt" => "Testing testing testing", +// ), +//); +// +//foreach($archivesToCreate as $archiveName => $fileDetails) { +// $zipFile = new ZipArchive(); +// $zipFile->open("$archiveName.zip", ZIPARCHIVE::CREATE); +// +// foreach ($fileDetails as $filename => $fileContents) { +// $zipFile->addFromString($filename, $fileContents); +// } +// +// $zipFile->close(); +//} + diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/composer-1.0.0-alpha6.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/correct/composer-1.0.0-alpha6.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/composer-1.0.0-alpha6.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/correct/composer-1.0.0-alpha6.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/not-an-artifact.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/correct/not-an-artifact.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/not-an-artifact.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/correct/not-an-artifact.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/package0.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/correct/package0.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/package0.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/correct/package0.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/package2.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/correct/package2.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/package2.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/correct/package2.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/not-an-artifact.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/not-an-artifact.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/not-an-artifact.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/not-an-artifact.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/package1.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/package1.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/package1.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/package1.zip From 24aba5b51fc90d0f15cb21f4f00e55cd1937bc9d Mon Sep 17 00:00:00 2001 From: Danack Date: Fri, 21 Feb 2014 09:44:04 +0000 Subject: [PATCH 4/6] Moved file back to correct location. (+1 squashed commit) Squashed commits: [eec32aa] Updated detection to only allow composer.josn in root or first level dir. --- src/Composer/Repository/ArtifactRepository.php | 12 ++++++++++++ .../Test/Repository/ArtifactRepositoryTest.php | 14 ++++---------- .../{correct => }/composer-1.0.0-alpha6.zip | Bin .../Fixtures/artifacts/jsonInFirstLevel.zip | Bin 0 -> 542 bytes .../Repository/Fixtures/artifacts/jsonInRoot.zip | Bin 0 -> 522 bytes .../Fixtures/artifacts/jsonInSecondLevel.zip | Bin 0 -> 805 bytes .../artifacts/{correct => }/not-an-artifact.zip | Bin .../Fixtures/artifacts/{correct => }/package0.zip | Bin .../Fixtures/artifacts/{correct => }/package2.zip | Bin .../{correct => }/subfolder/not-an-artifact.zip | Bin .../{correct => }/subfolder/package1.zip | Bin 11 files changed, 16 insertions(+), 10 deletions(-) rename tests/Composer/Test/Repository/Fixtures/artifacts/{correct => }/composer-1.0.0-alpha6.zip (100%) create mode 100644 tests/Composer/Test/Repository/Fixtures/artifacts/jsonInFirstLevel.zip create mode 100644 tests/Composer/Test/Repository/Fixtures/artifacts/jsonInRoot.zip create mode 100644 tests/Composer/Test/Repository/Fixtures/artifacts/jsonInSecondLevel.zip rename tests/Composer/Test/Repository/Fixtures/artifacts/{correct => }/not-an-artifact.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{correct => }/package0.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{correct => }/package2.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{correct => }/subfolder/not-an-artifact.zip (100%) rename tests/Composer/Test/Repository/Fixtures/artifacts/{correct => }/subfolder/package1.zip (100%) diff --git a/src/Composer/Repository/ArtifactRepository.php b/src/Composer/Repository/ArtifactRepository.php index 769e35b40..34ed64ea3 100644 --- a/src/Composer/Repository/ArtifactRepository.php +++ b/src/Composer/Repository/ArtifactRepository.php @@ -88,6 +88,18 @@ class ArtifactRepository extends ArrayRepository 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; + } + + if(strpos($directoryName, '\\') !== false || + strpos($directoryName, '/') !== false) { + continue; + } + $length = strlen($stat['name']); if ($indexOfShortestMatch == false || $length < $lengthOfShortestMatch) { //Check it's not a directory. diff --git a/tests/Composer/Test/Repository/ArtifactRepositoryTest.php b/tests/Composer/Test/Repository/ArtifactRepositoryTest.php index f83ceee8c..1958f5b9f 100644 --- a/tests/Composer/Test/Repository/ArtifactRepositoryTest.php +++ b/tests/Composer/Test/Repository/ArtifactRepositoryTest.php @@ -27,10 +27,12 @@ class ArtifactRepositoryTest extends TestCase 'vendor1/package2-4.3.2', 'vendor3/package1-5.4.3', 'test/jsonInRoot-1.0.0', - 'test/jsonInFirstLevel-1.0.0' + 'test/jsonInFirstLevel-1.0.0', + //The files not-an-artifact.zip and jsonSecondLevel are not valid + //artifacts and do not get detected. ); - $coordinates = array('type' => 'artifact', 'url' => __DIR__ . '/Fixtures/artifacts/correct'); + $coordinates = array('type' => 'artifact', 'url' => __DIR__ . '/Fixtures/artifacts'); $repo = new ArtifactRepository($coordinates, new NullIO(), new Config()); $foundPackages = array_map(function(BasePackage $package) { @@ -42,14 +44,6 @@ class ArtifactRepositoryTest extends TestCase $this->assertSame($expectedPackages, $foundPackages); } - - public function testExtractConfigFails() - { - $this->setExpectedException('RuntimeException', "Shouldn't have picked up composer.json from a location other than root or first level directory."); - - $coordinates = array('type' => 'artifact', 'url' => __DIR__ . '/Fixtures/artifacts/error/jsonWrongDirectory'); - new ArtifactRepository($coordinates, new NullIO(), new Config()); - } } //$archivesToCreate = array( diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/correct/composer-1.0.0-alpha6.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/composer-1.0.0-alpha6.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/correct/composer-1.0.0-alpha6.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/composer-1.0.0-alpha6.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInFirstLevel.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInFirstLevel.zip new file mode 100644 index 0000000000000000000000000000000000000000..498037464ca08d944c1d9349b59ab1c19ae47004 GIT binary patch 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 literal 0 HcmV?d00001 diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInRoot.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInRoot.zip new file mode 100644 index 0000000000000000000000000000000000000000..7b2a87eb9aea96dab1e4c2da86dff1332fd1de92 GIT binary patch literal 522 zcmWIWW@Zs#U|`^2n7%B?CH~jPFdiUJ9EdrAIJKgrC{eGZqJ-O1SMTH*?{E#rrHlb6 zTBIxrHW~qyf-o--C+FuD17q?=dBJ42sq_^#y50RP=FTDh>Pd7&Yk~!hWpfc z{lH6s0b1U=TIbH3XFe(QO7exIhUOI}22{h5-6H@rytp(eC9_DM2xlSd6hzZ$gy|zj zCI$xF!2r~c1mLy@c%y1V4+?}HV;~bHV9>Q9I~U|p1egG1qI(;kZjfgY;1rOFtQ+k0 T0B=?{kP;Rkd=I2q89_V%Pk?%l literal 0 HcmV?d00001 diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInSecondLevel.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/jsonInSecondLevel.zip new file mode 100644 index 0000000000000000000000000000000000000000..0e5abc61b79424258c13ea520717c6327d1f3fc1 GIT binary patch literal 805 zcmWIWW@Zs#U|`^2n7%B?CH~jPFdiUJ9EdrAIJKgrC{eGZqJ-O1SMTH*?{E#rrHlbM zwFm;W6qhEYWESazbsFN=y8rr=W0pWIAS{8=Dohg>|bb?*G)e_1%$i15j4v1m1g&6Y z!0l2|RO^UwEpphPd(;TONAYPj#?;El#K3?%PJk{)0`M3L@W!nTBp_yB005NB#IFDV literal 0 HcmV?d00001 diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/correct/not-an-artifact.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/not-an-artifact.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/correct/not-an-artifact.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/not-an-artifact.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/correct/package0.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/package0.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/correct/package0.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/package0.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/correct/package2.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/package2.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/correct/package2.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/package2.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/not-an-artifact.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/not-an-artifact.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/not-an-artifact.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/not-an-artifact.zip diff --git a/tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/package1.zip b/tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/package1.zip similarity index 100% rename from tests/Composer/Test/Repository/Fixtures/artifacts/correct/subfolder/package1.zip rename to tests/Composer/Test/Repository/Fixtures/artifacts/subfolder/package1.zip From e4d4a7ae1734e4aaeca6b6b93d04543588a1871a Mon Sep 17 00:00:00 2001 From: Danack Date: Fri, 21 Feb 2014 09:49:12 +0000 Subject: [PATCH 5/6] Added comment for generating files. --- tests/Composer/Test/Repository/ArtifactRepositoryTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Composer/Test/Repository/ArtifactRepositoryTest.php b/tests/Composer/Test/Repository/ArtifactRepositoryTest.php index 1958f5b9f..361f372ba 100644 --- a/tests/Composer/Test/Repository/ArtifactRepositoryTest.php +++ b/tests/Composer/Test/Repository/ArtifactRepositoryTest.php @@ -46,6 +46,8 @@ class ArtifactRepositoryTest extends TestCase } } +//Files jsonInFirstLevel.zip, jsonInRoot.zip and jsonInSecondLevel.zip were generated with: +// //$archivesToCreate = array( // 'jsonInRoot' => array( // "extra.txt" => "Testing testing testing", From 20a7dcd02c09168dfaabcf0b2048531ecaac1db0 Mon Sep 17 00:00:00 2001 From: Danack Date: Fri, 21 Feb 2014 09:54:42 +0000 Subject: [PATCH 6/6] Added explanation of why loop continues. --- src/Composer/Repository/ArtifactRepository.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Composer/Repository/ArtifactRepository.php b/src/Composer/Repository/ArtifactRepository.php index 34ed64ea3..09efc0b9f 100644 --- a/src/Composer/Repository/ArtifactRepository.php +++ b/src/Composer/Repository/ArtifactRepository.php @@ -97,6 +97,7 @@ class ArtifactRepository extends ArrayRepository if(strpos($directoryName, '\\') !== false || strpos($directoryName, '/') !== false) { + //composer.json files below first directory are rejected continue; }