Adding null-conditional operator on IEnumerable.GetEnumerator()

[smaglio81]

Summary of the changes (Less than 80 chars)

• Added ?. operator to prevent null reference exception (_items?.GetEnumerator())

Addresses #16938

[dnfclas]

All CLA requirements met.

[BrennanConroy]

Looks like there can also be a null ref in CopyTo
https://github.com/aspnet/AspNetCore/blob/d66db3b0fffdcb010914a4d5a8cb4426f5f1c7ce/src/Http/Http/src/Internal/ItemsDictionary.cs#L98-L104

[smaglio81]

Okay, I've attempted to integrate the recommended updates. Let me know how it looks.

[BrennanConroy]

Are you interested in fixing CopyTo also?

[smaglio81]

In the code snippet, it seemed like line 98 was doing a null check? Also, I'm kind of new to the github resolution system ... since that comment didn't have the conversational display (and resolution button) like the other comments I thought it might have been a momentary concern that was self-resolved.

Let me know if you still want it updated?

https://github.com/aspnet/AspNetCore/blob/d66db3b0fffdcb010914a4d5a8cb4426f5f1c7ce/src/Http/Http/src/Internal/ItemsDictionary.cs#L98-L104

[BrennanConroy]

line 98 was doing a null check?

Yeah, but the after the null check line 104 uses _items which would null ref.

[smaglio81]

Great Catch! Since it seems like .CopyTo wants to copy the items into the dictionary, what if around line 102 EnsureDictionary() was called to initialize _items?

[BrennanConroy]

It's copying from _items to the passed in array, we don't want to allocate the dictionary.

I think just adding an else would be good.

[smaglio81]

Can do!

[smaglio81]

So, I started going down the route of using an else statement, but I wanted to format the curly braces in the way the code normally does it. After poking around for an example, it started to seem like else statements weren't used a lot. So, I went back to your original suggestion and used the null-conditional operator.

I also added a unit test for it. I'm not sure if I hit the standards on it.

Sorry for the back and forth, I should have just implemented it on the first review pass.

[BrennanConroy]

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=422315&view=logs

[aspnet-hello]

This comment was made automatically. If there is a problem contact aspnetcore-build@microsoft.com.

I've triaged the above build. I've created/commented on the following issue(s)
https://github.com/aspnet/AspNetCore-Internal/issues/2730
https://github.com/aspnet/AspNetCore-Internal/issues/3241

[BrennanConroy]

Thanks @smaglio81 !