Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ban XmlReader overloads that take string #9076

Merged
merged 7 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/BannedSymbols.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
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
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
7 changes: 4 additions & 3 deletions src/Tasks/GenerateResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1745,9 +1745,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())
{
Expand Down
48 changes: 25 additions & 23 deletions src/Tasks/ManifestUtil/XmlUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/Tasks/ManifestUtil/mansign2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 17 additions & 9 deletions src/Tasks/XamlTaskFactory/RelationsParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -174,17 +177,19 @@ internal class RelationsParser
#endregion

/// <summary>
/// The method that loads in an XML file
/// The method that loads in an XML file.
/// </summary>
/// <param name="fileName">the xml file containing switches and properties</param>
private XmlDocument LoadFile(string fileName)
/// <param name="filePath">the xml file containing switches and properties.</param>
private XmlDocument LoadFile(string filePath)
{
try
{
var xmlDocument = new XmlDocument();
XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore };
XmlReader reader = XmlReader.Create(fileName, settings);
XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore, CloseInput = true };
FileStream fs = File.OpenRead(filePath);
using XmlReader reader = XmlReader.Create(fs, settings);
xmlDocument.Load(reader);

YuliiaKovalova marked this conversation as resolved.
Show resolved Hide resolved
return xmlDocument;
}
catch (FileNotFoundException e)
Expand All @@ -209,9 +214,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);
return xmlDocument;
using (XmlReader reader = XmlReader.Create(new StringReader(xml), settings))
{
xmlDocument.Load(reader);

return xmlDocument;
}
}
catch (XmlException e)
{
Expand All @@ -221,7 +229,7 @@ internal XmlDocument LoadXml(string xml)
}

/// <summary>
/// Parses the xml file
/// Parses the xml file.
/// </summary>
public bool ParseXmlDocument(string fileName)
{
Expand Down
8 changes: 5 additions & 3 deletions src/Tasks/XslTransformation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public override bool Execute()
/// Takes the raw XML and loads XsltArgumentList
/// </summary>
/// <param name="xsltParametersXml">The raw XML that holds each parameter as <Parameter Name="" Value="" Namespace="" /> </param>
/// <returns>XsltArgumentList</returns>
/// <returns>XsltArgumentList.</returns>
private static XsltArgumentList ProcessXsltArguments(string xsltParametersXml)
{
XsltArgumentList arguments = new XsltArgumentList();
Expand All @@ -214,8 +214,10 @@ private static XsltArgumentList ProcessXsltArguments(string xsltParametersXml)
try
{
XmlReaderSettings settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore };
XmlReader reader = XmlReader.Create(new StringReader("<XsltParameters>" + xsltParametersXml + "</XsltParameters>"), settings);
doc.Load(reader);
using (XmlReader reader = XmlReader.Create(new StringReader("<XsltParameters>" + xsltParametersXml + "</XsltParameters>"), settings))
{
doc.Load(reader);
}
}
catch (XmlException xe)
{
Expand Down