The ?. Operator in foreach Will Not Protect From NullReferenceException

Do you just like the ?. operator? Effectively, who would not? Many individuals like these concise null checks. Nevertheless, in the present day’s article exhibits that the ?. operator could also be tough generally. That’s, it could actually create an phantasm of security when used within the foreach loop.

foreach loop

Let’s begin with a small activity. Check out the next code:

void ForeachTest(IEnumerable<String> assortment)
  
  // #1  
  foreach (var merchandise in assortment.NotNullItems())    
    Console.WriteLine(merchandise);   

  // #2  
  foreach (var merchandise in assortment?.NotNullItems())    
    Console.WriteLine(merchandise); 

Suppose the assortment is null. Have you ever obtained any concepts on how every of the loops will run? Case #2 with ?. appears to be safer. However is that actually so? The article’s title ought to have already planted a seed of doubt in your thoughts.

In any case, we’ll attempt to determine this out beneath. We’ll return to this activity on the finish of the article when we’ve extra info.

Word. The C# Specification makes use of the time period “expression” to indicate the next entity. On this article, we use the time period “enumerable expression”. This may increasingly assist to keep away from confusion once we discuss totally different expressions.

foreach example

Why Is It Harmful To Use the ?. Operator within the Foreach Loop’s Enumerable Expression?

First, let’s recall what the ?. operator is.

It will not take lengthy.

var b = a?.Foo();

So:

  • if a == null,b == null;
  • if a != null, b == a.Foo().

Now let’s check out the foreach loop.

void Foo1(IEnumerable<String> assortment)
  
  foreach (var merchandise in assortment)
    Console.WriteLine(merchandise); 

IL code suggests you could rewrite the above code fragment in C# with out foreach. It could look one thing like this:

void Foo2(IEnumerable<String> assortment)
  
  var enumerator = assortment.GetEnumerator();  
  strive  
      
    whereas (enumerator.MoveNext())    
          
      var merchandise = enumerator.Present;      
      Console.WriteLine(merchandise);    
      
    
  lastly  
      
    if (enumerator != null)
          
      enumerator.Dispose();    
      
  

Word. In some instances, foreach loop’s IL code might develop into an identical to the code for the for loop. Nevertheless, the issue nonetheless persists. I feel we’ll have one other article in regards to the attainable optimizations of the foreach loop.

The assortment.GetEnumerator() is the important thing component right here. In black and white (though it relies on your shade scheme), the code says that when the GetEnumerator methodology known as, the reference is dereferenced. If this reference is null, we get NullReferenceException.

Now let’s check out what occurs within the foreach loop’s enumerable expression with the ?. operator:

static void Foo3(Wrapper wrapper)
  
  foreach (var merchandise in wrapper?.Strings)
    Console.WriteLine(merchandise); 

We might rewrite this code as follows:

static void Foo4(Wrapper wrapper)
  
  IEnumerable<String> strings;
  if (wrapper == null)
      
    strings = null;  
    
  else  
      
    strings = wrapper.Strings;  
  
   
  var enumerator = strings.GetEnumerator();  
  strive  
      
    whereas (enumerator.MoveNext())    
          
      var merchandise = enumerator.Present;      
      Console.WriteLine(merchandise);    
      
    
  lastly  
      
    if (enumerator != null)
          
      enumerator.Dispose();    
      
   

As within the earlier case, the GetEnumerator (strings.GetEnumerator) name happens. Nevertheless, observe that the strings worth could be null if the wrapper is null. Effectively, that is to be anticipated with the ?. operator (we mentioned it earlier). On this case, when making an attempt to name the string.GetEnumerator() methodology, we get a NullReferenceException.

That is why the ?. operator within the foreach loop’s enumerable expression doesn’t shield towards null dereference. It solely creates an phantasm of security.

What Prompted Us To Enhance the Analyzer?

As soon as my colleague got here to me and mentioned — this is the code, we won’t discover the error. I used to be shocked. I recall precisely how I provided to work on the case that concerned the foreach loop’s enumerable expression having the null worth. Checked it out. Certainly, the analyzer was not issuing warnings on the code beneath.

void Test1(IEnumerable<String> assortment,
           Func<String, bool> predicate)
  
  foreach (var merchandise in assortment?.The place(predicate))    
    Console.WriteLine(merchandise); 

The identical was with this code.

