Skip to content

Commit

Permalink
Nullable annotations for System.Security.Cryptography.Xml (#67198)
Browse files Browse the repository at this point in the history
* First pass

* More annotations

* Rever mistaken change to test

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* More annotations

* Remove red flag comment

* PR feedback

* Remove 'red flag' comments

* Revert unintended changes to tests

* Revert unintended changes

* PR feedback: remove redundant !'s from things that can never return null

* Added  `[MemberNotNullWhen(true, nameof(_cachedXml))]` on implementation.

* Updated ref t ypes (with `dotnet msbuild /t:GenerateReferenceAssemblySource`)

* Updated ref assembly

* PR feedback: add `MemberNotNull` attribute on `Initialize`)

* Updated ref again

* Put `MemberNotNull` attribute on the getter rather than the property in the reference file.

* PR feedback: remove MemberNotNullWhenAttribute from ref

* PR feedback: non nullable param to match method in base class

* Fix post-merge build errors

* PR feedback

* PR feedback from @bartonjs - add [DisallowNull] and cascade

* PR feedback

* PR feedback

* PR feedback

* Fix build again, by removing the `protected internal` property `CacheValid`

* PR feedback

* Added back `CacheValid` based on PR comment

* Fix issues from recent rebase

* Add `DisallowNull` attribute to ref file and remove it from `DefaultGenApiDocIds.txt`

* PR feedback

* Fix issues from recent merge

* Remove some more extraneous damnit operators

* Fix NRT analysis errors after prior rebase

* PR feedback

* PR feedback

* PR feedback

* PR feedback

* PR feedback

* CipherReference and CipherValue made to disallow null

* Make Uri non-nullable on EnctrypedReference

* Add AllowNull attribute on EncryptedType::KeyInfo

* AllowNull on EncryptedXml::Recipient

* Made baseUri nullable

* DisallowNull on PropertlyElement

* AllowNull on SignedInfo::CanonicalizationMethod

* AllowNull on SignedXml::EncryptedXml

* Fix build after recent rebase

* Remove redundant damnit operator now that #71860 has been merged

* PR feedback

* Fix build due to change of the CanonicalXmlElement constructor

* PR feedback

* Remove redundant damnit operators

* Fix broken tests

* Fix issues after rebase

* PR feedback

* PR feedback
  • Loading branch information
