[debhelper-devel] [debhelper] 02/08: Dh_Lib: Fix shell bug and add stdout redirect

Niels Thykier nthykier at moszumanska.debian.org
Sun Jul 16 12:43:50 UTC 2017


This is an automated email from the git hooks/post-receive script.

nthykier pushed a commit to branch shell-out-less
in repository debhelper.

commit 6826fbf31fb5f757babbf32f0adda292eddb046e
Author: Niels Thykier <niels at thykier.net>
Date:   Sat Jul 15 10:05:13 2017 +0000

    Dh_Lib: Fix shell bug and add stdout redirect
    
    Signed-off-by: Niels Thykier <niels at thykier.net>
---
 Debian/Debhelper/Dh_Lib.pm | 52 +++++++++++++++++++++++++++++++++-------------
 debhelper.pod              | 10 +++++++++
 debian/changelog           |  7 +++++++
 doc/PROGRAMMING            | 20 ++++++++++++++----
 4 files changed, 71 insertions(+), 18 deletions(-)

diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index 95b8f13..14bfcfe 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -286,33 +286,57 @@ sub escape_shell {
 # Note that this cannot handle complex commands, especially anything
 # involving redirection. Use complex_doit instead.
 sub doit {
-	doit_noerror(@_) || error_exitcode(join(" ", @_));
+	doit_noerror(@_) || error_exitcode(_format_cmdline(@_));
 }
 
 sub doit_noerror {
-	verbose_print(escape_shell(@_));
+	verbose_print(_format_cmdline(@_)) if $dh{VERBOSE};
 
-	if (! $dh{NO_ACT}) {
-		return (system(@_) == 0)
-	}
-	else {
-		return 1;
-	}
+	goto \&_doit;
 }
 
 sub print_and_doit {
-	print_and_doit_noerror(@_) || error_exitcode(join(" ", @_));
+	print_and_doit_noerror(@_) || error_exitcode(_format_cmdline(@_));
 }
 
 sub print_and_doit_noerror {
-	nonquiet_print(escape_shell(@_));
+	nonquiet_print(_format_cmdline(@_));
 
-	if (! $dh{NO_ACT}) {
-		return (system(@_) == 0)
+	goto \&_doit;
+}
+
+sub _doit {
+	my (@cmd) = @_;
+	my $options = ref($cmd[0]) ? shift(@cmd) : undef;
+	# In compat <= 10, we warn, compat 11 we detect and error, in
+	# compat 12 we assume peolpe know what they are doing.
+	if (not defined($options) and @cmd == 1 and not compat(11) and $cmd[0] =~ m/[\s<&>|;]/) {
+		deprecated_functionality('doit() + doit_*() calls will no longer spawn a shell in compat 11 for single string arguments (please use complex_doit instead)',
+								 11);
+		return 1 if $dh{NO_ACT};
+		return system(@cmd) == 0;
 	}
-	else {
-		return 1;
+	return 1 if $dh{NO_ACT};
+	my $pid = fork() // error("fork(): $!");
+	if (not $pid) {
+		if (defined($options)) {
+			if (defined(my $output = $options->{stdout})) {
+				open(STDOUT, '>', $output) or error("redirect STDOUT failed: $!");
+			}
+		}
+		exec(@cmd);
+	}
+	return waitpid($pid, 0) == $pid && $? == 0;
+}
+
+sub _format_cmdline {
+	my (@cmd) = @_;
+	my $options = ref($cmd[0]) ? shift(@cmd) : {};
+	my $cmd_line = escape_shell(@cmd);
+	if (defined(my $output = $options->{stdout})) {
+		$cmd_line .= ' > ' . escape_shell($output);
 	}
+	return $cmd_line;
 }
 
 # Run a command and display the command to stdout if verbose mode is on.
diff --git a/debhelper.pod b/debhelper.pod
index 036bde2..03fec65 100644
--- a/debhelper.pod
+++ b/debhelper.pod
@@ -679,6 +679,16 @@ to drop support for the B<PERL_USE_UNSAFE_INC> environment variable.
 When perl drops support for it, then this variable will be removed
 retroactively from existing compat levels as well.
 
+=item -
+
+There was a bug in the B<doit> (and similar) functions from
+L<Debian::Debhelper::Dh_Lib> that made them spawn a shell in one
+particular circumstance.  This bug is now removed and will cause
+helpers that rely on the bug to fail with an error.
+
+In compatibility 11, there is still a detection for this case.  This
+detection will be removed in compatibility level 12.
+
 =back
 
 =back
diff --git a/debian/changelog b/debian/changelog
index 458c891..3915710 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,13 @@ debhelper (10.7) UNRELEASED; urgency=medium
   * dh_installmodules: Ensure maintscripts are reproducible even with
     multiple kernel versions detected.
   * Apply patches from gregor herrmann to improve autopkgtests.
+  * Dh_Lib: Fix bug in doit + doit_* that made them fork a shell in
+    some cases.  For backwards compatibility, there is detection code
+    that should make warn for this case.  This can cause a weird
+    "Please specify the compatibility level in debian/compat" error if
+    the tools have chdir to a different directory.
+  * Dh_Lib: Support an optional hashref in doit + doit_* to enable some
+    trivial operations in the child process (e.g. redirect stdout).
 
  -- Niels Thykier <niels at thykier.net>  Sat, 15 Jul 2017 09:42:32 +0000
 
diff --git a/doc/PROGRAMMING b/doc/PROGRAMMING
index fa87fc5..1115d99 100644
--- a/doc/PROGRAMMING
+++ b/doc/PROGRAMMING
@@ -155,13 +155,25 @@ Functions:
 
 Dh_Lib.pm also contains a number of functions you may find useful.
 
-doit(@command)
-	Pass this function an array that is a 
-	shell command. It will run the command (unless $dh{NO_ACT} is set), and
+doit([$options, ]@command)
+	Pass this function an array that is a command with arguments.
+	It will run the command (unless $dh{NO_ACT} is set), and
 	if $dh{VERBOSE} is set, it will also output the command to stdout. You
 	should use this function for almost all commands your program performs
 	that manipulate files in the package build directories.
-print_and_doit(@command)
+
+	The $options argument (if passed) must be a hashref (added in debhelper 10.7).
+	The following key-value pairs can be used:
+	  * stdout => A file name.  The child process will have its STDOUT redirected
+	    to that file.  [debhelper (>= 10.7)]
+
+	This will *not* invoke a shell, so meta characters will not have any special
+	meaning.  Use complex_doit for that.
+	NB: In compat 10 and below, there was a bug that would make doit fork a shell
+	in one special case.  This is deprecated and will be removed in compat 11.
+	The detection code for this can be disabled by passing an empty hashref for
+	as $options.  This will make doit unconditionally avoid forking a shell.
+print_and_doit([$options, ]@command)
         Like doit but will print unless $dh{QUIET} is set. See "Standardization"
         above for when this is allowed to be called.
 complex_doit($command)

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/debhelper/debhelper.git




More information about the debhelper-devel mailing list