From 8ecb49c293a30f32d7d2d88f3de201a351bd6d47 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Wed, 26 Jul 2023 11:16:49 -0500 Subject: [PATCH 1/6] Ban XmlReader overloads that take string These overloads create a URI from the string and can cause problems with GB18030 certification, because that URI gets normalized in a way that doesn't work with all characters. We should instead pass a stream created from the file, as in #8931 and #9028. Formalize that rule for the whole repo. --- src/BannedSymbols.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/BannedSymbols.txt b/src/BannedSymbols.txt index f11ba0906af..16d9b61a3bc 100644 --- a/src/BannedSymbols.txt +++ b/src/BannedSymbols.txt @@ -1,2 +1,5 @@ M:System.Globalization.CompareInfo.IndexOf(System.String,System.Char);CompareInfo.IndexOf can unexpectedly allocate strings--use string.IndexOf P:Microsoft.Build.Construction.ProjectElementContainer.Children;Use ChildrenEnumerable instead to avoid boxing +M:System.Xml.XmlReader.Create(System.String);Do not pass paths to XmlReader.Create--use the Stream overload +M:System.Xml.XmlReader.Create(System.String,System.Xml.XmlReaderSettings);Do not pass paths to XmlReader.Create--use the Stream overload +M:System.Xml.XmlReader.Create(System.String,System.Xml.XmlReaderSettings,System.Xml.XmlParserContext);Do not pass paths to XmlReader.Create--use the Stream overload From bf6fe8704be0cbd5206baf5eca98ff1cb9be0e87 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Thu, 27 Jul 2023 08:08:28 +0200 Subject: [PATCH 2/6] Add XPathDocument string ctors to banned symbols --- src/BannedSymbols.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/BannedSymbols.txt b/src/BannedSymbols.txt index 16d9b61a3bc..c369fe9d42b 100644 --- a/src/BannedSymbols.txt +++ b/src/BannedSymbols.txt @@ -3,3 +3,5 @@ P:Microsoft.Build.Construction.ProjectElementContainer.Children;Use ChildrenEnum M:System.Xml.XmlReader.Create(System.String);Do not pass paths to XmlReader.Create--use the Stream overload M:System.Xml.XmlReader.Create(System.String,System.Xml.XmlReaderSettings);Do not pass paths to XmlReader.Create--use the Stream overload M:System.Xml.XmlReader.Create(System.String,System.Xml.XmlReaderSettings,System.Xml.XmlParserContext);Do not pass paths to XmlReader.Create--use the Stream overload +M:System.Xml.XPath.XPathDocument.#ctor(System.String);Do not pass string paths to XPathDocument ctor--use the Stream overload +M:System.Xml.XPath.XPathDocument.#ctor(System.String,System.Xml.XmlSpace);Do not pass string paths to XPathDocument ctor--use the Stream overload From 85913002e2a92cdb7f0775a4fa6eda67a727e456 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Thu, 27 Jul 2023 09:37:40 +0200 Subject: [PATCH 3/6] fix 2 more cases for XmlReader.Create call --- src/Tasks/GenerateResource.cs | 8 ++++--- src/Tasks/XamlTaskFactory/RelationsParser.cs | 22 +++++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 535e0a3dd2d..33b940e297c 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -42,6 +42,7 @@ using Microsoft.Build.Utilities; #if FEATURE_RESXREADER_LIVEDESERIALIZATION using Microsoft.Win32; +using System.Windows.Forms; #endif #nullable disable @@ -1745,9 +1746,10 @@ private bool NeedSeparateAppDomain() try { - XmlReaderSettings readerSettings = new XmlReaderSettings(); - readerSettings.DtdProcessing = DtdProcessing.Ignore; - reader = XmlReader.Create(source.ItemSpec, readerSettings); + XmlReaderSettings readerSettings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore, CloseInput = true }; + + FileStream fs = File.OpenRead(source.ItemSpec); + reader = XmlReader.Create(fs, readerSettings); while (reader.Read()) { diff --git a/src/Tasks/XamlTaskFactory/RelationsParser.cs b/src/Tasks/XamlTaskFactory/RelationsParser.cs index f9734a9720f..2220942ee18 100644 --- a/src/Tasks/XamlTaskFactory/RelationsParser.cs +++ b/src/Tasks/XamlTaskFactory/RelationsParser.cs @@ -6,6 +6,9 @@ using System.IO; using System.Xml; using Microsoft.Build.Shared; +using Microsoft.Build.Tasks.Deployment.ManifestUtilities; +using Microsoft.IO; +using File = System.IO.File; #nullable disable @@ -174,17 +177,21 @@ internal class RelationsParser #endregion /// - /// The method that loads in an XML file + /// The method that loads in an XML file. /// - /// the xml file containing switches and properties - private XmlDocument LoadFile(string fileName) + /// the xml file containing switches and properties. + private XmlDocument LoadFile(string filePath) { try { var xmlDocument = new XmlDocument(); - XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore }; - XmlReader reader = XmlReader.Create(fileName, settings); - xmlDocument.Load(reader); + XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore, CloseInput = true }; + FileStream fs = File.OpenRead(filePath); + using (XmlReader reader = XmlReader.Create(fs, settings)) + { + xmlDocument.Load(reader); + } + return xmlDocument; } catch (FileNotFoundException e) @@ -211,6 +218,7 @@ internal XmlDocument LoadXml(string xml) XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore }; XmlReader reader = XmlReader.Create(new StringReader(xml), settings); xmlDocument.Load(reader); + return xmlDocument; } catch (XmlException e) @@ -221,7 +229,7 @@ internal XmlDocument LoadXml(string xml) } /// - /// Parses the xml file + /// Parses the xml file. /// public bool ParseXmlDocument(string fileName) { From 4ddb7521d7aec8a580bd94403f919a6a41297c0e Mon Sep 17 00:00:00 2001 From: YuliiaKovalova <95473390+YuliiaKovalova@users.noreply.github.com> Date: Thu, 27 Jul 2023 11:11:14 +0200 Subject: [PATCH 4/6] Update src/Tasks/XamlTaskFactory/RelationsParser.cs Co-authored-by: Jan Krivanek --- src/Tasks/XamlTaskFactory/RelationsParser.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Tasks/XamlTaskFactory/RelationsParser.cs b/src/Tasks/XamlTaskFactory/RelationsParser.cs index 2220942ee18..9f4ee313205 100644 --- a/src/Tasks/XamlTaskFactory/RelationsParser.cs +++ b/src/Tasks/XamlTaskFactory/RelationsParser.cs @@ -187,10 +187,7 @@ private XmlDocument LoadFile(string filePath) var xmlDocument = new XmlDocument(); XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore, CloseInput = true }; FileStream fs = File.OpenRead(filePath); - using (XmlReader reader = XmlReader.Create(fs, settings)) - { - xmlDocument.Load(reader); - } + using XmlReader reader = XmlReader.Create(fs, settings); return xmlDocument; } From d03d3c27e29fddefb66a7e8c331034f69fd6afcf Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Thu, 27 Jul 2023 11:19:06 +0200 Subject: [PATCH 5/6] fix review comments --- src/Tasks/GenerateResource.cs | 1 - src/Tasks/ManifestUtil/XmlUtil.cs | 48 ++++++++++---------- src/Tasks/ManifestUtil/mansign2.cs | 6 ++- src/Tasks/XamlTaskFactory/RelationsParser.cs | 8 ++-- src/Tasks/XslTransformation.cs | 8 ++-- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index 33b940e297c..3c198c1d512 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -42,7 +42,6 @@ using Microsoft.Build.Utilities; #if FEATURE_RESXREADER_LIVEDESERIALIZATION using Microsoft.Win32; -using System.Windows.Forms; #endif #nullable disable diff --git a/src/Tasks/ManifestUtil/XmlUtil.cs b/src/Tasks/ManifestUtil/XmlUtil.cs index e2360d23b24..ca35d8090a0 100644 --- a/src/Tasks/ManifestUtil/XmlUtil.cs +++ b/src/Tasks/ManifestUtil/XmlUtil.cs @@ -97,36 +97,38 @@ public static Stream XslTransform(string resource, Stream input, params Dictiona Util.CopyStream(input, clonedInput); int t4 = Environment.TickCount; - XmlReader xml = XmlReader.Create(clonedInput); - Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "new XmlReader(2) t={0}", Environment.TickCount - t4)); - - XsltArgumentList args = null; - if (entries.Length > 0) + using (XmlReader reader = XmlReader.Create(clonedInput)) { - args = new XsltArgumentList(); - foreach (DictionaryEntry entry in entries) + Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "new XmlReader(2) t={0}", Environment.TickCount - t4)); + + XsltArgumentList args = null; + if (entries.Length > 0) { - string key = entry.Key.ToString(); - object val = entry.Value.ToString(); - args.AddParam(key, "", val); - Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "arg: key='{0}' value='{1}'", key, val.ToString())); + args = new XsltArgumentList(); + foreach (DictionaryEntry entry in entries) + { + string key = entry.Key.ToString(); + object val = entry.Value.ToString(); + args.AddParam(key, "", val); + Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "arg: key='{0}' value='{1}'", key, val.ToString())); + } } - } - var m = new MemoryStream(); - var w = new XmlTextWriter(m, Encoding.UTF8); - w.WriteStartDocument(); + var m = new MemoryStream(); + var w = new XmlTextWriter(m, Encoding.UTF8); + w.WriteStartDocument(); - int t5 = Environment.TickCount; - xslc.Transform(xml, args, w, s_resolver); - Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "XslCompiledTransform.Transform t={0}", Environment.TickCount - t4)); + int t5 = Environment.TickCount; + xslc.Transform(reader, args, w, s_resolver); + Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "XslCompiledTransform.Transform t={0}", Environment.TickCount - t4)); - w.WriteEndDocument(); - w.Flush(); - m.Position = 0; + w.WriteEndDocument(); + w.Flush(); + m.Position = 0; - Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "XslCompiledTransform(\"{0}\") t={1}", resource, Environment.TickCount - t1)); - return m; + Util.WriteLog(String.Format(CultureInfo.CurrentCulture, "XslCompiledTransform(\"{0}\") t={1}", resource, Environment.TickCount - t1)); + return m; + } } private class ResourceResolver : XmlUrlResolver diff --git a/src/Tasks/ManifestUtil/mansign2.cs b/src/Tasks/ManifestUtil/mansign2.cs index ef14371aa36..3b19aee2b1e 100644 --- a/src/Tasks/ManifestUtil/mansign2.cs +++ b/src/Tasks/ManifestUtil/mansign2.cs @@ -612,8 +612,10 @@ private static byte[] ComputeHashFromManifest(XmlDocument manifestDom, bool oldF { XmlReaderSettings settings = new XmlReaderSettings(); settings.DtdProcessing = DtdProcessing.Parse; - XmlReader reader = XmlReader.Create(stringReader, settings, manifestDom.BaseURI); - normalizedDom.Load(reader); + using (XmlReader reader = XmlReader.Create(stringReader, settings, manifestDom.BaseURI)) + { + normalizedDom.Load(reader); + } } XmlDsigExcC14NTransform exc = new XmlDsigExcC14NTransform(); diff --git a/src/Tasks/XamlTaskFactory/RelationsParser.cs b/src/Tasks/XamlTaskFactory/RelationsParser.cs index 2220942ee18..3a2b2cc1a14 100644 --- a/src/Tasks/XamlTaskFactory/RelationsParser.cs +++ b/src/Tasks/XamlTaskFactory/RelationsParser.cs @@ -216,10 +216,12 @@ internal XmlDocument LoadXml(string xml) { var xmlDocument = new XmlDocument(); XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore }; - XmlReader reader = XmlReader.Create(new StringReader(xml), settings); - xmlDocument.Load(reader); + using (XmlReader reader = XmlReader.Create(new StringReader(xml), settings)) + { + xmlDocument.Load(reader); - return xmlDocument; + return xmlDocument; + } } catch (XmlException e) { diff --git a/src/Tasks/XslTransformation.cs b/src/Tasks/XslTransformation.cs index c2829bd8851..f5e09078922 100644 --- a/src/Tasks/XslTransformation.cs +++ b/src/Tasks/XslTransformation.cs @@ -201,7 +201,7 @@ public override bool Execute() /// Takes the raw XML and loads XsltArgumentList /// /// The raw XML that holds each parameter as - /// XsltArgumentList + /// XsltArgumentList. private static XsltArgumentList ProcessXsltArguments(string xsltParametersXml) { XsltArgumentList arguments = new XsltArgumentList(); @@ -214,8 +214,10 @@ private static XsltArgumentList ProcessXsltArguments(string xsltParametersXml) try { XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore }; - XmlReader reader = XmlReader.Create(new StringReader("" + xsltParametersXml + ""), settings); - doc.Load(reader); + using (XmlReader reader = XmlReader.Create(new StringReader("" + xsltParametersXml + ""), settings)) + { + doc.Load(reader); + } } catch (XmlException xe) { From dffb524810fc142877f176a94c48fbf9772d41c3 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova <95473390+YuliiaKovalova@users.noreply.github.com> Date: Tue, 1 Aug 2023 16:00:14 +0200 Subject: [PATCH 6/6] Update src/Tasks/XamlTaskFactory/RelationsParser.cs Co-authored-by: Rainer Sigwald --- src/Tasks/XamlTaskFactory/RelationsParser.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tasks/XamlTaskFactory/RelationsParser.cs b/src/Tasks/XamlTaskFactory/RelationsParser.cs index 70f02cd0cd9..8ef3dee1bf3 100644 --- a/src/Tasks/XamlTaskFactory/RelationsParser.cs +++ b/src/Tasks/XamlTaskFactory/RelationsParser.cs @@ -188,6 +188,7 @@ private XmlDocument LoadFile(string filePath) XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore, CloseInput = true }; FileStream fs = File.OpenRead(filePath); using XmlReader reader = XmlReader.Create(fs, settings); + xmlDocument.Load(reader); return xmlDocument; }