Merge pull request #202 from oleg-nenashev/bug/JENKINS-42744-envVars

[JENKINS-42744] - Prevent conversion of environment variables to lowercase
pull/197/merge v2.0.3
Oleg Nenashev 2017-04-01 14:47:02 +02:00 committed by GitHub
commit 0da3841804
9 changed files with 317 additions and 65 deletions

View File

@ -133,25 +133,6 @@ namespace winsw
}
}
/// <summary>
/// Starts a thread that protects the execution with a try/catch block.
/// It appears that in .NET, unhandled exception in any thread causes the app to terminate
/// http://msdn.microsoft.com/en-us/library/ms228965.aspx
/// </summary>
private void StartThread(ThreadStart main)
{
new Thread(delegate() {
try
{
main();
}
catch (Exception e)
{
Log.Error("Thread failed unexpectedly",e);
}
}).Start();
}
/// <summary>
/// Handle the creation of the logfiles based on the optional logmode setting.
/// </summary>
@ -419,53 +400,26 @@ namespace winsw
private void StartProcess(Process processToStart, string arguments, String executable)
{
var ps = processToStart.StartInfo;
ps.FileName = executable;
ps.Arguments = arguments;
ps.WorkingDirectory = _descriptor.WorkingDirectory;
ps.CreateNoWindow = false;
ps.UseShellExecute = false;
ps.RedirectStandardInput = true; // this creates a pipe for stdin to the new process, instead of having it inherit our stdin.
ps.RedirectStandardOutput = true;
ps.RedirectStandardError = true;
foreach (string key in _envs.Keys)
{
Environment.SetEnvironmentVariable(key, _envs[key]);
// ps.EnvironmentVariables[key] = envs[key]; // bugged (lower cases all variable names due to StringDictionary being used, see http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=326163)
}
// TODO: Make it generic via extension points. The issue mentioned above should be ideally worked around somehow
ps.EnvironmentVariables[WinSWSystem.ENVVAR_NAME_SERVICE_ID.ToLower()] = _descriptor.Id;
processToStart.Start();
Log.Info("Started " + processToStart.Id);
var priority = _descriptor.Priority;
if (priority != ProcessPriorityClass.Normal)
processToStart.PriorityClass = priority;
// monitor the completion of the process
StartThread(delegate
// Define handler of the completed process
ProcessCompletionCallback processCompletionCallback = delegate(Process proc)
{
string msg = processToStart.Id + " - " + processToStart.StartInfo.FileName + " " + processToStart.StartInfo.Arguments;
processToStart.WaitForExit();
try
{
if (_orderlyShutdown)
{
LogEvent("Child process [" + msg + "] terminated with " + processToStart.ExitCode, EventLogEntryType.Information);
LogEvent("Child process [" + msg + "] terminated with " + proc.ExitCode, EventLogEntryType.Information);
}
else
{
LogEvent("Child process [" + msg + "] finished with " + processToStart.ExitCode, EventLogEntryType.Warning);
LogEvent("Child process [" + msg + "] finished with " + proc.ExitCode, EventLogEntryType.Warning);
// if we finished orderly, report that to SCM.
// by not reporting unclean shutdown, we let Windows SCM to decide if it wants to
// restart the service automatically
if (processToStart.ExitCode == 0)
if (proc.ExitCode == 0)
SignalShutdownComplete();
Environment.Exit(processToStart.ExitCode);
Environment.Exit(proc.ExitCode);
}
}
catch (InvalidOperationException ioe)
@ -475,13 +429,23 @@ namespace winsw
try
{
processToStart.Dispose();
proc.Dispose();
}
catch (InvalidOperationException ioe)
{
LogEvent("Dispose " + ioe.Message);
}
});
};
// Invoke process and exit
ProcessHelper.StartProcessAndCallbackForExit(
processToStart: processToStart,
executable: executable,
arguments: arguments,
envVars: _envs,
workingDirectory: _descriptor.WorkingDirectory,
priority: _descriptor.Priority,
callback: processCompletionCallback);
}
public static int Main(string[] args)

View File

