On Thursday 13 November 2014 15:45:19 Peter Wu wrote:
> Another round of patches around the test suite. This time the patches
> fix tests that run under LD_PRELOAD=libhostname.so and when cURL is
> built with Address Sanitizer (-fsanitize=address).
>
> The first patch ("runtests.pl: simplify command environment handling")
> actually contains three changes:
> - Convert from shell commands to an array of arguments.
> - Implement a subset of IPC::Run as AIX does not have this module.
> - Implement a custom subroutine that splits a shell command into an
> array. This is done because Text::ParseWords is broken and does not
> handle some cases correctly.
>
> The second patch further restricts the scope of environment variables.
>
> Reviews / comments are appreciated!
>
> Kind regards,
> Peter
>
> Peter Wu (2):
> runtests.pl: safer system() calls, fix LD_PRELOAD
> runtests.pl: simplify command environment handling
>
> tests/runtests.pl | 446
> +++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 287 insertions(+), 159 deletions(-)
Some last minute changes to replace IPC::Run and Text::ParseWords
introduced bugs (in runclient and splitcmd respectively). With the third
public revision of the "safer system calls" patch, all tests pass again
as usual.
Due to IPC::Run avoidance, the file grew a bit, but it will properly
handle the documented behavior. Fun fact about the splitcmd changes, the
regex looks simpler but it seems to be a bit faster on parsing commands
compared to Text::ParseWords, this was measured by passing all commands
from the test suite through the subroutine.
These two patches can also be found at
https://github.com/Lekensteyn/curl/compare/997eecd%5E...9a87bdc
Output of `perl -I$srcdir $srcdir/runtests.pl -n -a -k`:
TESTDONE: 794 tests out of 794 reported OK: 100%
TESTDONE: 955 tests were considered during 296 seconds.
TESTINFO: 161 tests were skipped due to these restraints:
TESTINFO: "curl lacks unittest support" 16 times (1300, 1301, 1302, 1303, 1304,
1305, 1306, 1307, 1308 and 7 more)
TESTINFO: "curl lacks large_file support" 3 times (99, 1044, 1063)
TESTINFO: "curl lacks debug support" 76 times (67, 68, 69, 81, 89, 90, 91, 150,
155 and 67 more)
TESTINFO: "failed starting HTTP-pipe server" 4 times (1900, 1901, 1902, 1903)
TESTINFO: "curl lacks TLS-SRP support" 5 times (320, 321, 322, 323, 324)
TESTINFO: "no stunnel" 28 times (300, 301, 302, 303, 304, 305, 306, 307, 308
and 19 more)
TESTINFO: "curl lacks Metalink support" 16 times (2005, 2008, 2009, 2010, 2011,
2012, 2013, 2014, 2015 and 7 more)
TESTINFO: "curl lacks TrackMemory support" 2 times (96, 558)
TESTINFO: "Resolving IPv6 'ip6-localhost' didn't work" 2 times (241, 1083)
TESTINFO: "curl lacks NTLM_WB support" 1 times (1310)
TESTINFO: "configured as DISABLED" 8 times (594, 836, 882, 938, 1209, 1211,
1316, 1512)
Kind regards,
Peter Wu
https://lekensteyn.nl
>From 9a87bdc6fd44dbca055176cd58c71a20323c8d80 Mon Sep 17 00:00:00 2001
From: Peter Wu <[email protected]>
Date: Sun, 2 Nov 2014 10:52:10 +0100
Subject: [PATCH 2/2] runtests.pl: simplify command environment handling
Rather than saving valid environment variables for restoration later,
use 'local' to limit the environment changes to the block executing
a command.
Rationale: LD_PRELOAD may not work with all executables. Previously
it would break on the shell when compiled with AddressSanitizer
(-fsanitize=address), but that is avoided by not executing string
commands anymore. gdb would still be executed with LD_PRELOAD though,
so setting $ENV in this way was not sustainable.
After this patch, environment variables are passed to executions of
precheck (for chkhostname), command (for curl) and postcheck (for
symmetry with precheck, it does not actually use it).
Note that this patch does not help with an autotools build which uses
shared libs and AddressSanitizer as the 'curl' program is a libtool
wrapper (shell script).
Signed-off-by: Peter Wu <[email protected]>
---
tests/runtests.pl | 66 ++++++++++++++++++++++---------------------------------
1 file changed, 26 insertions(+), 40 deletions(-)
diff --git a/tests/runtests.pl b/tests/runtests.pl
index f2180ae..93b1814 100755
--- a/tests/runtests.pl
+++ b/tests/runtests.pl
@@ -267,7 +267,6 @@ my %timesrvrlog; # timestamp for each test server logs lock removal
my %timevrfyend; # timestamp for each test result verification end
my $testnumcheck; # test number, set in singletest sub.
-my %oldenv;
#######################################################################
# variables that command line options may set
@@ -510,6 +509,7 @@ sub checktestcmd {
# - stdout and stderr are either strings (a filename or '&1' for stdout) or
# a reference to store the output. Note: stdout is always redirected before
# stderr.
+# - env is an optional hash which sets env vars for the child process.
#
sub runclient {
my %args = @_;
@@ -554,6 +554,8 @@ sub runclient {
}
}
+ # Export env vars and prevent leaking vars such as LD_PRELOAD to sh.
+ local %ENV = (%ENV, %{$args{env}}) if $args{env};
# Try to invoke the command or exit
exec(@{$args{cmd}}) or exit(127);
}
@@ -3156,17 +3158,6 @@ sub singletest {
# this is done this early, so that the precheck can use environment
# variables and still bail out fine on errors
- # restore environment variables that were modified in a previous run
- foreach my $var (keys %oldenv) {
- if($oldenv{$var} eq 'notset') {
- delete $ENV{$var} if($ENV{$var});
- }
- else {
- $ENV{$var} = $oldenv{$var};
- }
- delete $oldenv{$var};
- }
-
# remove test server commands file before servers are started/verified
unlink($FTPDCMD) if(-f $FTPDCMD);
@@ -3181,31 +3172,29 @@ sub singletest {
$timesrvrend{$testnum} = Time::HiRes::time() if($timestats);
my @setenv = getpart("client", "setenv");
+ # Environment variables for precheck, command and postcheck. Since
+ # LD_PRELOAD may not be able to load in all programs (for example, when
+ # compiling with AddressSanitizer enabled), only apply it to programs built
+ # in the source tree, and not perl, sh, etc.
+ my %env;
if(@setenv) {
foreach my $s (@setenv) {
chomp $s;
subVariables \$s;
if($s =~ /([^=]*)=(.*)/) {
my ($var, $content) = ($1, $2);
- # remember current setting, to restore it once test runs
- $oldenv{$var} = ($ENV{$var})?"$ENV{$var}":'notset';
# set new value
- if(!$content) {
- delete $ENV{$var} if($ENV{$var});
- }
- else {
- if($var =~ /^LD_PRELOAD/) {
- if(exe_ext() && (exe_ext() eq '.exe')) {
- # print "Skipping LD_PRELOAD due to lack of OS support\n";
- next;
- }
- if($debug_build || ($has_shared ne "yes")) {
- # print "Skipping LD_PRELOAD due to no release shared build\n";
- next;
- }
+ if($var =~ /^LD_PRELOAD/) {
+ if(exe_ext() && (exe_ext() eq '.exe')) {
+ # print "Skipping LD_PRELOAD due to lack of OS support\n";
+ next;
+ }
+ if($debug_build || ($has_shared ne "yes")) {
+ # print "Skipping LD_PRELOAD due to no release shared build\n";
+ next;
}
- $ENV{$var} = "$content";
}
+ $env{$var} = "$content";
}
}
}
@@ -3239,6 +3228,7 @@ sub singletest {
cmd => \@p,
stderr => File::Spec->devnull,
stdout => \$out,
+ env => \%env,
);
my $rc = runclient(%cmd);
$why = (split(/\n/, $out))[0];
@@ -3408,6 +3398,7 @@ sub singletest {
my %test_command = (
stdout => $STDOUT,
stderr => $STDERR,
+ env => \%env,
);
my $CMDLINE;
my $cmdargs;
@@ -3548,6 +3539,12 @@ sub singletest {
my $gdbinit = "$TESTDIR/gdbinit$testnum";
open(GDBCMD, ">$LOGDIR/gdbcmd");
print GDBCMD "set args $cmdargs\n";
+ keys %env;
+ while (my ($k, $v) = each %env) {
+ print GDBCMD "set environment $k = $v\n";
+ }
+ # Do not invoke using a shell as LD_PRELOAD may crash it.
+ print GDBCMD "set startup-with-shell off\n";
print GDBCMD "show args\n";
print GDBCMD "source $gdbinit\n" if -e $gdbinit;
close(GDBCMD);
@@ -3737,6 +3734,7 @@ sub singletest {
my @cmda = splitcmd("$cmd");
my $rc = runclient(
cmd => \@cmda,
+ env => \%env,
);
# Must run the postcheck command in torture mode in order
# to clean up, but the result can't be relied upon.
@@ -3749,18 +3747,6 @@ sub singletest {
}
}
- # restore environment variables that were modified
- if(%oldenv) {
- foreach my $var (keys %oldenv) {
- if($oldenv{$var} eq 'notset') {
- delete $ENV{$var} if($ENV{$var});
- }
- else {
- $ENV{$var} = "$oldenv{$var}";
- }
- }
- }
-
# Skip all the verification on torture tests
if ($torture) {
if(!$cmdres && !$keepoutfiles) {
--
2.1.2
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
