From 04f6c30d7cd72e008a0d35e56c4bde81778133fd Mon Sep 17 00:00:00 2001 From: NextTurn <45985406+NextTurn@users.noreply.github.com> Date: Fri, 4 Sep 2020 00:00:00 +0800 Subject: [PATCH] Fix XML queries --- .../Configuration/XmlServiceConfig.cs | 193 +++++------------- src/WinSW.Tests/ServiceConfigTests.cs | 26 +-- 2 files changed, 61 insertions(+), 158 deletions(-) diff --git a/src/WinSW.Core/Configuration/XmlServiceConfig.cs b/src/WinSW.Core/Configuration/XmlServiceConfig.cs index 22c5ee6..054585c 100644 --- a/src/WinSW.Core/Configuration/XmlServiceConfig.cs +++ b/src/WinSW.Core/Configuration/XmlServiceConfig.cs @@ -20,6 +20,7 @@ namespace WinSW { protected readonly XmlDocument dom = new XmlDocument(); + private readonly XmlNode root; private readonly Dictionary environmentVariables; internal static XmlServiceConfig? TestConfig; @@ -63,6 +64,8 @@ namespace WinSW throw new InvalidDataException(e.Message, e); } + this.root = this.dom.SelectSingleNode(Names.Service) ?? throw new InvalidDataException("<" + Names.Service + "> is missing in configuration XML"); + // register the base directory as environment variable so that future expansions can refer to this. Environment.SetEnvironmentVariable("BASE", baseDir); @@ -86,7 +89,7 @@ namespace WinSW #pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. { this.dom = dom; - + this.root = this.dom.SelectSingleNode(Names.Service) ?? throw new InvalidDataException("<" + Names.Service + "> is missing in configuration XML"); this.environmentVariables = this.LoadEnvironmentVariables(); } @@ -120,30 +123,25 @@ namespace WinSW private string SingleElement(string tagName) { - return this.SingleElement(tagName, false)!; + return this.SingleElementOrNull(tagName) ?? throw new InvalidDataException("<" + tagName + "> is missing in configuration XML"); } - private string? SingleElement(string tagName, bool optional) + private string? SingleElementOrNull(string tagName) { - XmlNode? n = this.dom.SelectSingleNode("//" + tagName); - if (n is null && !optional) - { - throw new InvalidDataException("<" + tagName + "> is missing in configuration XML"); - } - + XmlNode? n = this.root.SelectSingleNode(tagName); return n is null ? null : Environment.ExpandEnvironmentVariables(n.InnerText); } - private bool SingleBoolElement(string tagName, bool defaultValue) + private bool SingleBoolElementOrDefault(string tagName, bool defaultValue) { - XmlNode? e = this.dom.SelectSingleNode("//" + tagName); + XmlNode? e = this.root.SelectSingleNode(tagName); return e is null ? defaultValue : bool.Parse(e.InnerText); } private TimeSpan SingleTimeSpanElement(XmlNode parent, string tagName, TimeSpan defaultValue) { - string? value = this.SingleElement(tagName, true); + string? value = this.SingleElementOrNull(tagName); return value is null ? defaultValue : ParseTimeSpan(value); } @@ -167,48 +165,27 @@ namespace WinSW /// public override string Executable => this.SingleElement("executable"); - public override bool HideWindow => this.SingleBoolElement("hidewindow", base.HideWindow); + public override bool HideWindow => this.SingleBoolElementOrDefault("hidewindow", base.HideWindow); /// /// Optionally specify a different Path to an executable to shutdown the service. /// - public override string? StopExecutable => this.SingleElement("stopexecutable", true); + public override string? StopExecutable => this.SingleElementOrNull("stopexecutable"); /// /// The arguments element. /// - public override string Arguments - { - get - { - XmlNode? argumentsNode = this.dom.SelectSingleNode("//arguments"); - return argumentsNode is null ? base.Arguments : Environment.ExpandEnvironmentVariables(argumentsNode.InnerText); - } - } + public override string Arguments => this.SingleElementOrNull("arguments") ?? base.Arguments; /// /// The startarguments element. /// - public override string? StartArguments - { - get - { - XmlNode? startArgumentsNode = this.dom.SelectSingleNode("//startarguments"); - return startArgumentsNode is null ? null : Environment.ExpandEnvironmentVariables(startArgumentsNode.InnerText); - } - } + public override string? StartArguments => this.SingleElementOrNull("startarguments"); /// /// The stoparguments element. /// - public override string? StopArguments - { - get - { - XmlNode? stopArgumentsNode = this.dom.SelectSingleNode("//stoparguments"); - return stopArgumentsNode is null ? null : Environment.ExpandEnvironmentVariables(stopArgumentsNode.InnerText); - } - } + public override string? StopArguments => this.SingleElementOrNull("stoparguments"); public ProcessCommand Prestart => this.GetProcessCommand(Names.Prestart); @@ -222,7 +199,7 @@ namespace WinSW { get { - var wd = this.SingleElement("workingdirectory", true); + string? wd = this.SingleElementOrNull("workingdirectory"); return string.IsNullOrEmpty(wd) ? base.WorkingDirectory : wd!; } } @@ -248,64 +225,12 @@ namespace WinSW } } - public override XmlNode? ExtensionsConfiguration => this.dom.SelectSingleNode("//extensions"); - - /// - /// Combines the contents of all the elements of the given name, - /// or return null if no element exists. Handles whitespace quotation. - /// - private string? AppendTags(string tagName, string? defaultValue = null) - { - XmlNode? argumentNode = this.dom.SelectSingleNode("//" + tagName); - if (argumentNode is null) - { - return defaultValue; - } - - StringBuilder arguments = new StringBuilder(); - - XmlNodeList argumentNodeList = this.dom.SelectNodes("//" + tagName)!; - for (int i = 0; i < argumentNodeList.Count; i++) - { - arguments.Append(' '); - - string token = Environment.ExpandEnvironmentVariables(argumentNodeList[i]!.InnerText); - - if (token.StartsWith("\"") && token.EndsWith("\"")) - { - // for backward compatibility, if the argument is already quoted, leave it as is. - // in earlier versions we didn't handle quotation, so the user might have worked - // around it by themselves - } - else - { - if (token.Contains(" ")) - { - arguments.Append('"').Append(token).Append('"'); - continue; - } - } - - arguments.Append(token); - } - - return arguments.ToString(); - } + public override XmlNode? ExtensionsConfiguration => this.root.SelectSingleNode("extensions"); /// /// LogDirectory is the service wrapper executable directory or the optionally specified logpath element. /// - public override string LogDirectory - { - get - { - XmlNode? loggingNode = this.dom.SelectSingleNode("//logpath"); - - return loggingNode is null - ? base.LogDirectory - : Environment.ExpandEnvironmentVariables(loggingNode.InnerText); - } - } + public override string LogDirectory => this.SingleElementOrNull("logpath") ?? base.LogDirectory; public override string LogMode { @@ -314,7 +239,7 @@ namespace WinSW string? mode = null; // first, backward compatibility with older configuration - XmlElement? e = (XmlElement?)this.dom.SelectSingleNode("//logmode"); + XmlElement? e = (XmlElement?)this.root.SelectSingleNode("logmode"); if (e != null) { mode = e.InnerText; @@ -322,7 +247,7 @@ namespace WinSW else { // this is more modern way, to support nested elements as configuration - e = (XmlElement?)this.dom.SelectSingleNode("//log"); + e = (XmlElement?)this.root.SelectSingleNode("log"); if (e != null) { mode = e.GetAttribute("mode"); @@ -333,48 +258,24 @@ namespace WinSW } } - public string LogName - { - get - { - XmlNode? loggingName = this.dom.SelectSingleNode("//logname"); + public string LogName => this.SingleElementOrNull("logname") ?? this.BaseName; - return loggingName is null ? this.BaseName : Environment.ExpandEnvironmentVariables(loggingName.InnerText); - } - } + public override bool OutFileDisabled => this.SingleBoolElementOrDefault("outfiledisabled", base.OutFileDisabled); - public override bool OutFileDisabled => this.SingleBoolElement("outfiledisabled", base.OutFileDisabled); + public override bool ErrFileDisabled => this.SingleBoolElementOrDefault("errfiledisabled", base.ErrFileDisabled); - public override bool ErrFileDisabled => this.SingleBoolElement("errfiledisabled", base.ErrFileDisabled); + public override string OutFilePattern => this.SingleElementOrNull("outfilepattern") ?? base.OutFilePattern; - public override string OutFilePattern - { - get - { - XmlNode? loggingName = this.dom.SelectSingleNode("//outfilepattern"); - - return loggingName is null ? base.OutFilePattern : Environment.ExpandEnvironmentVariables(loggingName.InnerText); - } - } - - public override string ErrFilePattern - { - get - { - XmlNode? loggingName = this.dom.SelectSingleNode("//errfilepattern"); - - return loggingName is null ? base.ErrFilePattern : Environment.ExpandEnvironmentVariables(loggingName.InnerText); - } - } + public override string ErrFilePattern => this.SingleElementOrNull("errfilepattern") ?? base.ErrFilePattern; public LogHandler LogHandler { get { - XmlElement? e = (XmlElement?)this.dom.SelectSingleNode("//logmode"); + XmlElement? e = (XmlElement?)this.root.SelectSingleNode("logmode"); // this is more modern way, to support nested elements as configuration - e ??= (XmlElement?)this.dom.SelectSingleNode("//log")!; // WARNING: NRE + e ??= (XmlElement?)this.root.SelectSingleNode("log")!; // WARNING: NRE int sizeThreshold; switch (this.LogMode) @@ -462,7 +363,7 @@ namespace WinSW { get { - XmlNodeList? nodeList = this.dom.SelectNodes("//depend"); + XmlNodeList? nodeList = this.root.SelectNodes("depend"); if (nodeList is null) { return base.ServiceDependencies; @@ -480,9 +381,9 @@ namespace WinSW public override string Name => this.SingleElement("id"); - public override string DisplayName => this.SingleElement("name", true) ?? base.DisplayName; + public override string DisplayName => this.SingleElementOrNull("name") ?? base.DisplayName; - public override string Description => this.SingleElement("description", true) ?? base.Description; + public override string Description => this.SingleElementOrNull("description") ?? base.Description; /// /// Start mode of the Service @@ -491,7 +392,7 @@ namespace WinSW { get { - string? p = this.SingleElement("startmode", true); + string? p = this.SingleElementOrNull("startmode"); if (p is null) { return base.StartMode; @@ -519,15 +420,15 @@ namespace WinSW /// True if the service should be installed with the DelayedAutoStart flag. /// This setting will be applyed only during the install command and only when the Automatic start mode is configured. /// - public override bool DelayedAutoStart => this.SingleBoolElement("delayedAutoStart", base.DelayedAutoStart); + public override bool DelayedAutoStart => this.SingleBoolElementOrDefault("delayedAutoStart", base.DelayedAutoStart); - public override bool Preshutdown => this.SingleBoolElement("preshutdown", base.Preshutdown); + public override bool Preshutdown => this.SingleBoolElementOrDefault("preshutdown", base.Preshutdown); public TimeSpan? PreshutdownTimeout { get { - string? value = this.SingleElement("preshutdownTimeout", true); + string? value = this.SingleElementOrNull("preshutdownTimeout"); return value is null ? default : ParseTimeSpan(value); } } @@ -536,12 +437,12 @@ namespace WinSW /// True if the service should beep when finished on shutdown. /// This doesn't work on some OSes. See http://msdn.microsoft.com/en-us/library/ms679277%28VS.85%29.aspx /// - public override bool BeepOnShutdown => this.SingleBoolElement("beeponshutdown", base.DelayedAutoStart); + public override bool BeepOnShutdown => this.SingleBoolElementOrDefault("beeponshutdown", base.DelayedAutoStart); /// /// True if the service can interact with the desktop. /// - public override bool Interactive => this.SingleBoolElement("interactive", base.DelayedAutoStart); + public override bool Interactive => this.SingleBoolElementOrDefault("interactive", base.DelayedAutoStart); /// /// Environment variable overrides @@ -556,7 +457,7 @@ namespace WinSW { get { - XmlNodeList? nodeList = this.dom.SelectNodes("//download"); + XmlNodeList? nodeList = this.root.SelectNodes("download"); if (nodeList is null) { return base.Downloads; @@ -579,7 +480,7 @@ namespace WinSW { get { - XmlNodeList? childNodes = this.dom.SelectNodes("//onfailure"); + XmlNodeList? childNodes = this.root.SelectNodes("onfailure"); if (childNodes is null) { return Array.Empty(); @@ -605,11 +506,11 @@ namespace WinSW } } - public override TimeSpan ResetFailureAfter => this.SingleTimeSpanElement(this.dom, "resetfailure", base.ResetFailureAfter); + public override TimeSpan ResetFailureAfter => this.SingleTimeSpanElement(this.root, "resetfailure", base.ResetFailureAfter); protected string? GetServiceAccountPart(string subNodeName) { - XmlNode? node = this.dom.SelectSingleNode("//serviceaccount"); + XmlNode? node = this.root.SelectSingleNode("serviceaccount"); if (node != null) { @@ -633,7 +534,7 @@ namespace WinSW public bool HasServiceAccount() { - return this.dom.SelectSingleNode("//serviceaccount") != null; + return this.root.SelectSingleNode("serviceaccount") != null; } public override bool AllowServiceAcountLogonRight @@ -655,7 +556,7 @@ namespace WinSW /// /// Time to wait for the service to gracefully shutdown the executable before we forcibly kill it /// - public override TimeSpan StopTimeout => this.SingleTimeSpanElement(this.dom, "stoptimeout", base.StopTimeout); + public override TimeSpan StopTimeout => this.SingleTimeSpanElement(this.root, "stoptimeout", base.StopTimeout); public int StopTimeoutInMs => (int)this.StopTimeout.TotalMilliseconds; @@ -666,7 +567,7 @@ namespace WinSW { get { - string? p = this.SingleElement("priority", true); + string? p = this.SingleElementOrNull("priority"); if (p is null) { return base.Priority; @@ -676,13 +577,13 @@ namespace WinSW } } - public string? SecurityDescriptor => this.SingleElement("securityDescriptor", true); + public string? SecurityDescriptor => this.SingleElementOrNull("securityDescriptor"); - public bool AutoRefresh => this.SingleBoolElement("autoRefresh", true); + public bool AutoRefresh => this.SingleBoolElementOrDefault("autoRefresh", true); private Dictionary LoadEnvironmentVariables() { - XmlNodeList nodeList = this.dom.SelectNodes("//env")!; + XmlNodeList nodeList = this.root.SelectNodes("env")!; Dictionary environment = new Dictionary(nodeList.Count); for (int i = 0; i < nodeList.Count; i++) { @@ -699,7 +600,7 @@ namespace WinSW private ProcessCommand GetProcessCommand(string name) { - XmlNode? node = this.dom.SelectSingleNode(Names.Service)?.SelectSingleNode(name); + XmlNode? node = this.root.SelectSingleNode(name); return node is null ? default : new ProcessCommand { Executable = GetInnerText(Names.Executable), diff --git a/src/WinSW.Tests/ServiceConfigTests.cs b/src/WinSW.Tests/ServiceConfigTests.cs index 57c5e54..94166fa 100644 --- a/src/WinSW.Tests/ServiceConfigTests.cs +++ b/src/WinSW.Tests/ServiceConfigTests.cs @@ -355,24 +355,24 @@ $@" }; var poststart = new ProcessCommand { - Executable = "a1", - Arguments = "a2", - StdoutPath = "a3", - StderrPath = "a4", + Executable = "b1", + Arguments = "b2", + StdoutPath = "b3", + StderrPath = "b4", }; var prestop = new ProcessCommand { - Executable = "a1", - Arguments = "a2", - StdoutPath = "a3", - StderrPath = "a4", + Executable = "c1", + Arguments = "c2", + StdoutPath = "c3", + StderrPath = "c4", }; var poststop = new ProcessCommand { - Executable = "a1", - Arguments = "a2", - StdoutPath = "a3", - StderrPath = "a4", + Executable = "d1", + Arguments = "d2", + StdoutPath = "d3", + StderrPath = "d4", }; string seedXml = @@ -401,6 +401,8 @@ $@" {poststop.StdoutPath} {poststop.StderrPath} + executable + arguments "; XmlServiceConfig config = XmlServiceConfig.FromXml(seedXml);