@ -4,6 +4,7 @@ using System.Collections.Generic;
using System.Diagnostics;
using System.Management;
using System.Text;
using System.Threading;
namespace winsw.Util
{
@ -117,6 +118,79 @@ namespace winsw.Util
}
}
// TODO: Also move StartProcess methods once LogEvent()/WriteEvent() mess gets solved
/// <summary>
/// Starts a process and asynchronosly waits for its termination.
/// Once the process exits, the callback will be invoked.
/// </summary>
/// <param name="processToStart">Process object to be used</param>
/// <param name="arguments">Arguments to be passed</param>
/// <param name="executable">Executable, which should be invoked</param>
/// <param name="envVars">Additional environment variables</param>
/// <param name="workingDirectory">Working directory</param>
/// <param name="priority">Priority</param>
/// <param name="callback">Completion callback. If null, the completion won't be monitored</param>
public static void StartProcessAndCallbackForExit(Process processToStart, String executable = null, string arguments = null, Dictionary<string, string> envVars = null,
string workingDirectory = null, ProcessPriorityClass? priority = null, ProcessCompletionCallback callback = null)
{
var ps = processToStart.StartInfo;
ps.FileName = executable ?? ps.FileName;
ps.Arguments = arguments ?? ps.Arguments;
ps.WorkingDirectory = workingDirectory ?? ps.WorkingDirectory;
ps.CreateNoWindow = false;
ps.UseShellExecute = false;
ps.RedirectStandardInput = true; // this creates a pipe for stdin to the new process, instead of having it inherit our stdin.
ps.RedirectStandardOutput = true;
ps.RedirectStandardError = true;
if (envVars != null)
{
foreach (string key in envVars.Keys)
{
Environment.SetEnvironmentVariable(key, envVars[key]);
// DONTDO: ps.EnvironmentVariables[key] = envs[key];
// bugged (lower cases all variable names due to StringDictionary being used, see http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=326163)
}
}
processToStart.Start();
Logger.Info("Started process " + processToStart.Id);
if (priority != null && priority.Value != ProcessPriorityClass.Normal)
{
processToStart.PriorityClass = priority.Value;
}
// monitor the completion of the process
if (callback != null)
{
StartThread(delegate
{
processToStart.WaitForExit();
callback(processToStart);
});
}
}
/// <summary>
/// Starts a thread that protects the execution with a try/catch block.
/// It appears that in .NET, unhandled exception in any thread causes the app to terminate
/// http://msdn.microsoft.com/en-us/library/ms228965.aspx
/// </summary>
public static void StartThread(ThreadStart main)
{
new Thread(delegate()
{
try
{
main();
}
catch (Exception e)
{
Logger.Error("Thread failed unexpectedly", e);
}
}).Start();
}
}
public delegate void ProcessCompletionCallback(Process process);
}

View File