SteveDunn committed Sep 27, 2022
1 parent bc27a6a commit 0075639
Show file tree
Hide file tree
Showing 63 changed files with 845 additions and 795 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
context.Diagnostics);
if (!bool.Parse(diagnostic.Properties[ConvertToLibraryImportAnalyzer.ExactSpelling]))
{
CharSet charSet = (CharSet)Enum.Parse(typeof(CharSet), diagnostic.Properties[ConvertToLibraryImportAnalyzer.CharSet]);
CharSet charSet = (CharSet)Enum.Parse(typeof(CharSet), diagnostic.Properties[ConvertToLibraryImportAnalyzer.CharSet]!);
// CharSet.Auto traditionally maps to either an A or W suffix
// depending on the default CharSet of the platform.
// We will offer both suffix options when CharSet.Auto is provided
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<NoWarn>$(NoWarn);CA1850</NoWarn> <!-- CA1850 suppressed due to multitargeting -->
<IsPackable>true</IsPackable>
<PackageDescription>Provides classes to support the creation and validation of XML digital signatures. The classes in this namespace implement the World Wide Web Consortium Recommendation, "XML-Signature Syntax and Processing", described at http://www.w3.org/TR/xmldsig-core/.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ internal abstract class AncestralNamespaceContextManager

internal NamespaceFrame GetScopeAt(int i)
{
return (NamespaceFrame)_ancestorStack[i];
return (NamespaceFrame)_ancestorStack[i]!;
}

internal NamespaceFrame GetCurrentScope()
{
return GetScopeAt(_ancestorStack.Count - 1);
}

protected XmlAttribute GetNearestRenderedNamespaceWithMatchingPrefix(string nsPrefix, out int depth)
protected XmlAttribute? GetNearestRenderedNamespaceWithMatchingPrefix(string nsPrefix, out int depth)
{
XmlAttribute attr;
XmlAttribute? attr;
depth = -1;
for (int i = _ancestorStack.Count - 1; i >= 0; i--)
{
Expand All @@ -35,9 +35,9 @@ protected XmlAttribute GetNearestRenderedNamespaceWithMatchingPrefix(string nsPr
return null;
}

protected XmlAttribute GetNearestUnrenderedNamespaceWithMatchingPrefix(string nsPrefix, out int depth)
protected XmlAttribute? GetNearestUnrenderedNamespaceWithMatchingPrefix(string nsPrefix, out int depth)
{
XmlAttribute attr;
XmlAttribute? attr;
depth = -1;
for (int i = _ancestorStack.Count - 1; i >= 0; i--)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ internal sealed class AttributeSortOrder : IComparer
{
internal AttributeSortOrder() { }

public int Compare(object a, object b)
public int Compare(object? a, object? b)
{
XmlNode nodeA = a as XmlNode;
XmlNode nodeB = b as XmlNode;
XmlNode? nodeA = a as XmlNode;
XmlNode? nodeB = b as XmlNode;
if ((nodeA == null) || (nodeB == null))
throw new ArgumentException();
int namespaceCompare = string.CompareOrdinal(nodeA.NamespaceURI, nodeB.NamespaceURI);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ private void GetNamespaceToRender(string nsPrefix, SortedList attrListToRender,
}

int rDepth;
XmlAttribute local = (XmlAttribute)nsLocallyDeclared[nsPrefix];
XmlAttribute rAncestral = GetNearestRenderedNamespaceWithMatchingPrefix(nsPrefix, out rDepth);
XmlAttribute? local = (XmlAttribute?)nsLocallyDeclared[nsPrefix];
XmlAttribute? rAncestral = GetNearestRenderedNamespaceWithMatchingPrefix(nsPrefix, out rDepth);
if (local != null)
{
if (Utils.IsNonRedundantNamespaceDecl(local, rAncestral))
Expand All @@ -42,7 +42,7 @@ private void GetNamespaceToRender(string nsPrefix, SortedList attrListToRender,
else
{
int uDepth;
XmlAttribute uAncestral = GetNearestUnrenderedNamespaceWithMatchingPrefix(nsPrefix, out uDepth);
XmlAttribute? uAncestral = GetNearestUnrenderedNamespaceWithMatchingPrefix(nsPrefix, out uDepth);
if (uAncestral != null && uDepth > rDepth && Utils.IsNonRedundantNamespaceDecl(uAncestral, rAncestral))
{
if (Utils.IsXmlNamespaceNode(uAncestral))
Expand All @@ -55,14 +55,14 @@ private void GetNamespaceToRender(string nsPrefix, SortedList attrListToRender,

internal override void GetNamespacesToRender(XmlElement element, SortedList attrListToRender, SortedList nsListToRender, Hashtable nsLocallyDeclared)
{
XmlAttribute attrib;
XmlAttribute? attrib;
object[] attrs = new object[nsLocallyDeclared.Count];
nsLocallyDeclared.Values.CopyTo(attrs, 0);
foreach (object a in attrs)
{
attrib = (XmlAttribute)a;
int rDepth;
XmlAttribute rAncestral = GetNearestRenderedNamespaceWithMatchingPrefix(Utils.GetNamespacePrefix(attrib), out rDepth);
XmlAttribute? rAncestral = GetNearestRenderedNamespaceWithMatchingPrefix(Utils.GetNamespacePrefix(attrib), out rDepth);
if (Utils.IsNonRedundantNamespaceDecl(attrib, rAncestral))
{
nsLocallyDeclared.Remove(Utils.GetNamespacePrefix(attrib));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal sealed class CanonicalXml
// private static string defaultXPathWithComments = "(//. | //@* | //namespace::*)";
// private static string defaultXPathWithComments = "(//. | //@* | //namespace::*)";

internal CanonicalXml(Stream inputStream, bool includeComments, XmlResolver resolver, string strBaseUri)
internal CanonicalXml(Stream inputStream, bool includeComments, XmlResolver? resolver, string strBaseUri)
{
if (inputStream is null)
{
Expand All @@ -30,8 +30,8 @@ internal CanonicalXml(Stream inputStream, bool includeComments, XmlResolver reso
_ancMgr = new C14NAncestralNamespaceContextManager();
}

internal CanonicalXml(XmlDocument document, XmlResolver resolver) : this(document, resolver, false) { }
internal CanonicalXml(XmlDocument document, XmlResolver resolver, bool includeComments)
internal CanonicalXml(XmlDocument document, XmlResolver? resolver) : this(document, resolver, false) { }
internal CanonicalXml(XmlDocument document, XmlResolver? resolver, bool includeComments)
{
if (document is null)
{
Expand All @@ -44,14 +44,14 @@ internal CanonicalXml(XmlDocument document, XmlResolver resolver, bool includeCo
_ancMgr = new C14NAncestralNamespaceContextManager();
}

internal CanonicalXml(XmlNodeList nodeList, XmlResolver resolver, bool includeComments)
internal CanonicalXml(XmlNodeList nodeList, XmlResolver? resolver, bool includeComments)
{
if (nodeList is null)
{
throw new ArgumentNullException(nameof(nodeList));
}

XmlDocument doc = Utils.GetOwnerDocument(nodeList);
XmlDocument? doc = Utils.GetOwnerDocument(nodeList);
if (doc == null)
throw new ArgumentException(nameof(nodeList));

Expand Down Expand Up @@ -79,8 +79,8 @@ private static void MarkInclusionStateForNodes(XmlNodeList nodeList, XmlDocument

do
{
XmlNode currentNode = (XmlNode)elementList[index];
XmlNode currentNodeCanonical = (XmlNode)elementListCanonical[index];
XmlNode currentNode = (XmlNode)elementList[index]!;
XmlNode currentNodeCanonical = (XmlNode)elementListCanonical[index]!;
XmlNodeList childNodes = currentNode.ChildNodes;
XmlNodeList childNodesCanonical = currentNodeCanonical.ChildNodes;
for (int i = 0; i < childNodes.Count; i++)
Expand All @@ -90,17 +90,17 @@ private static void MarkInclusionStateForNodes(XmlNodeList nodeList, XmlDocument

if (Utils.NodeInList(childNodes[i], nodeList))
{
MarkNodeAsIncluded(childNodesCanonical[i]);
MarkNodeAsIncluded(childNodesCanonical[i]!);
}

XmlAttributeCollection attribNodes = childNodes[i].Attributes;
XmlAttributeCollection? attribNodes = childNodes[i]!.Attributes;
if (attribNodes != null)
{
for (int j = 0; j < attribNodes.Count; j++)
{
if (Utils.NodeInList(attribNodes[j], nodeList))
{
MarkNodeAsIncluded(childNodesCanonical[i].Attributes.Item(j));
MarkNodeAsIncluded(childNodesCanonical[i]!.Attributes!.Item(j)!);
}
}
}
Expand All @@ -121,7 +121,7 @@ internal byte[] GetDigestedBytes(HashAlgorithm hash)
{
_c14nDoc.WriteHash(hash, DocPosition.BeforeRootElement, _ancMgr);
hash.TransformFinalBlock(Array.Empty<byte>(), 0, 0);
byte[] res = (byte[])hash.Hash.Clone();
byte[] res = (byte[])hash.Hash!.Clone();
// reinitialize the hash so it is still usable after the call
hash.Initialize();
return res;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class CanonicalXmlAttribute : XmlAttribute, ICanonicalizableNode
{
private bool _isInNodeSet;

public CanonicalXmlAttribute(string prefix, string localName, string namespaceURI, XmlDocument doc, bool defaultNodeSetInclusionState)
public CanonicalXmlAttribute(string? prefix, string localName, string? namespaceURI, XmlDocument doc, bool defaultNodeSetInclusionState)
: base(prefix, localName, namespaceURI, doc)
{
IsInNodeSet = defaultNodeSetInclusionState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace System.Security.Cryptography.Xml
internal sealed class CanonicalXmlCDataSection : XmlCDataSection, ICanonicalizableNode
{
private bool _isInNodeSet;
public CanonicalXmlCDataSection(string data, XmlDocument doc, bool defaultNodeSetInclusionState) : base(data, doc)
public CanonicalXmlCDataSection(string? data, XmlDocument doc, bool defaultNodeSetInclusionState) : base(data, doc)
{
_isInNodeSet = defaultNodeSetInclusionState;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal sealed class CanonicalXmlComment : XmlComment, ICanonicalizableNode
private bool _isInNodeSet;
private readonly bool _includeComments;

public CanonicalXmlComment(string comment, XmlDocument doc, bool defaultNodeSetInclusionState, bool includeComments)
public CanonicalXmlComment(string? comment, XmlDocument doc, bool defaultNodeSetInclusionState, bool includeComments)
: base(comment, doc)
{
_isInNodeSet = defaultNodeSetInclusionState;
Expand Down Expand Up @@ -55,7 +55,7 @@ public void WriteHash(HashAlgorithm hash, DocPosition docPos, AncestralNamespace
hash.TransformBlock(rgbData, 0, rgbData.Length, rgbData, 0);
rgbData = utf8.GetBytes("<!--");
hash.TransformBlock(rgbData, 0, rgbData.Length, rgbData, 0);
rgbData = utf8.GetBytes(Value);
rgbData = utf8.GetBytes(Value!);
hash.TransformBlock(rgbData, 0, rgbData.Length, rgbData, 0);
rgbData = utf8.GetBytes("-->");
hash.TransformBlock(rgbData, 0, rgbData.Length, rgbData, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,42 +61,42 @@ public void WriteHash(HashAlgorithm hash, DocPosition docPos, AncestralNamespace
}
}

public override XmlElement CreateElement(string prefix, string localName, string namespaceURI)
public override XmlElement CreateElement(string? prefix, string localName, string? namespaceURI)
{
return new CanonicalXmlElement(prefix, localName, namespaceURI, this, _defaultNodeSetInclusionState);
}

public override XmlAttribute CreateAttribute(string prefix, string localName, string namespaceURI)
public override XmlAttribute CreateAttribute(string? prefix, string localName, string? namespaceURI)
{
return new CanonicalXmlAttribute(prefix, localName, namespaceURI, this, _defaultNodeSetInclusionState);
}

protected override XmlAttribute CreateDefaultAttribute(string prefix, string localName, string namespaceURI)
protected override XmlAttribute CreateDefaultAttribute(string? prefix, string localName, string? namespaceURI)
{
return new CanonicalXmlAttribute(prefix, localName, namespaceURI, this, _defaultNodeSetInclusionState);
}

public override XmlText CreateTextNode(string text)
public override XmlText CreateTextNode(string? text)
{
return new CanonicalXmlText(text, this, _defaultNodeSetInclusionState);
}

public override XmlWhitespace CreateWhitespace(string prefix)
public override XmlWhitespace CreateWhitespace(string? prefix)
{
return new CanonicalXmlWhitespace(prefix, this, _defaultNodeSetInclusionState);
}

public override XmlSignificantWhitespace CreateSignificantWhitespace(string text)
public override XmlSignificantWhitespace CreateSignificantWhitespace(string? text)
{
return new CanonicalXmlSignificantWhitespace(text, this, _defaultNodeSetInclusionState);
}

public override XmlProcessingInstruction CreateProcessingInstruction(string target, string data)
public override XmlProcessingInstruction CreateProcessingInstruction(string target, string? data)
{
return new CanonicalXmlProcessingInstruction(target, data, this, _defaultNodeSetInclusionState);
return new CanonicalXmlProcessingInstruction(target, data!, this, _defaultNodeSetInclusionState);
}

public override XmlComment CreateComment(string data)
public override XmlComment CreateComment(string? data)
{
return new CanonicalXmlComment(data, this, _defaultNodeSetInclusionState, _includeComments);
}
Expand All @@ -106,7 +106,7 @@ public override XmlEntityReference CreateEntityReference(string name)
return new CanonicalXmlEntityReference(name, this, _defaultNodeSetInclusionState);
}

public override XmlCDataSection CreateCDataSection(string data)
public override XmlCDataSection CreateCDataSection(string? data)
{
return new CanonicalXmlCDataSection(data, this, _defaultNodeSetInclusionState);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ internal sealed class CanonicalXmlElement : XmlElement, ICanonicalizableNode
{
private bool _isInNodeSet;

public CanonicalXmlElement(string prefix, string localName, string namespaceURI, XmlDocument doc, bool defaultNodeSetInclusionState)
: base(prefix, localName, namespaceURI, doc)
public CanonicalXmlElement(string? prefix, string localName, string? namespaceURI, XmlDocument doc, bool defaultNodeSetInclusionState)
: base(prefix!, localName, namespaceURI, doc)
{
_isInNodeSet = defaultNodeSetInclusionState;
}
Expand All @@ -30,7 +30,7 @@ public void Write(StringBuilder strBuilder, DocPosition docPos, AncestralNamespa
SortedList nsListToRender = new SortedList(new NamespaceSortOrder());
SortedList attrListToRender = new SortedList(new AttributeSortOrder());

XmlAttributeCollection attrList = Attributes;
XmlAttributeCollection? attrList = Attributes;
if (attrList != null)
{
foreach (XmlAttribute attr in attrList)
Expand Down Expand Up @@ -68,11 +68,11 @@ public void Write(StringBuilder strBuilder, DocPosition docPos, AncestralNamespa
strBuilder.Append('<').Append(Name);
foreach (object attr in nsListToRender.GetKeyList())
{
(attr as CanonicalXmlAttribute).Write(strBuilder, docPos, anc);
(attr as CanonicalXmlAttribute)!.Write(strBuilder, docPos, anc);
}
foreach (object attr in attrListToRender.GetKeyList())
{
(attr as CanonicalXmlAttribute).Write(strBuilder, docPos, anc);
(attr as CanonicalXmlAttribute)!.Write(strBuilder, docPos, anc);
}
strBuilder.Append('>');
}
Expand Down Expand Up @@ -141,11 +141,11 @@ public void WriteHash(HashAlgorithm hash, DocPosition docPos, AncestralNamespace
hash.TransformBlock(rgbData, 0, rgbData.Length, rgbData, 0);
foreach (object attr in nsListToRender.GetKeyList())
{
(attr as CanonicalXmlAttribute).WriteHash(hash, docPos, anc);
(attr as CanonicalXmlAttribute)!.WriteHash(hash, docPos, anc);
}
foreach (object attr in attrListToRender.GetKeyList())
{
(attr as CanonicalXmlAttribute).WriteHash(hash, docPos, anc);
(attr as CanonicalXmlAttribute)!.WriteHash(hash, docPos, anc);
}
rgbData = utf8.GetBytes(">");
hash.TransformBlock(rgbData, 0, rgbData.Length, rgbData, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal CanonicalXmlNodeList()

public override XmlNode Item(int index)
{
return (XmlNode)_nodeArray[index];
return (XmlNode)_nodeArray[index]!;
}

public override IEnumerator GetEnumerator()
Expand All @@ -31,7 +31,7 @@ public override int Count
}

// IList methods
public int Add(object value)
public int Add(object? value)
{
if (!(value is XmlNode))
throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, "node");
Expand All @@ -43,24 +43,24 @@ public void Clear()
_nodeArray.Clear();
}

public bool Contains(object value)
public bool Contains(object? value)
{
return _nodeArray.Contains(value);
}

public int IndexOf(object value)
public int IndexOf(object? value)
{
return _nodeArray.IndexOf(value);
}

public void Insert(int index, object value)
public void Insert(int index, object? value)
{
if (!(value is XmlNode))
throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value));
_nodeArray.Insert(index, value);
}

public void Remove(object value)
public void Remove(object? value)
{
_nodeArray.Remove(value);
}
Expand All @@ -80,7 +80,7 @@ public bool IsReadOnly
get { return _nodeArray.IsReadOnly; }
}

object IList.this[int index]
object? IList.this[int index]
{
get { return _nodeArray[index]; }
set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class CanonicalXmlSignificantWhitespace : XmlSignificantWhitespa
{
private bool _isInNodeSet;

public CanonicalXmlSignificantWhitespace(string strData, XmlDocument doc, bool defaultNodeSetInclusionState)
public CanonicalXmlSignificantWhitespace(string? strData, XmlDocument doc, bool defaultNodeSetInclusionState)
: base(strData, doc)
{
_isInNodeSet = defaultNodeSetInclusionState;
Expand Down
Loading

0 comments on commit 0075639

Please sign in to comment.