rspec/rules/S2384/csharp/rule.adoc

== Why is this an issue?

Mutable collections are those whose state can be changed. For instance, ``++Array++`` and ``++List<T>++`` are mutable, but ``++System.Collections.ObjectModel.ReadOnlyCollection<T>++`` and ``++System.Collections.Immutable.ImmutableList<T>++`` are not. Mutable collection class members should not be returned to a caller or accepted and stored directly. Doing so leaves you vulnerable to unexpected changes in your class state.


Instead use and store a copy of the mutable collection, or return an immutable collection wrapper, e.g. ``++System.Collections.ObjectModel.ReadOnlyCollection<T>++``.


Note that you can't just return your mutable collection through the ``++IEnumerable<T>++`` interface because the caller of your method/property could cast it down to the mutable type and then change it.


This rule checks that mutable collections are not stored or returned directly.

=== Noncompliant code example

[source,csharp]
----
class A 
{
    private List<string> names = new List<string>();

    public ICollection<string> Names => names; // Noncompliant

    public IEnumerable<string> GetNames()  // Noncompliant
    {
        return names;
    }

    public void SetNames(List<string> strings) 
    {
        this.names = strings;  // Noncompliant
    }
}
----

=== Compliant solution

[source,csharp]
----
class A 
{
    private List<string> names = new List<string>();
    private ReadOnlyCollection<string> readOnlyNames = new ReadOnlyCollection<string>(names);

    public ICollection<string> Names => readOnlyNames; // Return a collection wrapper

    public IEnumerable<string> GetNames()
    {
        names.ToList(); // Make a copy
    }

    public void SetNames(List<string> strings) 
    {
        this.names.Clear();
        this.names.AddRange(strings); // Make a copy
    }
}
----

== Resources

* CWE - https://cwe.mitre.org/data/definitions/374[CWE-374 - Passing Mutable Objects to an Untrusted Method]
* CWE - https://cwe.mitre.org/data/definitions/375[CWE-375 - Returning a Mutable Object to an Untrusted Caller]


ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

include::../message.adoc[]

'''
== Comments And Links
(visible only on this page)

=== on 15 Mar 2017, 16:13:35 Amaury Levé wrote:
\[~ann.campbell.2] I changed a bit this rule to focus only on collections for C# as ``++Date++`` is immutable for us.

=== on 15 Mar 2017, 18:47:09 Ann Campbell wrote:
Works for me [~amaury.leve]

include::../comments-and-links.adoc[]

endif::env-github,rspecator-view[]