@ -27,6 +27,12 @@ namespace winsw.Plugins.RunawayProcessKiller
/// </summary>
public bool StopParentProcessFirst { get; private set; }
/// <summary>
/// If true, the runaway process will be checked for the WinSW environment variable before termination.
/// This option is not documented AND not supposed to be used by users.
/// </summary>
public bool CheckWinSWEnvironmentVariable { get; private set; }
public override String DisplayName { get { return "Runaway Process Killer"; } }
private String ServiceId { get; set; }
@ -38,9 +44,12 @@ namespace winsw.Plugins.RunawayProcessKiller
// Default initializer
}
public RunawayProcessKillerExtension(String pidfile)
public RunawayProcessKillerExtension(String pidfile, int stopTimeoutMs = 5000, bool stopParentFirst = false, bool checkWinSWEnvironmentVariable = true)
{
this.Pidfile = pidfile;
this.StopTimeout = TimeSpan.FromMilliseconds(5000);
this.StopParentProcessFirst = stopParentFirst;
this.CheckWinSWEnvironmentVariable = checkWinSWEnvironmentVariable;
}
public override void Configure(ServiceDescriptor descriptor, XmlNode node)
@ -51,6 +60,9 @@ namespace winsw.Plugins.RunawayProcessKiller
StopTimeout = TimeSpan.FromMilliseconds(Int32.Parse(XmlHelper.SingleElement(node, "stopTimeout", false)));
StopParentProcessFirst = Boolean.Parse(XmlHelper.SingleElement(node, "stopParentFirst", false));
ServiceId = descriptor.Id;
//TODO: Consider making it documented
var checkWinSWEnvironmentVariable = XmlHelper.SingleElement(node, "checkWinSWEnvironmentVariable", true);
CheckWinSWEnvironmentVariable = checkWinSWEnvironmentVariable != null ? Boolean.Parse(checkWinSWEnvironmentVariable) : true;
}
/// <summary>
@ -89,6 +101,7 @@ namespace winsw.Plugins.RunawayProcessKiller
}
// Now check the process
Logger.DebugFormat("Checking the potentially runaway process with PID={0}", pid);
Process proc;
try
{
@ -105,25 +118,32 @@ namespace winsw.Plugins.RunawayProcessKiller
// TODO: This method is not ideal since it works only for vars explicitly mentioned in the start info
// No Windows 10- compatible solution for EnvVars retrieval, see https://blog.gapotchenko.com/eazfuscator.net/reading-environment-variables
StringDictionary previousProcessEnvVars = proc.StartInfo.EnvironmentVariables;
String expectedEnvVarName = WinSWSystem.ENVVAR_NAME_SERVICE_ID.ToLower();
String expectedEnvVarName = WinSWSystem.ENVVAR_NAME_SERVICE_ID;
if (previousProcessEnvVars.ContainsKey(expectedEnvVarName))
{
// StringDictionary is case-insensitive, hence it will fetch variable definitions in any case
affiliatedServiceId = previousProcessEnvVars[expectedEnvVarName];
}
else
else if (CheckWinSWEnvironmentVariable)
{
Logger.Warn("The process " + pid + " has no " + expectedEnvVarName + " environment variable defined. "
+ "The process has not been started by this service, hence it won't be terminated.");
+ "The process has not been started by WinSW, hence it won't be terminated.");
if (Logger.IsDebugEnabled) {
foreach (string key in previousProcessEnvVars.Keys) {
Logger.Debug("Env var of " + pid + ": " + key + "=" + previousProcessEnvVars[key]);
}
//TODO replace by String.Join() in .NET 4
String[] keys = new String[previousProcessEnvVars.Count];
previousProcessEnvVars.Keys.CopyTo(keys, 0);
Logger.DebugFormat("Env vars of the process with PID={0}: {1}", new Object[] {pid, String.Join(",", keys)});
}
return;
}
else
{
// We just skip this check
affiliatedServiceId = null;
}
// Check the service ID value
if (!ServiceId.Equals(affiliatedServiceId))
if (CheckWinSWEnvironmentVariable && !ServiceId.Equals(affiliatedServiceId))
{
Logger.Warn("The process " + pid + " has been started by Windows service with ID='" + affiliatedServiceId + "'. "
+ "It is another service (current service id is '" + ServiceId + "'), hence the process won't be terminated.");

View File

@ -15,7 +15,7 @@ namespace winswTests.Extensions
/// </summary>
/// <param name="type">Type of the extension</param>
/// <returns>String for Type locator, which includes class and assembly names</returns>
protected static String getExtensionClassNameWithAssembly(Type type)
public static String getExtensionClassNameWithAssembly(Type type)
{
return type.ToString() + ", " + type.Assembly;
}

View File

@ -3,6 +3,12 @@ using NUnit.Framework;
using winsw.Extensions;
using winsw.Plugins.SharedDirectoryMapper;
using winsw.Plugins.RunawayProcessKiller;
using winswTests.Util;
using System.IO;
using System.Diagnostics;
using winsw.Util;
using System;
using System.Collections.Generic;
namespace winswTests.Extensions
{
@ -58,5 +64,63 @@ namespace winswTests.Extensions
manager.FireOnWrapperStarted();
manager.FireBeforeWrapperStopped();
}
[Test]
public void ShouldKillTheSpawnedProcess()
{
var winswId = "myAppWithRunaway";
var extensionId = "runawayProcessKiller";
var tmpDir = FilesystemTestHelper.CreateTmpDirectory();
// Prepare the env var
String varName = WinSWSystem.ENVVAR_NAME_SERVICE_ID;
var env = new Dictionary<string, string>();
env.Add("varName", winswId);
// Spawn the test process
var scriptFile = Path.Combine(tmpDir, "dosleep.bat");
var envFile = Path.Combine(tmpDir, "env.txt");
File.WriteAllText(scriptFile, "set > " + envFile + "\nsleep 100500");
Process proc = new Process();
var ps = proc.StartInfo;
ps.FileName = scriptFile;
ProcessHelper.StartProcessAndCallbackForExit(proc, envVars: env);
try
{
// Generate extension and ensure that the roundtrip is correct
//TODO: checkWinSWEnvironmentVariable should be true, but it does not work due to proc.StartInfo.EnvironmentVariables
var pidfile = Path.Combine(tmpDir, "process.pid");
var sd = ConfigXmlBuilder.create(id: winswId)
.WithRunawayProcessKiller(new RunawayProcessKillerExtension(pidfile, checkWinSWEnvironmentVariable: false), extensionId)
.ToServiceDescriptor();
WinSWExtensionManager manager = new WinSWExtensionManager(sd);
manager.LoadExtensions();
var extension = manager.Extensions[extensionId] as RunawayProcessKillerExtension;
Assert.IsNotNull(extension, "RunawayProcessKillerExtension should be loaded");
Assert.AreEqual(pidfile, extension.Pidfile, "PidFile should have been retained during the config roundtrip");
// Inject PID
File.WriteAllText(pidfile, proc.Id.ToString());
// Try to terminate
Assert.That(!proc.HasExited, "Process " + proc + " has exited before the RunawayProcessKiller extension invocation");
extension.OnWrapperStarted();
Assert.That(proc.HasExited, "Process " + proc + " should have been terminated by RunawayProcessKiller");
}
finally
{
if (!proc.HasExited)
{
Console.Error.WriteLine("Test: Killing runaway process with ID=" + proc.Id);
ProcessHelper.StopProcessAndChildren(proc.Id, TimeSpan.FromMilliseconds(100), false);
if (!proc.HasExited)
{
// The test is failed here anyway, but we add additional diagnostics info
Console.Error.WriteLine("Test: ProcessHelper failed to properly terminate process with ID=" + proc.Id);
}
}
}
}
}
}

