ItemsDictionary - NullReferenceException - Merge into AspNetCore 3.1.0?

[smaglio81]

Recently, a pull request (#16947) was merged to fix #16938.

In the details of the merge, there was a line describing that the merge would be applied to 5.0.0-preview1.

However, the issue is occurring in the current release of AspNetCore 3.0.0. I was wondering what I might be able to do to help this fix/patch become a part of the AspNetCore 3.1.0 release?

[Tratcher]

3.1.0 is only accepting critical patches at this point. Anything with an easy workaround is unlikely to qualify for patching.

[smaglio81]

Gotcha. So, putting 3.1.0 to the side, the bug still exists in the 3.X releases.

Is there a way to apply the fix somewhere within the 3.X roadmap?

[Tratcher]

It's a similar bar to patch any released component. It needs to be a high impact issue that's difficult to work around.

The workaround in this scenario is first checking if the collection is empty?

[smaglio81]

Yeah, it definitely possible, but it's not as straight forward as we both might hope.

I ran into the bug when I was trying to use Elmah with an AspNetCore website. The original bug report can be found here, ElmahCore/ElmahCore#51.

In the code snippet below, the code is using reflection to get the public properties from the httpContext.Items collection. Within the code, the Items collection is cast to an IEnumerable (as the code can have any IEnumerable passed into it). Once the collection is cast to IEnumerable, many properties are unavailable to check if the collection is empty.

In an updated Pull Request for Elmah Core (#57) I've got it setup to cast the original object to an IDictionary<object, object> to match the type of httpContext.Items. At that point I can verify if httpContext.Items.Keys.Count == 0, but that doesn't seem very straight forward.

https://github.com/ElmahCore/ElmahCore/blob/a97886f54a20fce5fa6b96b5ab283b96911bd40b/ElmahCore/Error.cs#L155-L177
(hmm ... how do I get this to appear as a code block?)

But, here's the part that lingers with me. The .GetEnumerator() method did work in AspNetCore 2.2.X. It was a bug introduced in AspNetCore 3.X. And it's a bug on a method (.GetEnumerator()) that is generally expected to be safe. So ... if possible, shouldn't that be fixed in a release of AspNetCore 3.X?

[davidfowl]

The null refs seems like a reasonable thing to backport to 3.x.

cc @anurse @Pilchie for a bar check.

[Tratcher]

You're right, this is a regression introduced in 3.0 by PR #9284. That is one of the servicing criteria.

[analogrelay]

Yep, this seems like a significant enough issue and a regression from 3.0, so I think it meets the 3.1.x bar. Given that there is a workaround, and 3.1.0 is locked down tight prior to the upcoming release, I think it would be a 3.1.1 patch candidate.

The workaround (for those looking until it's patched) is to check the count of the items collection prior to enumerating and to avoid enumerating it if the collection is empty.

[analogrelay]

(Clearing milestone just to put it on our triage radar)

[smaglio81]

Thanks everyone

[analogrelay]

@Tratcher go ahead and open a PR to fix this in 3.1 when you're ready.