The following commit has been merged in the master branch:
commit 7b79d64a9721db92a02309562244d3bb61d40a5f
Author: Raphael Hertzog <[email protected]>
Date: Sat Feb 21 22:22:29 2009 +0100
update-alternatives: add more checks on --install parameters
Check that links and alternative paths are absolute. Check that the
master alternative path exists. Forbid "/" and spaces in alternative names.
Closes: #423176
Add corresponding tests in the test-suite.
diff --git a/ChangeLog b/ChangeLog
index c3682fb..d9b42de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2009-02-22 Raphael Hertzog <[email protected]>
+ * scripts/update-alternatives.pl: Add more sanity check for
+ --install: ensure alternative path and links are absolute, ensure
+ master alternative path exists, forbid / and spaces in alternative
+ names.
+ * scripts/t/900_update_alternatives.t: Add corresponding tests in
+ the test suite.
+
+2009-02-22 Raphael Hertzog <[email protected]>
+
* scripts/update-alternatives.pl: Checks that --install does not
reuse alternatives or links in a way that is not compatible with
the current setup.
diff --git a/debian/changelog b/debian/changelog
index df96417..ce32c5c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -160,6 +160,7 @@ dpkg (1.15.0) UNRELEASED; urgency=low
- it forbids reusing master alternative as slave and vice-versa.
Closes: #342566
- it forbids reusing alternative links managed by other alternatives
+ - new sanity checks on --install parameters. Closes: #423176
[ 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 4d1004b..997d663 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
- + 44; # rest
+ + 49; # rest
sub cleanup {
system("rm -rf $srcdir/t.tmp/ua && mkdir -p $admindir && mkdir -p
$altdir");
@@ -281,5 +281,21 @@ call_ua(["--install", "$bindir/slave1", "testmaster",
"/bin/date", "10"],
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");
+# lack of absolute filenames in links or file path, non-existing path,
+call_ua(["--install", "../testmaster", "testmaster", "/bin/date", "10"],
+ expect_failure => 1, to_file => "/dev/null", error_to_file =>
"/dev/null");
+call_ua(["--install", "$bindir/testmaster", "testmaster",
"./update-alternatives.pl", "10"],
+ expect_failure => 1, to_file => "/dev/null", error_to_file =>
"/dev/null");
+# non-existing alternative path
+call_ua(["--install", "$bindir/testmaster", "testmaster",
"$bindir/doesntexist", "10"],
+ expect_failure => 1, to_file => "/dev/null", error_to_file =>
"/dev/null");
+# invalid alternative name in master
+call_ua(["--install", "$bindir/testmaster", "test/master", "/bin/date", "10"],
+ expect_failure => 1, to_file => "/dev/null", error_to_file =>
"/dev/null");
+# invalid alternative name in slave
+call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10",
+ "--slave", "$bindir/testslave", "test slave", "/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
+# TODO: install in non-existing dir, handle of pre-existing files in place
+# of alternative links
diff --git a/scripts/update-alternatives.pl b/scripts/update-alternatives.pl
index 2f5bb35..cdf6675 100755
--- a/scripts/update-alternatives.pl
+++ b/scripts/update-alternatives.pl
@@ -122,9 +122,10 @@ foreach my $alt_name (get_all_alternatives()) {
}
}
# Check that caller don't mix links between alternatives and don't mix
-# alternatives between
+# alternatives between slave/master, and that the various parameters
+# are fine
if ($action eq "install") {
- my ($name, $link) = ($inst_alt->name(), $inst_alt->link());
+ my ($name, $link, $file) = ($inst_alt->name(), $inst_alt->link(),
$fileset->master());
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}));
@@ -133,8 +134,16 @@ if ($action eq "install") {
badusage(_g("Alternative link %s is already managed by %s."),
$link, $ALL{parent}{$ALL{links}{$link}});
}
+ badusage(_g("Alternative link (%s) is not absolute as it should be."),
+ $link) unless $link =~ m|^/|;
+ badusage(_g("Alternative path (%s) is not absolute as it should be."),
+ $file) unless $file =~ m|^/|;
+ badusage(_g("Alternative path (%s) doesn't exist."), $file)
+ unless -e $file;
+ badusage(_g("Alternative name (%s) is invalid."), $name) if $name =~
m|[/\s]|;
foreach my $slave ($inst_alt->slaves()) {
$link = $inst_alt->slave_link($slave);
+ $file = $fileset->slave($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) ?
@@ -146,6 +155,12 @@ if ($action eq "install") {
badusage(_g("Alternative link %s is already managed by %s."),
$link, $ALL{parent}{$ALL{links}{$link}});
}
+ badusage(_g("Alternative link (%s) is not absolute as it should be."),
+ $link) unless $link =~ m|^/|;
+ badusage(_g("Alternative path (%s) is not absolute as it should be."),
+ $file) unless $file =~ m|^/|;
+ badusage(_g("Alternative name (%s) is invalid."), $slave)
+ if $slave =~ m|[/\s]|;
}
}
--
dpkg's main repository
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
