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 <peter_at_lekensteyn.nl>

---
 tests/runtests.pl | 66 ++++++++++++++++++++++---------------------------------
 1 file changed, 26 insertions(+), 40 deletions(-)
diff --git a/tests/runtests.pl b/tests/runtests.pl
index 265b9f8..c16452a 100755
--- a/tests/runtests.pl
+++ b/tests/runtests.pl
@@ -266,7 +266,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
@@ -509,6 +508,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 = @_;
@@ -533,6 +533,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);
     }
@@ -3123,17 +3125,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);
 
@@ -3148,31 +3139,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";
             }
         }
     }
@@ -3206,6 +3195,7 @@ sub singletest {
                     cmd     => \@p,
                     stderr  => File::Spec->devnull,
                     stdout  => \$out,
+                    env     => \%env,
                 );
                 my $rc = runclient(%cmd);
                 $why = (split(/\n/, $out))[0];
@@ -3375,6 +3365,7 @@ sub singletest {
     my %test_command = (
         stdout  => $STDOUT,
         stderr  => $STDERR,
+        env     => \%env,
     );
     my $CMDLINE;
     my $cmdargs;
@@ -3515,6 +3506,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);
@@ -3704,6 +3701,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.
@@ -3716,18 +3714,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