void Test2(IEnumerable<String> assortment,           
           Func<String, bool> predicate)
  
  var question = assortment?.The place(predicate);  
  foreach (var merchandise in question)    
    Console.WriteLine(merchandise); 

Nevertheless, the analyzer issued a warning on the next code fragment.

void Test3(IEnumerable<String> assortment,           
           Func<String, bool> predicate,          
           bool flag)
  
  var question = assortment != null ? assortment.The place(predicate) : null;  
  foreach (var merchandise in question)    
    Console.WriteLine(merchandise); 

PVS-Studio warning: V3080 Attainable null dereference. Take into account inspecting ‘question’.

The analyzer would additionally concern a warning on the next code.

IEnumerable<String> GetPotentialNull(IEnumerable<String> assortment,
                                     Func<String, bool> predicate,
                                     bool flag)
  
  return assortment != null ? assortment.The place(predicate) : null; 


void Test4(IEnumerable<String> assortment,           
           Func<String, bool> predicate,          
           bool flag)
  
  foreach (var merchandise in GetPotentialNull(assortment, predicate, flag))    
    Console.WriteLine(merchandise); 

PVS-Studio warning: V3080 Attainable null dereference of the tactic return worth. Take into account inspecting: GetPotentialNull(…).

Why did the analyzer concern warnings for Test3 and Test4, however not for Test1 and Test2? The purpose is that the analyzer sees these instances as totally different:

  • the analyzer didn’t concern a warning if a variable acquired the ?. operator’s end result;
  • an expression might have a null worth. For instance, if a variable instantly acquired null or if a technique returned null. On this case, the analyzer issued a warning.

This differentiation helps the analyzer to deal with every scenario totally. So, consequently, the analyzer:

  • points a extra correct warning;
  • has the flexibility to deal with these instances individually (to lift/decrease the warning stage, to suppress / not suppress, and so on.);
  • has documentation for every case.

What Diagnostics We Refined

In consequence, we improved 2 diagnostic guidelines: V3105 and V3153.

V3105 now detects suspicious code fragments when a variable accommodates the results of the ?. operator. Then, the enumerable expression foreach makes use of this variable.

void Take a look at(IEnumerable<String> assortment,           
          Func<String, bool> predicate)
  
  var question = assortment?.The place(predicate);  
  foreach (var merchandise in question)    
    Console.WriteLine(merchandise); 

PVS-Studio warning: V3105 The ‘question’ variable was used after it was assigned by way of null-conditional operator. NullReferenceException is feasible.

V3153 now detects instances the place the foreach loop’s enumerable expression instantly makes use of the ?. operator.

void Take a look at(IEnumerable<String> assortment, 
          Func<String, bool> predicate)
  
  foreach (var merchandise in assortment?.The place(predicate))    
    Console.WriteLine(merchandise); 

PVS-Studio warning: V3153 Enumerating the results of null-conditional entry operator can result in NullReferenceException. Take into account inspecting: assortment?.The place(predicate).

The Improved Analyzer Detects Extra Points

It is a terrific feeling to see the analyzer works higher! As I’ve already mentioned, we commonly take a look at the analyzer on open-source initiatives. So, after we improved V3105 and V3153, we managed to seek out some new triggerings!

Word. This code was updated once we added the initiatives to our assessments. By now the code might have modified and should not comprise these code fragments.

RavenDB

personal void HandleInternalReplication(DatabaseRecord newRecord,
                                       Listing<IDisposable> instancesToDispose)
  
  var newInternalDestinations = newRecord.Topology?.GetDestinations(_server.NodeTag,
                                                                    Database.Identify,
                                                                    newRecord.DeletionInProgress,
                                                                    _clusterTopology,
                                                                    _server.Engine.CurrentState);

  var internalConnections = DatabaseTopology.FindChanges(_internalDestinations,
                                                         newInternalDestinations);

  if (internalConnections.RemovedDestiantions.Rely > 0) 
      
    var eliminated = internalConnections.RemovedDestiantions
                                     .Choose(r => new InternalReplication      
      
        NodeTag = _clusterTopology.TryGetNodeTagByUrl(r).NodeTag,
        Url = r,
        Database = Database.Identify
      );     

    DropOutgoingConnections(eliminated, instancesToDispose);  
    

  if (internalConnections.AddedDestinations.Rely > 0)  
      
    var added = internalConnections.AddedDestinations
                                   .Choose(r => new InternalReplication    
            
        NodeTag = _clusterTopology.TryGetNodeTagByUrl(r).NodeTag,
        Url = r,
        Database = Database.Identify
      );
    StartOutgoingConnections(added.ToList());  
    

  _internalDestinations.Clear();  
  foreach (var merchandise in newInternalDestinations)  
      
    _internalDestinations.Add(merchandise);  
   

