Clear environment variables for nacl_helper

base/process/process_util_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #define _CRT_SECURE_NO_WARNINGS
 
 #include <limits>
 
 #include "base/command_line.h"
 #include "base/debug/alias.h"
@@ skipping 545 matching lines @@
   DPCHECK(ret == 0);
   ret = IGNORE_EINTR(close(sockets[1]));
   DPCHECK(ret == 0);
   ret = IGNORE_EINTR(close(dev_null));
   DPCHECK(ret == 0);
 }
 
 namespace {
 
 std::string TestLaunchProcess(const base::EnvironmentMap& env_changes,
+                              const bool clear_environ,
                               const int clone_flags) {
   std::vector<std::string> args;
   base::FileHandleMappingVector fds_to_remap;
 
   args.push_back(kPosixShell);
   args.push_back("-c");
   args.push_back("echo $BASE_TEST");

[Mark Seaborn, 2014/06/02 23:10:29]

Could you add a separate test that runs /usr/bin/env to check that no env vars
are set?

The current test doesn't actually test that BASE_TEST is absent.  It would pass
if BASE_TEST were set to the empty string.

It also doesn't check that other env vars aren't getting tacked onto the
environment somewhere.

[elijahtaylor1, 2014/06/03 20:47:54]

Done.

 
   int fds[2];
   PCHECK(pipe(fds) == 0);
 
   fds_to_remap.push_back(std::make_pair(fds[1], 1));
   base::LaunchOptions options;
   options.wait = true;
   options.environ = env_changes;
+  options.clear_environ = clear_environ;
   options.fds_to_remap = &fds_to_remap;
 #if defined(OS_LINUX)
   options.clone_flags = clone_flags;
 #else
   CHECK_EQ(0, clone_flags);
 #endif  // OS_LINUX
   EXPECT_TRUE(base::LaunchProcess(args, options, NULL));
   PCHECK(IGNORE_EINTR(close(fds[1])) == 0);
 
   char buf[512];
@@ skipping 12 matching lines @@
     "0123456789012345678901234567890123456789012345678901234567890123456789"
     "0123456789012345678901234567890123456789012345678901234567890123456789"
     "0123456789012345678901234567890123456789012345678901234567890123456789"
     "0123456789012345678901234567890123456789012345678901234567890123456789";
 
 }  // namespace
 
 TEST_F(ProcessUtilTest, LaunchProcess) {
   base::EnvironmentMap env_changes;
   const int no_clone_flags = 0;
+  const bool no_clear_environ = false;
 
   const char kBaseTest[] = "BASE_TEST";
 
   env_changes[kBaseTest] = "bar";
-  EXPECT_EQ("bar\n", TestLaunchProcess(env_changes, no_clone_flags));
+  EXPECT_EQ("bar\n",
+            TestLaunchProcess(env_changes, no_clear_environ, no_clone_flags));
+  EXPECT_EQ("bar\n", TestLaunchProcess(env_changes, true, no_clone_flags));
   env_changes.clear();
 
   EXPECT_EQ(0, setenv(kBaseTest, "testing", 1 /* override */));
-  EXPECT_EQ("testing\n", TestLaunchProcess(env_changes, no_clone_flags));
+  EXPECT_EQ("testing\n",
+            TestLaunchProcess(env_changes, no_clear_environ, no_clone_flags));
+  EXPECT_EQ("\n", TestLaunchProcess(env_changes, true, no_clone_flags));
 
   env_changes[kBaseTest] = std::string();
-  EXPECT_EQ("\n", TestLaunchProcess(env_changes, no_clone_flags));
+  EXPECT_EQ("\n",
+            TestLaunchProcess(env_changes, no_clear_environ, no_clone_flags));
 
   env_changes[kBaseTest] = "foo";
-  EXPECT_EQ("foo\n", TestLaunchProcess(env_changes, no_clone_flags));
+  EXPECT_EQ("foo\n",
+            TestLaunchProcess(env_changes, no_clear_environ, no_clone_flags));
 
   env_changes.clear();
   EXPECT_EQ(0, setenv(kBaseTest, kLargeString, 1 /* override */));
   EXPECT_EQ(std::string(kLargeString) + "\n",
-            TestLaunchProcess(env_changes, no_clone_flags));
+            TestLaunchProcess(env_changes, no_clear_environ, no_clone_flags));
 
   env_changes[kBaseTest] = "wibble";
-  EXPECT_EQ("wibble\n", TestLaunchProcess(env_changes, no_clone_flags));
+  EXPECT_EQ("wibble\n",
+            TestLaunchProcess(env_changes, no_clear_environ, no_clone_flags));
 
 #if defined(OS_LINUX)
   // Test a non-trival value for clone_flags.
   // Don't test on Valgrind as it has limited support for clone().
   if (!RunningOnValgrind()) {
-    EXPECT_EQ("wibble\n", TestLaunchProcess(env_changes, CLONE_FS | SIGCHLD));
+    EXPECT_EQ(
+        "wibble\n",
+        TestLaunchProcess(env_changes, no_clear_environ, CLONE_FS | SIGCHLD));
   }

[jln (very slow on Chromium), 2014/06/02 21:31:47]

Could you add a test with clear_environ set to true?

I suspect that EXPECT_EQ("\n", TestLaunchProces(., true /* clear_environ */,
..); should work?

[elijahtaylor1, 2014/06/03 20:47:54]

Done.

 #endif
 }
 
 TEST_F(ProcessUtilTest, GetAppOutput) {
   std::string output;
 
 #if defined(OS_ANDROID)
   std::vector<std::string> argv;
   argv.push_back("sh");  // Instead of /bin/sh, force path search to find it.
   argv.push_back("-c");
@@ skipping 193 matching lines @@
   // Check that process was really killed.
   EXPECT_TRUE(IsProcessDead(child_process));
   base::CloseProcessHandle(child_process);
 }
 
 MULTIPROCESS_TEST_MAIN(process_util_test_die_immediately) {
   return 0;
 }
 
 #endif  // defined(OS_POSIX)