We have moved to GitHub Issues
Created by Daniël Knippers 24 Jan 2018, 16:08:39 Updated by Daniël Knippers 13 Jun 2018, 11:29:23Tags: PR
I noticed the
GridValueConverter relies on
HttpContext.Current being set, but only to set an "IsDebug" flag on
IGridConfig. This raises a NullReferenceException whenever a Grid property is read in contexts without an HttpContext, like a background thread, resulting in the Grid property value being null. Our specific case is sending scheduled e-mails from a background thread, where the e-mail content is created using the Grid. We can obtain the IPublishedContent just fine, but the Grid property is NULL due to the GridValueConverter crashing in the ConvertDataToSource method. Other property values are present without issues.
Could this be changed to not crash when HttpContext.Current is NULL? Below is the source, where I added a comment above the line that causes the crash. // Umbraco.Core.PropertyEditors.ValueConverters.GridValueConverter.cs#L43 var gridConfig = UmbracoConfig.For.GridConfig( ApplicationContext.Current.ProfilingLogger.Logger, ApplicationContext.Current.ApplicationCache.RuntimeCache, new DirectoryInfo(IOHelper.MapPath(SystemDirectories.AppPlugins)), new DirectoryInfo(IOHelper.MapPath(SystemDirectories.Config)),
// Cause of crash. HttpContext.Current.IsDebuggingEnabled);
Changing this line to
HttpContext.Current?.IsDebuggingEnabled == true would be enough to remedy the NullReferenceException (and of course, causing IsDebug to be set to false whenever there is no
-- Daniël Knippers
Sure, send us a PR?
HttpContext.Current?.IsDebuggingEnabled == true will need to be the verbose version, something like
HttpContext.Current == null || HttpContext.Current.IsDebuggingEnabled I think is what you're trying to achieve?
We don't support C# 6 yet.
@sebastiaan Ah I see, however the equivalent longer version would be
HttpContext.Current != null && HttpContext.Current.IsDebuggingEnabled (otherwise it would set IsDebug to true in Production environments when
HttpContext is null). Will likely submit a PR this week and link it here. Thanks for the quick reply!
Sure thing! I didn't really think it through, will do for the review though ;)
@sebastiaan PR here: https://github.com/umbraco/Umbraco-CMS/pull/2420
@sebastiaan Was this never merged in? The [current version of dev-v7|https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Core/PropertyEditors/ValueConverters/GridValueConverter.cs#L48] still has the old code and will crash without an
That is super weird! I can see it merged and being part of 7.7.10+.. See screenshot.
But it's not in the current file and the current file history says... neh, that commit was never there.
I've cherry-picked it now and it will be part of 7.11.0. I really cannot figure out what went wrong here, very strange.
Reference to the commit, which is definitely in dev-v7 this time: https://github.com/umbraco/Umbraco-CMS/commit/3520f2d1e4138e5da5350fafb5897d3fac2f0641
@sebastiaan Weird, but thanks for including it again regardless :)!
Backwards Compatible: True
Fix Submitted: Pull request
Due in version: 7.11.0