I deliberately listed the complete code fragment. You’ll in all probability agree this concern just isn’t very apparent. And naturally, it is simpler to seek out one thing if you realize what you are on the lookout for. 😉

For those who simplify the code, the difficulty turns into extra apparent.

personal void HandleInternalReplication(DatabaseRecord newRecord,
                                       Listing<IDisposable> instancesToDispose)
  
  var newInternalDestinations = newRecord.Topology?.GetDestinations(....);  
  ....  
  foreach (var merchandise in newInternalDestinations)    
    .... 

The newInternalDestinations variable takes the ?. operator’s end result. If newRecord.Topology is null, newInternalDestinations will even be null. When the execution stream reaches the foreach loop, the NullReferenceException exception shall be thrown.

PVS-Studio warning: V3105 The ‘newInternalDestinations’ variable was used after it was assigned by way of the null-conditional operator. NullReferenceException is feasible. ReplicationLoader.cs 828

What’s extra attention-grabbing, the DatabaseTopology.FindChanges methodology takes the newInternalDestinations variable because the newDestinations parameter and checks it for null.

inside static (HashSet<string> AddedDestinations, HashSet<string> RemovedDestiantions)
FindChanges(IEnumerable<ReplicationNode> oldDestinations,
            Listing<ReplicationNode> newDestinations)
  
  ....  
  if (newDestinations != null)  
      
    newList.AddRange(newDestinations.Choose(s => s.Url));  
    
  .... 

MSBuild

public void LogTelemetry(string eventName,  IDictionary<string, string> properties)
  
  string message = $"Acquired telemetry occasion 'eventName'Atmosphere.NewLine";
  foreach (string key in properties?.Keys)  
      
    message += $"  Property 'key' = 'properties[key]'Atmosphere.NewLine";  
    
  .... 

PVS-Studio warning: V3153 Enumerating the results of null-conditional entry operator can result in NullReferenceException. Take into account inspecting: properties?.Keys. MockEngine.cs 159

Right here the foreach instantly accommodates the ?. operator. Maybe the developer thought the ?. operator would shield from NullReferenceException. However we all know that it is not safer. 😉

Nethermind

This instance is much like the earlier one.

public NLogLogger(....)
  
  ....   
  foreach (FileTarget goal in international::NLog.LogManager
                                            .Configuration                                           
                                           ?.AllTargets                                            
                                            .OfType<FileTarget>())  
      
    ....  
    
  .... 

PVS-Studio warning: V3153 Enumerating the results of null-conditional entry operator can result in NullReferenceException. NLogLogger.cs 50

Additionally, the builders used the ?. operator instantly within the foreach loop’s enumerable expression to keep away from NullReferenceException. Perhaps they’re going to get fortunate, and the Configuration property won’t ever return null. In any other case, someday later this code might play a trick on you.

Roslyn

personal ImmutableArray<char>
GetExcludedCommitCharacters(ImmutableArray<RoslynCompletionItem> roslynItems)
{  
  var hashSet = new HashSet<char>();
  foreach (var roslynItem in roslynItems) 
      
    foreach (var rule in roslynItem.Guidelines?.FilterCharacterRules)    
          
      if (rule.Form == CharacterSetModificationKind.Add)      
              
        foreach (var c in rule.Characters)        
                  
          hashSet.Add(c);        
              
      
      
  
  return hashSet.ToImmutableArray(); 
}

PVS-Studio warning: V3153 Enumerating the results of null-conditional entry operator can result in NullReferenceException. CompletionSource.cs 482

That is nice, is not it? I like it when PVS-Studio finds attention-grabbing instances in compilers or different analyzers.

PVS-Studio

And now it is time to admit that we’re not excellent both. We have made the identical errors.

thumbs up

We commonly verify PVS-Studio with PVS-Studio. That is the way it works:

  • At night time, we construct a brand new model of the analyzer distribution. It consists of modifications we dedicated to the primary department throughout the day;
  • This new model checks numerous initiatives, together with PVS-Studio itself;
  • The BlameNotifier utility notifies builders and managers in regards to the warnings the analyzer issued;
  • Then, we repair the warnings discovered.