View File

@ -2,6 +2,8 @@
using System.Collections.Generic;
using System.Text;
using winsw;
using winsw.Plugins.RunawayProcessKiller;
using winswTests.Extensions;
namespace winswTests.Util
{
@ -16,6 +18,7 @@ namespace winswTests.Util
public string Executable { get; set; }
public bool PrintXMLVersion { get; set; }
public string XMLComment { get; set; }
public List<string> ExtensionXmls { get; private set; }
private List<String> configEntries;
@ -23,6 +26,7 @@ namespace winswTests.Util
private ConfigXmlBuilder()
{
configEntries = new List<string>();
ExtensionXmls = new List<string>();
}
public static ConfigXmlBuilder create(string id = null, string name = null,
@ -63,6 +67,18 @@ namespace winswTests.Util
// We do not care much about pretty formatting here
str.AppendFormat(" {0}\n", entry);
}
// Extensions
if (ExtensionXmls.Count > 0)
{
str.Append(" <extensions>\n");
foreach (string xml in ExtensionXmls)
{
str.Append(xml);
}
str.Append(" </extensions>\n");
}
str.Append("</service>\n");
string res = str.ToString();
if (dumpConfig)
@ -88,5 +104,20 @@ namespace winswTests.Util
{
return WithRawEntry(String.Format("<{0}>{1}</{0}>", tagName, value));
}
public ConfigXmlBuilder WithRunawayProcessKiller(RunawayProcessKillerExtension ext, string extensionId = "killRunawayProcess", bool enabled = true)
{
var fullyQualifiedExtensionName = ExtensionTestBase.getExtensionClassNameWithAssembly(typeof(RunawayProcessKillerExtension));
StringBuilder str = new StringBuilder();
str.AppendFormat(" <extension enabled=\"{0}\" className=\"{1}\" id=\"{2}\">\n", new Object[] { enabled, fullyQualifiedExtensionName, extensionId});
str.AppendFormat(" <pidfile>{0}</pidfile>\n", ext.Pidfile);
str.AppendFormat(" <stopTimeout>{0}</stopTimeout>\n", ext.StopTimeout.TotalMilliseconds);
str.AppendFormat(" <stopParentFirst>{0}</stopParentFirst>\n", ext.StopParentProcessFirst);
str.AppendFormat(" <checkWinSWEnvironmentVariable>{0}</checkWinSWEnvironmentVariable>\n", ext.CheckWinSWEnvironmentVariable);
str.Append( " </extension>\n");
ExtensionXmls.Add(str.ToString());
return this;
}
}
}

