From 9bf910f5ed0f37ddc2aed14b21368f43025982d3 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Sun, 27 Mar 2022 07:53:21 +0100 Subject: [PATCH 01/11] First pass --- .../System.Security.Cryptography.Xml.csproj | 1 + .../Xml/AncestralNamespaceContextManager.cs | 8 +-- .../Cryptography/Xml/AttributeSortOrder.cs | 6 +- .../Cryptography/Xml/CanonicalXmlDocument.cs | 16 ++--- .../Cryptography/Xml/CanonicalXmlNodeList.cs | 14 ++--- .../Cryptography/Xml/CanonicalXmlText.cs | 2 +- .../Security/Cryptography/Xml/CipherData.cs | 8 +-- .../Cryptography/Xml/CipherReference.cs | 4 +- .../Security/Cryptography/Xml/DataObject.cs | 17 +++--- .../Cryptography/Xml/EncryptedReference.cs | 9 +-- .../Cryptography/Xml/EncryptedType.cs | 28 ++++----- .../Security/Cryptography/Xml/EncryptedXml.cs | 48 +++++++-------- .../Cryptography/Xml/EncryptionProperty.cs | 17 +++--- .../Xml/EncryptionPropertyCollection.cs | 12 ++-- .../Cryptography/Xml/KeyInfoEncryptedKey.cs | 4 +- .../Cryptography/Xml/MyXmlDocument.cs | 2 +- .../Cryptography/Xml/NamespaceFrame.cs | 4 +- .../Cryptography/Xml/NamespaceSortOrder.cs | 6 +- .../Xml/RSAPKCS1SignatureDescription.cs | 3 +- .../Cryptography/Xml/ReferenceList.cs | 10 ++-- .../Security/Cryptography/Xml/SignedXml.cs | 59 ++++++++++--------- .../Cryptography/Xml/TransformChain.cs | 8 ++- .../System/Security/Cryptography/Xml/Utils.cs | 4 +- 23 files changed, 150 insertions(+), 140 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System.Security.Cryptography.Xml.csproj b/src/libraries/System.Security.Cryptography.Xml/src/System.Security.Cryptography.Xml.csproj index b61c2ba35a5b93..557b5195291930 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System.Security.Cryptography.Xml.csproj +++ b/src/libraries/System.Security.Cryptography.Xml/src/System.Security.Cryptography.Xml.csproj @@ -4,6 +4,7 @@ $(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum) true $(NoWarn);CA1850 + enable 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/. Commonly Used Types: diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AncestralNamespaceContextManager.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AncestralNamespaceContextManager.cs index 0bbafb6db14ca1..d5e46146f6c3bd 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AncestralNamespaceContextManager.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AncestralNamespaceContextManager.cs @@ -20,9 +20,9 @@ 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--) { @@ -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--) { diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AttributeSortOrder.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AttributeSortOrder.cs index c0ff1120f8b181..ece8cfb89fa20e 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AttributeSortOrder.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AttributeSortOrder.cs @@ -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); diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlDocument.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlDocument.cs index 9713d998428f25..b519bb4a2a273e 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlDocument.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlDocument.cs @@ -61,32 +61,32 @@ 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); } @@ -96,7 +96,7 @@ public override XmlProcessingInstruction CreateProcessingInstruction(string targ 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); } @@ -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); } diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlNodeList.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlNodeList.cs index e1308ea8849808..239eb54b53e57f 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlNodeList.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlNodeList.cs @@ -15,7 +15,7 @@ internal CanonicalXmlNodeList() _nodeArray = new ArrayList(); } - public override XmlNode Item(int index) + public override XmlNode? Item(int index) { return (XmlNode)_nodeArray[index]; } @@ -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"); @@ -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); } @@ -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 diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlText.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlText.cs index 90982628d4be33..de81dfea23e087 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlText.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CanonicalXmlText.cs @@ -11,7 +11,7 @@ internal sealed class CanonicalXmlText : XmlText, ICanonicalizableNode { private bool _isInNodeSet; - public CanonicalXmlText(string strData, XmlDocument doc, bool defaultNodeSetInclusionState) + public CanonicalXmlText(string? strData, XmlDocument doc, bool defaultNodeSetInclusionState) : base(strData, doc) { _isInNodeSet = defaultNodeSetInclusionState; diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherData.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherData.cs index e03b4004a79f97..5e1d3fbc00ab9c 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherData.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherData.cs @@ -7,9 +7,9 @@ namespace System.Security.Cryptography.Xml { public sealed class CipherData { - private XmlElement _cachedXml; - private CipherReference _cipherReference; - private byte[] _cipherValue; + private XmlElement? _cachedXml; + private CipherReference? _cipherReference; + private byte[]? _cipherValue; public CipherData() { } @@ -31,7 +31,7 @@ private bool CacheValid } } - public CipherReference CipherReference + public CipherReference? CipherReference { get { return _cipherReference; } set diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherReference.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherReference.cs index 04e7e8aaa9d8f3..44b38a6f24c784 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherReference.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherReference.cs @@ -7,7 +7,7 @@ namespace System.Security.Cryptography.Xml { public sealed class CipherReference : EncryptedReference { - private byte[] _cipherValue; + private byte[]? _cipherValue; public CipherReference() : base() { @@ -25,7 +25,7 @@ public CipherReference(string uri, TransformChain transformChain) : base(uri, tr } // This method is used to cache results from resolved cipher references. - internal byte[] CipherValue + internal byte[]? CipherValue { get { diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/DataObject.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/DataObject.cs index 146e1ccffe21dc..42d6866cc04d51 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/DataObject.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/DataObject.cs @@ -7,11 +7,11 @@ namespace System.Security.Cryptography.Xml { public class DataObject { - private string _id; - private string _mimeType; - private string _encoding; + private string? _id; + private string? _mimeType; + private string? _encoding; private CanonicalXmlNodeList _elData; - private XmlElement _cachedXml; + private XmlElement? _cachedXml; // // public constructors @@ -37,7 +37,7 @@ public DataObject(string id, string mimeType, string encoding, XmlElement data!! // public properties // - public string Id + public string? Id { get { return _id; } set @@ -47,7 +47,7 @@ public string Id } } - public string MimeType + public string? MimeType { get { return _mimeType; } set @@ -57,7 +57,7 @@ public string MimeType } } - public string Encoding + public string? Encoding { get { return _encoding; } set @@ -97,7 +97,7 @@ private bool CacheValid // public methods // - public XmlElement GetXml() + public XmlElement? GetXml() { if (CacheValid) return (_cachedXml); @@ -117,6 +117,7 @@ internal XmlElement GetXml(XmlDocument document) if (!string.IsNullOrEmpty(_encoding)) objectElement.SetAttribute("Encoding", _encoding); + // red flag - not changed, but always false if (_elData != null) { foreach (XmlNode node in _elData) diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedReference.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedReference.cs index 8bbeffb18e99c9..cc27396c52e94b 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedReference.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedReference.cs @@ -9,8 +9,8 @@ public abstract class EncryptedReference { private string _uri; private string _referenceType; - private TransformChain _transformChain; - internal XmlElement _cachedXml; + private TransformChain? _transformChain; + internal XmlElement? _cachedXml; protected EncryptedReference() : this(string.Empty, new TransformChain()) { @@ -79,7 +79,8 @@ protected internal bool CacheValid public virtual XmlElement GetXml() { - if (CacheValid) return _cachedXml; + // red flag + if (CacheValid) return _cachedXml!; XmlDocument document = new XmlDocument(); document.PreserveWhitespace = true; @@ -115,7 +116,7 @@ public virtual void LoadXml(XmlElement value!!) // Transforms XmlNamespaceManager nsm = new XmlNamespaceManager(value.OwnerDocument.NameTable); nsm.AddNamespace("ds", SignedXml.XmlDsigNamespaceUrl); - XmlNode transformsNode = value.SelectSingleNode("ds:Transforms", nsm); + XmlNode? transformsNode = value.SelectSingleNode("ds:Transforms", nsm); if (transformsNode != null) TransformChain.LoadXml(transformsNode as XmlElement); diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedType.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedType.cs index 2e5680f94f37a5..0b832419a8a481 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedType.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedType.cs @@ -7,15 +7,15 @@ namespace System.Security.Cryptography.Xml { public abstract class EncryptedType { - private string _id; - private string _type; - private string _mimeType; - private string _encoding; - private EncryptionMethod _encryptionMethod; - private CipherData _cipherData; - private EncryptionPropertyCollection _props; - private KeyInfo _keyInfo; - internal XmlElement _cachedXml; + private string? _id; + private string? _type; + private string? _mimeType; + private string? _encoding; + private EncryptionMethod? _encryptionMethod; + private CipherData? _cipherData; + private EncryptionPropertyCollection? _props; + private KeyInfo? _keyInfo; + internal XmlElement? _cachedXml; internal bool CacheValid { @@ -25,7 +25,7 @@ internal bool CacheValid } } - public virtual string Id + public virtual string? Id { get { return _id; } set @@ -35,7 +35,7 @@ public virtual string Id } } - public virtual string Type + public virtual string? Type { get { return _type; } set @@ -45,7 +45,7 @@ public virtual string Type } } - public virtual string MimeType + public virtual string? MimeType { get { return _mimeType; } set @@ -55,7 +55,7 @@ public virtual string MimeType } } - public virtual string Encoding + public virtual string? Encoding { get { return _encoding; } set @@ -76,7 +76,7 @@ public KeyInfo KeyInfo set { _keyInfo = value; } } - public virtual EncryptionMethod EncryptionMethod + public virtual EncryptionMethod? EncryptionMethod { get { return _encryptionMethod; } set diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs index 4ea614d17c1fac..380e958ce6a6e4 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs @@ -59,8 +59,8 @@ public class EncryptedXml // private readonly XmlDocument _document; - private Evidence _evidence; - private XmlResolver _xmlResolver; + private Evidence? _evidence; + private XmlResolver? _xmlResolver; // hash table defining the key name mapping private const int _capacity = 4; // 4 is a reasonable capacity for // the key name mapping hash table @@ -79,7 +79,7 @@ public EncryptedXml() : this(new XmlDocument()) { } public EncryptedXml(XmlDocument document) : this(document, null) { } - public EncryptedXml(XmlDocument document, Evidence evidence) + public EncryptedXml(XmlDocument document, Evidence? evidence) { _document = document; _evidence = evidence; @@ -124,14 +124,14 @@ public int XmlDSigSearchDepth } // The evidence of the document being loaded: will be used to resolve external URIs - public Evidence DocumentEvidence + public Evidence? DocumentEvidence { get { return _evidence; } set { _evidence = value; } } // The resolver to use for external entities - public XmlResolver Resolver + public XmlResolver? Resolver { get { return _xmlResolver; } set { _xmlResolver = value; } @@ -180,7 +180,7 @@ public string Recipient private byte[] GetCipherValue(CipherData cipherData!!) { - Stream inputStream = null; + Stream? inputStream = null; if (cipherData.CipherValue != null) { @@ -199,7 +199,7 @@ private byte[] GetCipherValue(CipherData cipherData!!) if (cipherData.CipherReference.Uri.Length == 0) { // self referenced Uri - string baseUri = (_document == null ? null : _document.BaseURI); + string? baseUri = (_document == null ? null : _document.BaseURI); TransformChain tc = cipherData.CipherReference.TransformChain; if (tc == null) { @@ -230,7 +230,7 @@ private byte[] GetCipherValue(CipherData cipherData!!) throw new CryptographicException(SR.Cryptography_Xml_UriNotResolved, cipherData.CipherReference.Uri); } // read the output stream into a memory stream - byte[] cipherValue = null; + byte[]? cipherValue = null; using (MemoryStream ms = new MemoryStream()) { Utils.Pump(decInputStream, ms); @@ -255,13 +255,13 @@ private byte[] GetCipherValue(CipherData cipherData!!) // // This describes how the application wants to associate id references to elements - public virtual XmlElement GetIdElement(XmlDocument document, string idValue) + public virtual XmlElement? GetIdElement(XmlDocument document, string idValue) { return SignedXml.DefaultGetIdElement(document, idValue); } // default behaviour is to look for the IV in the CipherValue - public virtual byte[] GetDecryptionIV(EncryptedData encryptedData!!, string symmetricAlgorithmUri) + public virtual byte[] GetDecryptionIV(EncryptedData encryptedData!!, string? symmetricAlgorithmUri) { int initBytesSize; // If the Uri is not provided by the application, try to get it from the EncryptionMethod @@ -294,15 +294,15 @@ public virtual byte[] GetDecryptionIV(EncryptedData encryptedData!!, string symm // default behaviour is to look for keys defined by an EncryptedKey clause // either directly or through a KeyInfoRetrievalMethod, and key names in the key mapping - public virtual SymmetricAlgorithm GetDecryptionKey(EncryptedData encryptedData!!, string symmetricAlgorithmUri) + public virtual SymmetricAlgorithm? GetDecryptionKey(EncryptedData encryptedData!!, string symmetricAlgorithmUri) { if (encryptedData.KeyInfo == null) return null; IEnumerator keyInfoEnum = encryptedData.KeyInfo.GetEnumerator(); - KeyInfoRetrievalMethod kiRetrievalMethod; - KeyInfoName kiName; - KeyInfoEncryptedKey kiEncKey; - EncryptedKey ek = null; + KeyInfoRetrievalMethod? kiRetrievalMethod; + KeyInfoName? kiName; + KeyInfoEncryptedKey? kiEncKey; + EncryptedKey? ek = null; while (keyInfoEnum.MoveNext()) { @@ -316,12 +316,12 @@ public virtual SymmetricAlgorithm GetDecryptionKey(EncryptedData encryptedData!! // try to get it from a CarriedKeyName XmlNamespaceManager nsm = new XmlNamespaceManager(_document.NameTable); nsm.AddNamespace("enc", EncryptedXml.XmlEncNamespaceUrl); - XmlNodeList encryptedKeyList = _document.SelectNodes("//enc:EncryptedKey", nsm); + XmlNodeList? encryptedKeyList = _document.SelectNodes("//enc:EncryptedKey", nsm); if (encryptedKeyList != null) { foreach (XmlNode encryptedKeyNode in encryptedKeyList) { - XmlElement encryptedKeyElement = encryptedKeyNode as XmlElement; + XmlElement? encryptedKeyElement = encryptedKeyNode as XmlElement; EncryptedKey ek1 = new EncryptedKey(); ek1.LoadXml(encryptedKeyElement); if (ek1.CarriedKeyName == keyName && ek1.Recipient == Recipient) @@ -376,17 +376,17 @@ public virtual SymmetricAlgorithm GetDecryptionKey(EncryptedData encryptedData!! } // Try to decrypt the EncryptedKey given the key mapping - public virtual byte[] DecryptEncryptedKey(EncryptedKey encryptedKey!!) + public virtual byte[]? DecryptEncryptedKey(EncryptedKey encryptedKey!!) { if (encryptedKey.KeyInfo == null) return null; IEnumerator keyInfoEnum = encryptedKey.KeyInfo.GetEnumerator(); - KeyInfoName kiName; - KeyInfoX509Data kiX509Data; - KeyInfoRetrievalMethod kiRetrievalMethod; - KeyInfoEncryptedKey kiEncKey; - EncryptedKey ek; + KeyInfoName? kiName; + KeyInfoX509Data? kiX509Data; + KeyInfoRetrievalMethod? kiRetrievalMethod; + KeyInfoEncryptedKey? kiEncKey; + EncryptedKey? ek; bool fOAEP; while (keyInfoEnum.MoveNext()) @@ -396,7 +396,7 @@ public virtual byte[] DecryptEncryptedKey(EncryptedKey encryptedKey!!) { // Get the decryption key from the key mapping string keyName = kiName.Value; - object kek = _keyNameMapping[keyName]; + object? kek = _keyNameMapping[keyName]; if (kek != null) { if (encryptedKey.CipherData == null || encryptedKey.CipherData.CipherValue == null) diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionProperty.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionProperty.cs index e7e0c6131025c6..1a7ff8cd137e93 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionProperty.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionProperty.cs @@ -1,16 +1,17 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Xml; namespace System.Security.Cryptography.Xml { public sealed class EncryptionProperty { - private string _target; - private string _id; + private string? _target; + private string? _id; private XmlElement _elemProp; - private XmlElement _cachedXml; + private XmlElement? _cachedXml; // We are being lax here as per the spec public EncryptionProperty() { } @@ -24,12 +25,12 @@ public EncryptionProperty(XmlElement elementProperty!!) _cachedXml = null; } - public string Id + public string? Id { get { return _id; } } - public string Target + public string? Target { get { return _target; } } @@ -59,7 +60,8 @@ private bool CacheValid public XmlElement GetXml() { - if (CacheValid) return _cachedXml; + // red flag + if (CacheValid) return _cachedXml!; XmlDocument document = new XmlDocument(); document.PreserveWhitespace = true; @@ -68,7 +70,8 @@ public XmlElement GetXml() internal XmlElement GetXml(XmlDocument document) { - return document.ImportNode(_elemProp, true) as XmlElement; + //red flag + return (document.ImportNode(_elemProp, true) as XmlElement)!; } public void LoadXml(XmlElement value!!) diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionPropertyCollection.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionPropertyCollection.cs index 0fca5ef01afeb6..94553fab29a066 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionPropertyCollection.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptionPropertyCollection.cs @@ -25,7 +25,7 @@ public int Count } /// - int IList.Add(object value) + int IList.Add(object? value) { if (!(value is EncryptionProperty)) throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value)); @@ -44,7 +44,7 @@ public void Clear() } /// - bool IList.Contains(object value) + bool IList.Contains(object? value) { if (!(value is EncryptionProperty)) throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value)); @@ -58,7 +58,7 @@ public bool Contains(EncryptionProperty value) } /// - int IList.IndexOf(object value) + int IList.IndexOf(object? value) { if (!(value is EncryptionProperty)) throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value)); @@ -72,7 +72,7 @@ public int IndexOf(EncryptionProperty value) } /// - void IList.Insert(int index, object value) + void IList.Insert(int index, object? value) { if (!(value is EncryptionProperty)) throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value)); @@ -86,7 +86,7 @@ public void Insert(int index, EncryptionProperty value) } /// - void IList.Remove(object value) + void IList.Remove(object? value) { if (!(value is EncryptionProperty)) throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value)); @@ -133,7 +133,7 @@ public EncryptionProperty this[int index] } /// - object IList.this[int index] + object? IList.this[int index] { get { return _props[index]; } set diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/KeyInfoEncryptedKey.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/KeyInfoEncryptedKey.cs index 2ddeee99d99029..02321596fdb833 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/KeyInfoEncryptedKey.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/KeyInfoEncryptedKey.cs @@ -7,7 +7,7 @@ namespace System.Security.Cryptography.Xml { public class KeyInfoEncryptedKey : KeyInfoClause { - private EncryptedKey _encryptedKey; + private EncryptedKey? _encryptedKey; public KeyInfoEncryptedKey() { } @@ -16,7 +16,7 @@ public KeyInfoEncryptedKey(EncryptedKey encryptedKey) _encryptedKey = encryptedKey; } - public EncryptedKey EncryptedKey + public EncryptedKey? EncryptedKey { get { return _encryptedKey; } set { _encryptedKey = value; } diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/MyXmlDocument.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/MyXmlDocument.cs index 808a3bbbd777f8..281c64f6929e9d 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/MyXmlDocument.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/MyXmlDocument.cs @@ -7,7 +7,7 @@ namespace System.Security.Cryptography.Xml { internal sealed class MyXmlDocument : XmlDocument { - protected override XmlAttribute CreateDefaultAttribute(string prefix, string localName, string namespaceURI) + protected override XmlAttribute CreateDefaultAttribute(string? prefix, string localName, string? namespaceURI) { return CreateAttribute(prefix, localName, namespaceURI); } diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceFrame.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceFrame.cs index 1af976d8fb8a39..4aa6ce9bd113bf 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceFrame.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceFrame.cs @@ -21,7 +21,7 @@ internal void AddRendered(XmlAttribute attr) _rendered.Add(Utils.GetNamespacePrefix(attr), attr); } - internal XmlAttribute GetRendered(string nsPrefix) + internal XmlAttribute? GetRendered(string nsPrefix) { return (XmlAttribute)_rendered[nsPrefix]; } @@ -31,7 +31,7 @@ internal void AddUnrendered(XmlAttribute attr) _unrendered.Add(Utils.GetNamespacePrefix(attr), attr); } - internal XmlAttribute GetUnrendered(string nsPrefix) + internal XmlAttribute? GetUnrendered(string nsPrefix) { return (XmlAttribute)_unrendered[nsPrefix]; } diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceSortOrder.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceSortOrder.cs index 1e998ef7e88e20..7a0766dbdc3fd7 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceSortOrder.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceSortOrder.cs @@ -10,10 +10,10 @@ internal sealed class NamespaceSortOrder : IComparer { internal NamespaceSortOrder() { } - 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(); bool nodeAdefault = Utils.IsDefaultNamespaceNode(nodeA); diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/RSAPKCS1SignatureDescription.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/RSAPKCS1SignatureDescription.cs index f19884fbc88609..5b9f14552f8646 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/RSAPKCS1SignatureDescription.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/RSAPKCS1SignatureDescription.cs @@ -16,7 +16,8 @@ public RSAPKCS1SignatureDescription(string hashAlgorithmName) public sealed override AsymmetricSignatureDeformatter CreateDeformatter(AsymmetricAlgorithm key) { var item = (AsymmetricSignatureDeformatter)CryptoConfig.CreateFromName(DeformatterAlgorithm); - item.SetKey(key); + //red flag + item!.SetKey(key); item.SetHashAlgorithm(DigestAlgorithm); return item; } diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/ReferenceList.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/ReferenceList.cs index 6c4dc63a443179..fb65df2e7c47f9 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/ReferenceList.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/ReferenceList.cs @@ -24,7 +24,7 @@ public int Count get { return _references.Count; } } - public int Add(object value!!) + public int Add(object? value!!) { if (!(value is DataReference) && !(value is KeyReference)) throw new ArgumentException(SR.Cryptography_Xml_IncorrectObjectType, nameof(value)); @@ -37,12 +37,12 @@ public void Clear() _references.Clear(); } - public bool Contains(object value) + public bool Contains(object? value) { return _references.Contains(value); } - public int IndexOf(object value) + public int IndexOf(object? value) { return _references.IndexOf(value); } @@ -55,7 +55,7 @@ public void Insert(int index, object value!!) _references.Insert(index, value); } - public void Remove(object value) + public void Remove(object? value) { _references.Remove(value); } @@ -84,7 +84,7 @@ public EncryptedReference this[int index] } /// - object IList.this[int index] + object? IList.this[int index] { get { return _references[index]; } set diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/SignedXml.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/SignedXml.cs index 48b75deec69584..35cc0f6f16c18e 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/SignedXml.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/SignedXml.cs @@ -4,6 +4,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Security.Cryptography.X509Certificates; using System.Xml; @@ -13,28 +14,28 @@ namespace System.Security.Cryptography.Xml public class SignedXml { protected Signature m_signature; - protected string m_strSigningKeyName; + protected string? m_strSigningKeyName; - private AsymmetricAlgorithm _signingKey; - private XmlDocument _containingDocument; - private IEnumerator _keyInfoEnum; - private X509Certificate2Collection _x509Collection; - private IEnumerator _x509Enum; + private AsymmetricAlgorithm? _signingKey; + private XmlDocument? _containingDocument; + private IEnumerator? _keyInfoEnum; + private X509Certificate2Collection? _x509Collection; + private IEnumerator? _x509Enum; private bool[] _refProcessed; private int[] _refLevelCache; internal XmlResolver _xmlResolver; - internal XmlElement _context; + internal XmlElement? _context; private bool _bResolverSet; private Func _signatureFormatValidator = DefaultSignatureFormatValidator; - private Collection _safeCanonicalizationMethods; + private Collection? _safeCanonicalizationMethods; // Built in canonicalization algorithm URIs - private static IList s_knownCanonicalizationMethods; + private static IList? s_knownCanonicalizationMethods; // Built in transform algorithm URIs (excluding canonicalization URIs) - private static IList s_defaultSafeTransformMethods; + private static IList? s_defaultSafeTransformMethods; // additional HMAC Url identifiers private const string XmlDsigMoreHMACMD5Url = "http://www.w3.org/2001/04/xmldsig-more#hmac-md5"; @@ -44,7 +45,7 @@ public class SignedXml private const string XmlDsigMoreHMACRIPEMD160Url = "http://www.w3.org/2001/04/xmldsig-more#hmac-ripemd160"; // defines the XML encryption processing rules - private EncryptedXml _exml; + private EncryptedXml? _exml; // // public constant Url identifiers most frequently used within the XML Signature classes @@ -100,7 +101,7 @@ public SignedXml(XmlElement elem!!) Initialize(elem); } - private void Initialize(XmlElement element) + private void Initialize(XmlElement? element) { _containingDocument = (element == null ? null : element.OwnerDocument); _context = element; @@ -117,7 +118,7 @@ private void Initialize(XmlElement element) // /// - public string SigningKeyName + public string? SigningKeyName { get { return m_strSigningKeyName; } set { m_strSigningKeyName = value; } @@ -450,14 +451,14 @@ public void ComputeSignature(KeyedHashAlgorithm macAlg!!) // virtual methods // - protected virtual AsymmetricAlgorithm GetPublicKey() + protected virtual AsymmetricAlgorithm? GetPublicKey() { if (KeyInfo == null) throw new CryptographicException(SR.Cryptography_Xml_KeyInfoRequired); if (_x509Enum != null) { - AsymmetricAlgorithm key = GetNextCertificatePublicKey(); + AsymmetricAlgorithm? key = GetNextCertificatePublicKey(); if (key != null) return key; } @@ -468,22 +469,22 @@ protected virtual AsymmetricAlgorithm GetPublicKey() // In our implementation, we move to the next KeyInfo clause which is an RSAKeyValue, DSAKeyValue or KeyInfoX509Data while (_keyInfoEnum.MoveNext()) { - RSAKeyValue rsaKeyValue = _keyInfoEnum.Current as RSAKeyValue; + RSAKeyValue? rsaKeyValue = _keyInfoEnum.Current as RSAKeyValue; if (rsaKeyValue != null) return rsaKeyValue.Key; - DSAKeyValue dsaKeyValue = _keyInfoEnum.Current as DSAKeyValue; + DSAKeyValue? dsaKeyValue = _keyInfoEnum.Current as DSAKeyValue; if (dsaKeyValue != null) return dsaKeyValue.Key; - KeyInfoX509Data x509Data = _keyInfoEnum.Current as KeyInfoX509Data; + KeyInfoX509Data? x509Data = _keyInfoEnum.Current as KeyInfoX509Data; if (x509Data != null) { _x509Collection = Utils.BuildBagOfCerts(x509Data, CertUsageType.Verification); if (_x509Collection.Count > 0) { _x509Enum = _x509Collection.GetEnumerator(); - AsymmetricAlgorithm key = GetNextCertificatePublicKey(); + AsymmetricAlgorithm? key = GetNextCertificatePublicKey(); if (key != null) return key; } @@ -500,7 +501,7 @@ private X509Certificate2Collection BuildBagOfCerts() { foreach (KeyInfoClause clause in KeyInfo) { - KeyInfoX509Data x509Data = clause as KeyInfoX509Data; + KeyInfoX509Data? x509Data = clause as KeyInfoX509Data; if (x509Data != null) collection.AddRange(Utils.BuildBagOfCerts(x509Data, CertUsageType.Verification)); } @@ -509,11 +510,11 @@ private X509Certificate2Collection BuildBagOfCerts() return collection; } - private AsymmetricAlgorithm GetNextCertificatePublicKey() + private AsymmetricAlgorithm? GetNextCertificatePublicKey() { while (_x509Enum.MoveNext()) { - X509Certificate2 certificate = (X509Certificate2)_x509Enum.Current; + X509Certificate2? certificate = (X509Certificate2)_x509Enum.Current; if (certificate != null) return Utils.GetAnyPublicKey(certificate); } @@ -521,12 +522,12 @@ private AsymmetricAlgorithm GetNextCertificatePublicKey() return null; } - public virtual XmlElement GetIdElement(XmlDocument document, string idValue) + public virtual XmlElement? GetIdElement(XmlDocument? document, string idValue) { return DefaultGetIdElement(document, idValue); } - internal static XmlElement DefaultGetIdElement(XmlDocument document, string idValue) + internal static XmlElement? DefaultGetIdElement(XmlDocument? document, string idValue) { if (document == null) return null; @@ -833,10 +834,10 @@ public ArrayList References set { _references = value; } } - public int Compare(object a, object b) + public int Compare(object? a, object? b) { - Reference referenceA = a as Reference; - Reference referenceB = b as Reference; + Reference? referenceA = a as Reference; + Reference? referenceB = b as Reference; // Get the indexes int iIndexA = 0; @@ -1040,7 +1041,7 @@ private bool CheckSignedInfo(KeyedHashAlgorithm macAlg!!) return true; } - private static XmlElement GetSingleReferenceTarget(XmlDocument document, string idAttributeName, string idValue) + private static XmlElement? GetSingleReferenceTarget(XmlDocument document, string idAttributeName, string idValue) { // idValue has already been tested as an NCName (unless overridden for compatibility), so there's no // escaping that needs to be done here. @@ -1055,7 +1056,7 @@ private static XmlElement GetSingleReferenceTarget(XmlDocument document, string // In this case, we'll treat it the same as having found nothing across all fallbacks (but shortcut so that we don't // fall into a trap of finding a secondary element which wasn't the originally signed one). - XmlNodeList nodeList = document.SelectNodes(xPath); + XmlNodeList? nodeList = document.SelectNodes(xPath); if (nodeList == null || nodeList.Count == 0) { diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/TransformChain.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/TransformChain.cs index 2b208fda41a54e..32995c8907d4ac 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/TransformChain.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/TransformChain.cs @@ -181,14 +181,16 @@ internal void LoadXml(XmlElement value!!) XmlNamespaceManager nsm = new XmlNamespaceManager(value.OwnerDocument.NameTable); nsm.AddNamespace("ds", SignedXml.XmlDsigNamespaceUrl); - XmlNodeList transformNodes = value.SelectNodes("ds:Transform", nsm); - if (transformNodes.Count == 0) + XmlNodeList? transformNodes = value.SelectNodes("ds:Transform", nsm); + //red flag + if (transformNodes!.Count == 0) throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Transforms"); _transforms.Clear(); for (int i = 0; i < transformNodes.Count; ++i) { - XmlElement transformElement = (XmlElement)transformNodes.Item(i); + //red flag + XmlElement transformElement = (XmlElement)transformNodes.Item(i)!; string algorithm = Utils.GetAttribute(transformElement, "Algorithm", SignedXml.XmlDsigNamespaceUrl); Transform transform = CryptoHelpers.CreateFromName(algorithm); if (transform == null) diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/Utils.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/Utils.cs index a3b766792701b6..99f76a83584798 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/Utils.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/Utils.cs @@ -654,7 +654,7 @@ internal static X509Certificate2Collection BuildBagOfCerts(KeyInfoX509Data keyIn { if (stores[index] != null) { - X509Certificate2Collection filters = null; + X509Certificate2Collection? filters = null; // We don't care if we can't open the store. try { @@ -736,7 +736,7 @@ internal static bool IsSelfSigned(X509Chain chain) return false; } - internal static AsymmetricAlgorithm GetAnyPublicKey(X509Certificate2 certificate) + internal static AsymmetricAlgorithm? GetAnyPublicKey(X509Certificate2 certificate) { AsymmetricAlgorithm algorithm = (AsymmetricAlgorithm)certificate.GetRSAPublicKey() ?? certificate.GetECDsaPublicKey(); From 9d70502a8137f951cbd9af26197be457772930b7 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Wed, 23 Mar 2022 07:20:35 +0000 Subject: [PATCH 02/11] Bind to immutables --- .../src/ConfigurationBinder.cs | 5 +++- .../tests/ConfigurationBinderTests.cs | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index dc248c4cc45a37..b6f7d5111e66e7 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -422,7 +422,10 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig bool hasDefaultConstructor = type.GetConstructors(DeclaredOnlyLookup).Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); if (!hasDefaultConstructor) { - throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + //steve: records don't have default constructors, and there's no way to differentiate between a record and + //any other type -- or is there...? + return Activator.CreateInstance(type, new object[] {"", 0}); + //throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index bcbeddfd0a7d92..8b708aca507697 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -112,6 +112,10 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public IConfigurationSection DerivedSection { get; set; } } +#if NETCOREAPP + public record RecordTypeOptions(string Color, int Length); +#endif + public struct ValueTypeOptions { public int MyInt32 { get; set; } @@ -994,6 +998,25 @@ public void CanBindValueTypeOptions() Assert.Equal("hello world", options.MyString); } +#if NETCOREAPP + [Fact] + public void CanBindRecordOptions() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } +#endif + [Fact] public void CanBindByteArray() { From b3c423befa0041ec9284114c0de2158da8ab3ec6 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Thu, 24 Mar 2022 21:38:04 +0000 Subject: [PATCH 03/11] Supports records and immutable types, but breaks a couple of tests --- .../src/ConfigurationBinder.cs | 77 +++++++++++++++++-- .../tests/ConfigurationBinderTests.cs | 29 +++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index b6f7d5111e66e7..60cbeee15713ad 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -238,13 +238,33 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig object? propertyValue = property.GetValue(instance); bool hasSetter = property.SetMethod != null && (property.SetMethod.IsPublic || options.BindNonPublicProperties); - if (propertyValue == null && !hasSetter) + if (!hasSetter) { - // Property doesn't have a value and we cannot set it so there is no - // point in going further down the graph + var backingField = property.DeclaringType?.GetField($"<{property.Name}>k__BackingField", DeclaredOnlyLookup); + if (backingField != null) + { + // Property doesn't have a value and we cannot set it so there is no + // point in going further down the graph + backingField!.SetValue(instance, propertyValue); + + propertyValue = GetFieldValue(property, backingField, instance, config, options); + + if (propertyValue != null) + { + backingField.SetValue(instance, propertyValue); + } + + return; + } + return; } + // if (propertyValue == null) + // { + // return; + // } + propertyValue = GetPropertyValue(property, instance, config, options); if (propertyValue != null && hasSetter) @@ -419,12 +439,37 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig if (!type.IsValueType) { - bool hasDefaultConstructor = type.GetConstructors(DeclaredOnlyLookup).Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); + ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); + + bool hasDefaultConstructor = constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); + if (!hasDefaultConstructor) { + if (constructors.Length == 1 && !constructors[0].IsPublic ) + { + throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + } //steve: records don't have default constructors, and there's no way to differentiate between a record and + + // call the one with the least amount of parameters + + ParameterInfo[] smallestParameters = constructors[0].GetParameters(); + + for (int index = 1; index < constructors.Length; index++) + { + ParameterInfo[] parameterInfos = constructors[index].GetParameters(); + + // we want the smallest amount of parameters, but we don't the copy constructor (a constructor with 1 parameter of the same type as the containing type) + if (parameterInfos.Length < smallestParameters.Length && (parameterInfos.Length != 1 && parameterInfos[0].ParameterType != type)) + { + smallestParameters = parameterInfos; + } + } + + object?[] parameters = smallestParameters.Select(p => GetDefault(p.ParameterType)).ToArray(); + //any other type -- or is there...? - return Activator.CreateInstance(type, new object[] {"", 0}); + return Activator.CreateInstance(type, parameters); //throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } } @@ -439,6 +484,16 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig } } + private static object? GetDefault(Type type) + { + if (type.IsValueType) + { + return Activator.CreateInstance(type); + } + + return null; + } + [RequiresUnreferencedCode("Cannot statically analyze what the element type is of the value objects in the dictionary so its members may be trimmed.")] private static void BindDictionary( object? dictionary, @@ -701,6 +756,18 @@ private static List GetAllProperties( options); } + [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] + private static object? GetFieldValue(MemberInfo propertyInfo, FieldInfo field, object instance, + IConfiguration config, BinderOptions options) + { + string fieldName = GetPropertyName(propertyInfo); + return BindInstance( + field.FieldType, + field.GetValue(instance), + config.GetSection(fieldName), + options); + } + private static string GetPropertyName(MemberInfo property!!) { // Check for a custom property name used for configuration key binding diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 8b708aca507697..dedc7721d7d999 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -116,6 +116,18 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public record RecordTypeOptions(string Color, int Length); #endif + public class ImmutableLengthAndColorClass + { + public ImmutableLengthAndColorClass(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + } + public struct ValueTypeOptions { public int MyInt32 { get; set; } @@ -998,6 +1010,23 @@ public void CanBindValueTypeOptions() Assert.Equal("hello world", options.MyString); } + [Fact] + public void CanBindImmutableClass() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + #if NETCOREAPP [Fact] public void CanBindRecordOptions() From 074dfb704c0545fec37c7bdf5632e015bea57d93 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 25 Mar 2022 21:15:48 +0000 Subject: [PATCH 04/11] Tidy up --- .../src/ConfigurationBinder.cs | 115 +++++++++--------- 1 file changed, 60 insertions(+), 55 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 60cbeee15713ad..34863f54ce0b66 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -5,7 +5,6 @@ using System.Collections; using System.Collections.Generic; using System.ComponentModel; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; @@ -238,33 +237,11 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig object? propertyValue = property.GetValue(instance); bool hasSetter = property.SetMethod != null && (property.SetMethod.IsPublic || options.BindNonPublicProperties); - if (!hasSetter) + if (propertyValue == null && !hasSetter) { - var backingField = property.DeclaringType?.GetField($"<{property.Name}>k__BackingField", DeclaredOnlyLookup); - if (backingField != null) - { - // Property doesn't have a value and we cannot set it so there is no - // point in going further down the graph - backingField!.SetValue(instance, propertyValue); - - propertyValue = GetFieldValue(property, backingField, instance, config, options); - - if (propertyValue != null) - { - backingField.SetValue(instance, propertyValue); - } - - return; - } - return; } - // if (propertyValue == null) - // { - // return; - // } - propertyValue = GetPropertyValue(property, instance, config, options); if (propertyValue != null && hasSetter) @@ -379,7 +356,7 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig return instance; } - instance = CreateInstance(type); + instance = CreateInstance(type, config, options); } // See if its a Dictionary @@ -420,7 +397,12 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig return instance; } - private static object? CreateInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type) + [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] + private static object? CreateInstance( + [DynamicallyAccessedMembers( + DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type, + IConfiguration config, + BinderOptions options) { if (type.IsInterface || type.IsAbstract) { @@ -441,36 +423,26 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig { ConstructorInfo[] constructors = type.GetConstructors(DeclaredOnlyLookup); - bool hasDefaultConstructor = constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); + bool hasParameterlessPublicConstructor = constructors.Any(ctor => ctor.IsPublic && ctor.GetParameters().Length == 0); - if (!hasDefaultConstructor) + if (!hasParameterlessPublicConstructor) { - if (constructors.Length == 1 && !constructors[0].IsPublic ) + // if the only constructor is private, then throw + if (constructors.Length == 1 && !constructors[0].IsPublic) { throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } - //steve: records don't have default constructors, and there's no way to differentiate between a record and - // call the one with the least amount of parameters + ParameterInfo[] parametersForFirstConstructor = constructors[0].GetParameters(); - ParameterInfo[] smallestParameters = constructors[0].GetParameters(); + List parameterValues = new List(); - for (int index = 1; index < constructors.Length; index++) + foreach (var parameter in parametersForFirstConstructor) { - ParameterInfo[] parameterInfos = constructors[index].GetParameters(); - - // we want the smallest amount of parameters, but we don't the copy constructor (a constructor with 1 parameter of the same type as the containing type) - if (parameterInfos.Length < smallestParameters.Length && (parameterInfos.Length != 1 && parameterInfos[0].ParameterType != type)) - { - smallestParameters = parameterInfos; - } + parameterValues.Add(GetParameterValue(parameter, config, options)); } - object?[] parameters = smallestParameters.Select(p => GetDefault(p.ParameterType)).ToArray(); - - //any other type -- or is there...? - return Activator.CreateInstance(type, parameters); - //throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); + return Activator.CreateInstance(type, parameterValues.ToArray()); } } @@ -484,16 +456,6 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig } } - private static object? GetDefault(Type type) - { - if (type.IsValueType) - { - return Activator.CreateInstance(type); - } - - return null; - } - [RequiresUnreferencedCode("Cannot statically analyze what the element type is of the value objects in the dictionary so its members may be trimmed.")] private static void BindDictionary( object? dictionary, @@ -745,6 +707,18 @@ private static List GetAllProperties( return allProperties; } + [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] + private static object? GetParameterValue(ParameterInfo property, IConfiguration config, BinderOptions options) + { + string parameterName = GetParameterName(property); + + return BindInstance( + property.ParameterType, + null, + config.GetSection(parameterName), + options); + } + [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] private static object? GetPropertyValue(PropertyInfo property, object instance, IConfiguration config, BinderOptions options) { @@ -768,6 +742,37 @@ private static List GetAllProperties( options); } + // todo: steve - we might not need this; currently, these attributes are only applicable to properties and + // not parameters. We might be able to get rid of this method once confirm whether it's needed or not. + + private static string GetParameterName(ParameterInfo parameter) + { + // Check for a custom property name used for configuration key binding + foreach (var attributeData in parameter.GetCustomAttributesData()) + { + if (attributeData.AttributeType != typeof(ConfigurationKeyNameAttribute)) + { + continue; + } + + // Ensure ConfigurationKeyName constructor signature matches expectations + if (attributeData.ConstructorArguments.Count != 1) + { + break; + } + + // Assumes ConfigurationKeyName constructor first arg is the string key name + string? name = attributeData + .ConstructorArguments[0] + .Value? + .ToString(); + + return !string.IsNullOrWhiteSpace(name) ? name : parameter.Name!; + } + + return parameter.Name!; + } + private static string GetPropertyName(MemberInfo property!!) { // Check for a custom property name used for configuration key binding From 961dfb5465925776668d62374fa14bb47ddf47e7 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 25 Mar 2022 21:23:59 +0000 Subject: [PATCH 05/11] Reinstate comments that were mistakenly removed --- .../src/ConfigurationBinder.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 34863f54ce0b66..29f57a0a50ab2a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -239,6 +239,8 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig if (propertyValue == null && !hasSetter) { + // Property doesn't have a value and we cannot set it so there is no + // point in going further down the graph return; } From 47b199f5daf010be67a124bcec46f4cd81c105ac Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 25 Mar 2022 21:25:46 +0000 Subject: [PATCH 06/11] Removed unused method --- .../src/ConfigurationBinder.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 29f57a0a50ab2a..49be09332c2676 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -732,18 +732,6 @@ private static List GetAllProperties( options); } - [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)] - private static object? GetFieldValue(MemberInfo propertyInfo, FieldInfo field, object instance, - IConfiguration config, BinderOptions options) - { - string fieldName = GetPropertyName(propertyInfo); - return BindInstance( - field.FieldType, - field.GetValue(instance), - config.GetSection(fieldName), - options); - } - // todo: steve - we might not need this; currently, these attributes are only applicable to properties and // not parameters. We might be able to get rid of this method once confirm whether it's needed or not. From e3fcbd055aa93483c4665b6806ed7a0e89908562 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 25 Mar 2022 21:44:50 +0000 Subject: [PATCH 07/11] Change to pick biggest non-parameterless constructor --- .../src/ConfigurationBinder.cs | 15 +++- .../tests/ConfigurationBinderTests.cs | 72 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 49be09332c2676..a4e2bc219fbdf2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -435,11 +435,22 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } - ParameterInfo[] parametersForFirstConstructor = constructors[0].GetParameters(); + // find the constructor that most closely matches the amount of fields in config + + ParameterInfo[] parameters = constructors[0].GetParameters(); + + for (int index = 1; index < constructors.Length; index++) + { + ParameterInfo[] constructorParameters = constructors[index].GetParameters(); + if (constructorParameters.Length > parameters.Length) + { + parameters = constructorParameters; + } + } List parameterValues = new List(); - foreach (var parameter in parametersForFirstConstructor) + foreach (var parameter in parameters) { parameterValues.Add(GetParameterValue(parameter, config, options)); } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index dedc7721d7d999..0cb4a63af2649f 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -128,6 +128,36 @@ public ImmutableLengthAndColorClass(string color, int length) public int Length { get; } } + public class ImmutableClassWithConstructorOverloads + { + public ImmutableClassWithConstructorOverloads(string color) + { + Color = color; + } + + public ImmutableClassWithConstructorOverloads(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + } + + public class SemiImmutableType + { + public SemiImmutableType(string color, int length) + { + Color = color; + Length = length; + } + + public string Color { get; } + public int Length { get; } + public decimal Thickness { get; set; } + } + public struct ValueTypeOptions { public int MyInt32 { get; set; } @@ -1027,6 +1057,48 @@ public void CanBindImmutableClass() Assert.Equal("Green", options.Color); } + // If the immutable type has multiple constructors, + // then pick the one with the most parameters + // and try to bind as many as possible. + // The example below has a type that has two constructors, one + // that just sets one property, and one that sets both. + // It should pick the one that sets both. + [Fact] + public void CanBindImmutableClass_PicksBiggestNonParameterlessConstructor() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + } + + [Fact] + public void CanBindSemiImmutableClass() + { + var dic = new Dictionary + { + {"Length", "42"}, + {"Color", "Green"}, + {"Thickness", "1.23"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal(42, options.Length); + Assert.Equal("Green", options.Color); + Assert.Equal(1.23m, options.Thickness); + } + #if NETCOREAPP [Fact] public void CanBindRecordOptions() From 7e356750978381dd2c9addddaefbcdac11287900 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 25 Mar 2022 21:52:36 +0000 Subject: [PATCH 08/11] Rephrase comment --- .../src/ConfigurationBinder.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index a4e2bc219fbdf2..d2e41d4efb4588 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -435,8 +435,7 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig throw new InvalidOperationException(SR.Format(SR.Error_MissingParameterlessConstructor, type)); } - // find the constructor that most closely matches the amount of fields in config - + // find the biggest constructor so that we can bind to the most parameters ParameterInfo[] parameters = constructors[0].GetParameters(); for (int index = 1; index < constructors.Length; index++) From 6bb530b7e2e0a69e6eecdfea77276579f8954377 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 25 Mar 2022 23:23:31 +0000 Subject: [PATCH 09/11] Add test `CanBindMutableClassWitNestedImmutableObject` --- .../tests/ConfigurationBinderTests.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 0cb4a63af2649f..3b98c4291b471a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -116,6 +116,12 @@ public class DerivedOptionsWithIConfigurationSection : DerivedOptions public record RecordTypeOptions(string Color, int Length); #endif + public class ContainerWithNestedImmutableObject + { + public string ContainerName { get; set; } + public ImmutableLengthAndColorClass LengthAndColor { get; set; } + } + public class ImmutableLengthAndColorClass { public ImmutableLengthAndColorClass(string color, int length) @@ -1057,6 +1063,25 @@ public void CanBindImmutableClass() Assert.Equal("Green", options.Color); } + [Fact] + public void CanBindMutableClassWitNestedImmutableObject() + { + var dic = new Dictionary + { + {"ContainerName", "Container123"}, + {"LengthAndColor:Length", "42"}, + {"LengthAndColor:Color", "Green"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + Assert.Equal("Container123", options.ContainerName); + Assert.Equal(42, options.LengthAndColor.Length); + Assert.Equal("Green", options.LengthAndColor.Color); + } + // If the immutable type has multiple constructors, // then pick the one with the most parameters // and try to bind as many as possible. From 6150dbdaae7710077826c0e4ddd4d574ca955097 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Sat, 26 Mar 2022 05:44:26 +0000 Subject: [PATCH 10/11] PR feedback. Make 'chosen constructor' test more obvious, and directly invoke `ConstructorInfo` --- .../src/ConfigurationBinder.cs | 5 +- .../tests/ConfigurationBinderTests.cs | 49 +++++++++++++------ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index d2e41d4efb4588..f6e4b15efe42ff 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -438,12 +438,15 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig // find the biggest constructor so that we can bind to the most parameters ParameterInfo[] parameters = constructors[0].GetParameters(); + int indexOfChosenConstructor = 0; + for (int index = 1; index < constructors.Length; index++) { ParameterInfo[] constructorParameters = constructors[index].GetParameters(); if (constructorParameters.Length > parameters.Length) { parameters = constructorParameters; + indexOfChosenConstructor = index; } } @@ -454,7 +457,7 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig parameterValues.Add(GetParameterValue(parameter, config, options)); } - return Activator.CreateInstance(type, parameterValues.ToArray()); + return constructors[indexOfChosenConstructor].Invoke(parameterValues.ToArray()); } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 3b98c4291b471a..8b02fd7d8bb547 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -136,19 +136,36 @@ public ImmutableLengthAndColorClass(string color, int length) public class ImmutableClassWithConstructorOverloads { - public ImmutableClassWithConstructorOverloads(string color) + public ImmutableClassWithConstructorOverloads(string string1, int int1) { - Color = color; + String1 = string1; + Int1 = int1; } - public ImmutableClassWithConstructorOverloads(string color, int length) + public ImmutableClassWithConstructorOverloads(string string1, int int1, string string2) { - Color = color; - Length = length; + String1 = string1; + Int1 = int1; + String2 = string2; } - public string Color { get; } - public int Length { get; } + public ImmutableClassWithConstructorOverloads(string string1, int int1, string string2, int int2) + { + String1 = string1; + Int1 = int1; + String2 = string2; + Int2 = int2; + } + + public ImmutableClassWithConstructorOverloads(string string1) + { + String1 = string1; + } + + public string String1 { get; } + public string String2 { get; } + public int Int1 { get; } + public int Int2 { get; } } public class SemiImmutableType @@ -1085,24 +1102,28 @@ public void CanBindMutableClassWitNestedImmutableObject() // If the immutable type has multiple constructors, // then pick the one with the most parameters // and try to bind as many as possible. - // The example below has a type that has two constructors, one - // that just sets one property, and one that sets both. - // It should pick the one that sets both. + // The type used in example below has multiple constructors in + // an arbitrary order, but/ with the biggest (chosen) one + // deliberately not first or last. [Fact] public void CanBindImmutableClass_PicksBiggestNonParameterlessConstructor() { var dic = new Dictionary { - {"Length", "42"}, - {"Color", "Green"}, + {"String1", "s1"}, + {"Int1", "1"}, + {"String2", "s2"}, + {"Int2", "2"}, }; var configurationBuilder = new ConfigurationBuilder(); configurationBuilder.AddInMemoryCollection(dic); var config = configurationBuilder.Build(); var options = config.Get(); - Assert.Equal(42, options.Length); - Assert.Equal("Green", options.Color); + Assert.Equal("s1", options.String1); + Assert.Equal("s2", options.String2); + Assert.Equal(1, options.Int1); + Assert.Equal(2, options.Int2); } [Fact] From d7356e4c68ed7fafb955369c97dd608ce497bfd3 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Sat, 26 Mar 2022 06:19:33 +0000 Subject: [PATCH 11/11] PR feedback - use array instead of list to hold parameter values --- .../src/ConfigurationBinder.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index f6e4b15efe42ff..2be0cfa2f78e20 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Buffers; using System.Collections; using System.Collections.Generic; using System.ComponentModel; @@ -450,14 +451,14 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig } } - List parameterValues = new List(); + object?[] parameterValues = new object?[parameters.Length]; - foreach (var parameter in parameters) + for (int index = 0; index < parameters.Length; index++) { - parameterValues.Add(GetParameterValue(parameter, config, options)); + parameterValues[index] = GetParameterValue(parameters[index], config, options); } - return constructors[indexOfChosenConstructor].Invoke(parameterValues.ToArray()); + return constructors[indexOfChosenConstructor].Invoke(parameterValues); } }