And so, after we have improved V3153 and V3105, the analyzer issued a number of warnings on our code. Certainly, the analyzer detected instances when the foreach loop’s enumerable expression contained the ?. operator. Additionally, we discovered oblique instances (when a variable takes a price). We had been fortunate that we hadn’t been getting an exception. In any case, we have already taken the warnings into consideration and stuck the corresponding instances. 😉

This is a code fragment that triggered a warning:

public override void
VisitAnonymousObjectCreationExpression(AnonymousObjectCreationExpressionSyntax node)
  
  foreach (var initializer in node?.Initializers)
    initializer?.Expression?.Settle for(this); 

Yeah, there is a bunch of ?. right here. Attempt to discover the one that can shoot you within the foot. It looks as if ?. operators present most security (use the Crysis nanosuit voice impact whereas studying) to your code, however actually, that is not true.

Is It Attainable To Use the ?. Operator within the Enumerable Expression With out Exceptions?

In fact, you are able to do that. And we have seen such code examples. For instance, the ?? operator can come to the rescue.

The next code is harmful and should result in NullReferenceException:

static void Take a look at(IEnumerable<String> assortment, Func<String, bool> predicate)
  
  foreach (var merchandise in assortment?.The place(predicate))    
    Console.WriteLine(merchandise); 

And this code is not going to throw an exception:

static void Take a look at(IEnumerable<String> assortment, Func<String, bool> predicate)
  
  foreach (var merchandise in    assortment?.The place(predicate) ?? Enumerable.Empty<String>())  
      
    Console.WriteLine(merchandise);  
   

Whereas the ?. operator returns a null worth, the ?? operator leads to Enumerable.Empty<String>(). Due to this fact, there shall be no exception. Nevertheless, including an specific null verify as a substitute might be a good suggestion.

static void Take a look at(IEnumerable<String> assortment, Func<String, bool> predicate)
  
  if (assortment != null)  
      
    foreach (var merchandise in assortment.The place(predicate))      
      Console.WriteLine(merchandise);  
   

Clearly, it seems not so fashionable however clear and straightforward to learn.

Let’s Remedy the Activity Mentioned on the Starting

As chances are you’ll keep in mind, we began the article with the next activity:

void ForeachTest(IEnumerable<String> assortment)
  
  // #1  
  foreach (var merchandise in assortment.NotNullItems())    
    Console.WriteLine(merchandise);   

  // #2  
  foreach (var merchandise in assortment?.NotNullItems())    
    Console.WriteLine(merchandise); 

Now you realize that choice #2 just isn’t protected in any respect. It will not assist you to keep away from NullReferenceException. And what about choice #1? At first look, evidently we’ll have NullReferenceException when calling assortment.NotNullItems(). However that is not essentially true! Suppose NotNullItems is an extension methodology with the next physique:

public static IEnumerable<T> NotNullItems<T>(this IEnumerable<T> assortment) the place T : class
  
  if (assortment == null)    
    return Enumerable.Empty<T>();   

  return assortment.The place(merchandise => merchandise != null); 

As we are able to see, the tactic checks assortment for null. Since on this case, the tactic returns the Enumerable.Empty<T>() worth, there shall be no exception. That’s, loop #1 works efficiently, even when assortment is null.

However the second loop stays harmful. If assortment is null, the NotNullItems methodology just isn’t referred to as. Due to this fact, verify for null doesn’t work. In consequence, we’ve the identical scenario which we saved seeing again and again — an try to name the GetEnumerator() methodology for a null reference.

That is one attention-grabbing case we’ve! Calling the assortment.NotNullItems() methodology explicitly prevents NullReferenceException, however a “protected” name — assortment?.NotNullItems() — doesn’t.

Conclusion

We now have a number of conclusions right here:

  • Don’t use the ?. operator within the foreach loop’s enumerable expression instantly or not directly. It solely creates an phantasm of security;
  • Use a static analyzer commonly.

We, as builders as soon as once more realized that it is essential not solely to develop new diagnostics but additionally to refine the prevailing ones.

As common, be at liberty to comply with me on Twitter.




Supply hyperlink

About PARTH SHAH

Check Also

Kidnappers in Nigeria Release 28 Schoolchildren, Another 81 Still Held, Says Negotiator | World News

KADUNA, Nigeria (Reuters) – Kidnappers who raided a boarding college in northern Nigeria earlier this …

Leave a Reply

Your email address will not be published. Required fields are marked *

x