View File

@ -0,0 +1,46 @@
using NUnit.Framework;
using System;
using System.Collections.Generic;
using System.IO;
using System.Text;
namespace winswTests.Util
{
class FilesystemTestHelper
{
/// <summary>
/// Creates a temporary directory for testing.
/// </summary>
/// <returns>tmp Dir</returns>
public static string CreateTmpDirectory(String testName = null)
{
string tempDirectory = Path.Combine(Path.GetTempPath(), "winswTests_" + (testName ?? "") + Path.GetRandomFileName());
Directory.CreateDirectory(tempDirectory);
Console.Out.WriteLine("Created the temporary directory: {0}", tempDirectory);
return tempDirectory;
}
/// <summary>
/// Parses output of the "set" command from the file
/// </summary>
/// <param name="filePath">File path</param>
/// <returns>Dictionary of the strings.</returns>
public static Dictionary<string, string> parseSetOutput(string filePath)
{
Dictionary<string, string> res = new Dictionary<string, string>();
var lines = File.ReadAllLines(filePath);
foreach(var line in lines) {
var parsed = line.Split("=".ToCharArray(), 2);
if (parsed.Length == 2)
{
res.Add(parsed[0], parsed[1]);
}
else
{
Assert.Fail("Wrong line in the parsed Set output file: " + line);
}
}
return res;
}
}
}

View File

@ -0,0 +1,51 @@
using System;
using System.Diagnostics;
using NUnit.Framework;
using winsw;
using System.IO;
using winsw.Util;
namespace winswTests.Util
{
[TestFixture]
class ProcessHelperTest
{
/// <summary>
/// Also reported as <a href="https://issues.jenkins-ci.org/browse/JENKINS-42744">JENKINS-42744</a>
/// </summary>
[Test]
public void ShouldPropagateVariablesInUppercase()
{
var tmpDir = FilesystemTestHelper.CreateTmpDirectory();
String envFile = Path.Combine(tmpDir, "env.properties");
String scriptFile = Path.Combine(tmpDir, "printenv.bat");
File.WriteAllText(scriptFile, "set > " + envFile);
Process proc = new Process();
var ps = proc.StartInfo;
ps.FileName = scriptFile;
ProcessHelper.StartProcessAndCallbackForExit(proc);
var exited = proc.WaitForExit(5000);
if (!exited)
{
Assert.Fail("Process " + proc + " didn't exit after 5 seconds");
}
// Check several veriables, which are expected to be in Uppercase
var envVars = FilesystemTestHelper.parseSetOutput(envFile);
String[] keys = new String[envVars.Count];
envVars.Keys.CopyTo(keys, 0);
String availableVars = "[" + String.Join(",", keys) + "]";
Assert.That(envVars.ContainsKey("PROCESSOR_ARCHITECTURE"), "No PROCESSOR_ARCHITECTURE in the injected vars: " + availableVars);
Assert.That(envVars.ContainsKey("COMPUTERNAME"), "No COMPUTERNAME in the injected vars: " + availableVars);
Assert.That(envVars.ContainsKey("PATHEXT"), "No PATHEXT in the injected vars: " + availableVars);
// And just ensure that the parsing logic is case-sensitive
Assert.That(!envVars.ContainsKey("computername"), "Test error: the environment parsing logic is case-insensitive");
}
}
}

View File

@ -60,6 +60,8 @@
<Compile Include="Extensions\RunawayProcessKillerTest.cs" />
<Compile Include="Extensions\SharedDirectoryMapperTest.cs" />
<Compile Include="MainTest.cs" />
<Compile Include="Util\FilesystemTestHelper.cs" />
<Compile Include="Util\ProcessHelperTest.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="ServiceDescriptorTests.cs" />
<Compile Include="Util\CLITestHelper.cs" />