-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Verify package name(s) when using an update whitelist #1112
Changes from all commits
c99607d
8e53ffb
ef5f64d
7a1a4f2
3af6f30
49df80a
86f616f
6fb616a
64e3861
48ef987
41adbb7
e0553f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of Composer. | ||
* | ||
* (c) Nils Adermann <naderman@naderman.de> | ||
* Jordi Boggiano <j.boggiano@seld.be> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong indentation here (the |
||
|
||
namespace Composer\Exception; | ||
|
||
/** | ||
* Unknown package exception. | ||
* | ||
* Used when a package isn't found in a list of valid packages. | ||
* | ||
* @author Chris Wilkinson <chriswilkinson84@gmail.com> | ||
*/ | ||
class UnknownPackageException extends \UnexpectedValueException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
use Composer\DependencyResolver\Solver; | ||
use Composer\DependencyResolver\SolverProblemsException; | ||
use Composer\Downloader\DownloadManager; | ||
use Composer\Exception\UnknownPackageException; | ||
use Composer\Installer\InstallationManager; | ||
use Composer\Config; | ||
use Composer\Installer\NoopInstaller; | ||
|
@@ -740,12 +741,44 @@ public function setVerbose($verbose = true) | |
* restrict the update operation to a few packages, all other packages | ||
* that are already installed will be kept at their current version | ||
* | ||
* @param array $packages | ||
* @param array $packages Array of package names | ||
* @return Installer | ||
* @throws UnknownPackageException If a package name is not known | ||
*/ | ||
public function setUpdateWhitelist(array $packages) | ||
{ | ||
$this->updateWhitelist = array_flip(array_map('strtolower', $packages)); | ||
if (count($packages) === 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if (count($packages) === 0 || (isset($packages[0]) && strtolower($packages[0]) === 'nothing')) { With such you could remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stloyd changing only the condition here would not be equivalent |
||
$this->updateWhitelist = array(); | ||
return $this; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not equivalent to the previous code. You should reset the update whitelist to an empty array here, otherwise resetting it is not possible anymore |
||
} | ||
|
||
$lowercasePackages = array_map('strtolower', $packages); | ||
|
||
if (count($packages) > 1 || $packages[0] !== 'nothing') { | ||
$knownPackages = array(); | ||
foreach ($this->repositoryManager->getLocalRepository()->getPackages() as $localPackage) { | ||
$knownPackages = array_merge($knownPackages, $localPackage->getNames()); | ||
} | ||
foreach ($this->package->getRequires() as $requiredPackage) { | ||
$knownPackages[] = $requiredPackage->getTarget(); | ||
} | ||
if ($this->devMode) { | ||
foreach ($this->repositoryManager->getLocalDevRepository()->getPackages() as $localPackage) { | ||
$knownPackages = array_merge($knownPackages, $localPackage->getNames()); | ||
} | ||
foreach ($this->package->getDevRequires() as $requiredPackage) { | ||
$knownPackages[] = $requiredPackage->getTarget(); | ||
} | ||
} | ||
|
||
foreach ($lowercasePackages as $key => $package) { | ||
if (!in_array($package, $knownPackages)) { | ||
throw new UnknownPackageException('Package ' . $packages[$key] . ' not known'); | ||
} | ||
} | ||
} | ||
|
||
$this->updateWhitelist = array_flip($lowercasePackages); | ||
|
||
return $this; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a great idea, if you throw a very specific exception and catch that ok, but as such it will basically prevent us from seeing the backtrace when a real exception occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point; there aren't any Composer exceptions yet, ok to create something like
Composer\Exception\UnknownPackageNameException
which extends\UnexpectedValueException
? Couldn't see a non-exception method to halt the flow here (without rather ugly refactoring).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's alright (the new exception)