The following commit has been merged in the master branch: commit a82c44c4ae20fdff7c9b083e75a1b28a528b59c6 Author: Raphael Hertzog <hert...@debian.org> Date: Sat Feb 21 21:08:01 2009 +0100
update-alternatives: stricter validation of --install parameters Fail if --install tries to reuse links owned by other alternatives, or if the alternative type (slave/master) doesn't match the information already available. Closes: #342566 Modify test-suite to verify this behaviour. diff --git a/ChangeLog b/ChangeLog index 59f5a57..c3682fb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2009-02-22 Raphael Hertzog <hert...@debian.org> + * scripts/update-alternatives.pl: Checks that --install does not + reuse alternatives or links in a way that is not compatible with + the current setup. + * scripts/t/900_update_alternatives.t: Add corresponding tests in + the test suite. + +2009-02-22 Raphael Hertzog <hert...@debian.org> + * scripts/update-alternatives.pl: Add logging to /var/log/dpkg.log. diff --git a/debian/changelog b/debian/changelog index 49d0f70..df96417 100644 --- a/debian/changelog +++ b/debian/changelog @@ -157,6 +157,9 @@ dpkg (1.15.0) UNRELEASED; urgency=low implement new features on top of it: - the --config output is now sorted. Closes: #437060 - it now logs information to /var/log/dpkg.log. Closes: #445270 + - it forbids reusing master alternative as slave and vice-versa. + Closes: #342566 + - it forbids reusing alternative links managed by other alternatives [ Pierre Habouzit ] * Add a --query option to update-alternatives. Closes: #336091, #441904 diff --git a/scripts/t/900_update_alternatives.t b/scripts/t/900_update_alternatives.t index ad7bd89..4d1004b 100644 --- a/scripts/t/900_update_alternatives.t +++ b/scripts/t/900_update_alternatives.t @@ -53,7 +53,7 @@ my @choices = ( ); my $nb_slaves = 2; plan tests => (4 * ($nb_slaves + 1) + 2) * 18 # number of check_choices - + 39; # rest + + 44; # rest sub cleanup { system("rm -rf $srcdir/t.tmp/ua && mkdir -p $admindir && mkdir -p $altdir"); @@ -264,5 +264,22 @@ cleanup(); system("echo garbage > $admindir/generic-test"); install_choice(0, error_to_file => "/dev/null", expect_failure => 1); -# TODO: add checks for invalid values of parameters, add checks for -# usage of links as both master and slave, install in non-existing dir +# test invalid usages +cleanup(); +install_choice(0); +# try to install a slave alternative as new master +call_ua(["--install", "$bindir/testmaster", "slave1", "/bin/date", "10"], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# try to install a master alternative as slave +call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10", + "--slave", "$bindir/testslave", "generic-test", "/bin/true" ], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# try to reuse links in master alternative +call_ua(["--install", "$bindir/slave1", "testmaster", "/bin/date", "10"], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# try to reuse links in slave alternative +call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10", + "--slave", "$bindir/generic-test", "testslave", "/bin/true" ], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); + +# TODO: add checks for invalid values of parameters, install in non-existing dir diff --git a/scripts/update-alternatives.pl b/scripts/update-alternatives.pl index 5076d84..2f5bb35 100755 --- a/scripts/update-alternatives.pl +++ b/scripts/update-alternatives.pl @@ -108,6 +108,47 @@ while (@ARGV) { badusage(_g("need --display, --query, --list, --config, --set, --install," . "--remove, --all, --remove-all or --auto")) unless $action; +# Load infos about all alternatives to be able to check for mistakes +my %ALL; +foreach my $alt_name (get_all_alternatives()) { + my $alt = Alternative->new($alt_name); + next unless $alt->load("$admdir/$alt_name", 1); + $ALL{objects}{$alt_name} = $alt; + $ALL{links}{$alt->link()} = $alt_name; + $ALL{parent}{$alt_name} = $alt_name; + foreach my $slave ($alt->slaves()) { + $ALL{links}{$alt->slave_link($slave)} = $slave; + $ALL{parent}{$slave} = $alt_name; + } +} +# Check that caller don't mix links between alternatives and don't mix +# alternatives between +if ($action eq "install") { + my ($name, $link) = ($inst_alt->name(), $inst_alt->link()); + if (exists $ALL{parent}{$name} and $ALL{parent}{$name} ne $name) { + badusage(_g("Alternative %s can't be master: %s"), $name, + sprintf(_g("it is a slave of %s"), $ALL{parent}{$name})); + } + if (exists $ALL{links}{$link} and $ALL{links}{$link} ne $name) { + badusage(_g("Alternative link %s is already managed by %s."), + $link, $ALL{parent}{$ALL{links}{$link}}); + } + foreach my $slave ($inst_alt->slaves()) { + $link = $inst_alt->slave_link($slave); + if (exists $ALL{parent}{$slave} and $ALL{parent}{$slave} ne $name) { + badusage(_g("Alternative %s can't be slave of %s: %s"), + $slave, $name, ($ALL{parent}{$slave} eq $slave) ? + _g("it is a master alternative.") : + sprintf(_g("it is a slave of %s"), $ALL{parent}{$slave}) + ); + } + if (exists $ALL{links}{$link} and $ALL{links}{$link} ne $slave) { + badusage(_g("Alternative link %s is already managed by %s."), + $link, $ALL{parent}{$ALL{links}{$link}}); + } + } +} + # Handle actions if ($action eq 'all') { config_all(); @@ -373,12 +414,16 @@ sub set_action { } } -sub config_all { +sub get_all_alternatives { opendir(ADMINDIR, $admdir) or quit(_g("can't readdir %s: %s"), $admdir, $!); - my @filenames = grep !/^\.\.?$/, readdir ADMINDIR; + my @filenames = grep { !/^\.\.?$/ and !/\.dpkg-tmp$/ } (readdir(ADMINDIR)); close(ADMINDIR); - foreach my $name (@filenames) { + return sort @filenames; +} + +sub config_all { + foreach my $name (get_all_alternatives()) { system "$0 $skip --config $name"; exit $? if $?; print "\n"; -- dpkg's main repository -- To UNSUBSCRIBE, email to debian-dpkg-cvs-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org