1
Vote

Add data validation to Geolocation settings for empty string

description

In the v2.0 release, if you go into the Settings>Advanced and clear out the Latitude and/or Longitude fields then BlogEngine won't run correctly after restarting. The problem is that "" does not parse correctly into a Single/float, so the Settings are never loaded. This causes a NullReferenceException later on in the code.

I had originally tried the "locate" feature but it came back "undefined", so I cleared it out to "reset it". Resulting in much hair pulling as to why nothing worked :-)

comments

neuromancer wrote Jan 8, 2011 at 2:56 AM

Setting them to '0' should work around the issue.
When do you see NullReferenceException?

codenaked wrote Jan 8, 2011 at 5:48 PM

Yeah, the problem is you can only do that by manually updating the settings, as the pages don't work (after restarting the app pool).

The initial callstack was:
[NullReferenceException: Object reference not set to an instance of an object.]
BlogEngine.Core.Web.HttpModules.CompressionModule.System.Web.IHttpModule.Init(HttpApplication context) in CompressionModule.cs:77
System.Web.HttpApplication.InitModulesCommon() +65
System.Web.HttpApplication.InitModules() +43
System.Web.HttpApplication.InitInternal(HttpContext context, HttpApplicationState state, MethodInfo[] handlers) +729
System.Web.HttpApplicationFactory.GetNormalApplicationInstance(HttpContext context) +298
System.Web.HttpApplicationFactory.GetApplicationInstance(HttpContext context) +107
System.Web.HttpRuntime.ProcessRequestInternal(HttpWorkerRequest wr) +289
If I modify CompressionModel.Init like so:
void IHttpModule.Init(HttpApplication context)
{
BlogSettings settings = BlogSettings.Instance;
if (settings == null)
    throw new ArgumentNullException("settings");

// ... original code ...
}
Then I can tell that settings is null. It seems to be a bit of a circular problem with the call to Utils.Log in BlogSettings.Load. The Convert.ChangeType call in the following statement fails, because it can't convert string.Empty to a float, thus calling Utils.Log.
property.SetValue(this, Convert.ChangeType(value, propType, CultureInfo.CurrentCulture), null);
If you take out the call to Utils.Log, then there is no exception raised.

codenaked wrote Jan 9, 2011 at 11:34 PM

The issue is with the Logger class, specifically the GetFileName() method. Logger.GetFileName is ultimately called from the Utils.Log, which was called from the singleton construction of the BlogSettings singleton. As such, when Logger.GetFileName calls BlogSettings.Instance then null is returned.

Basically, as thing that is ultimately called from code that is executed from the singleton construction cannot access BlogSettings.Instance.

neuromancer wrote Jan 10, 2011 at 1:11 AM

You're quite right, there'a a problem here. Having said that, it seems to be the kind of problem, which could be ignored; after all the default value for Single is 0.0. NullreferenceException is not necessary related to this.
You mentioned that it occurs 'after restarting the app pool'. Do you mean stopping and starting app pool'. by 'restarting', do you mean stopping and starting again?

neuromancer wrote Jan 10, 2011 at 2:27 AM

codenaked, you absolutely right. I haven;t seen the error, because it happens only while using DbBlogProvider. In case of XmlBlogProvider logging gets enabled later, after BlogSettongs has been initialized.
Do we need logger, if we have Trace?

codenaked wrote Jan 10, 2011 at 11:57 AM

You are correct that I am using DbBlogProvider. I'm not sure if you'd need Logger or not, just seems like a bad usability issue (i.e. user enters data that is accepted by settings page that prevents the blog app from starting up) that was worth reporting. So adding simple data validation for the geolocation properties, so an empty string is not allowed or converted to 0, on the setting page would probably be fine.

By "restarting" I mean, recycling the app pool. Since this flushes the application and creates a new instance, it results in BlogSettings.Instance to be recreated. Since that can't be loaded, the entire blog app can't be loaded/started.

Hope that makes sense :-)

neuromancer wrote Jan 10, 2011 at 2:23 PM

I totally agree, this needs to be fixed.
there are two issues here: one is an error raised in Settings.load() (as you've explained); the other is the error raised within Util.Log. Neither shouldn't have happened.