DotNetNuke

BUG OF THE MONTH | The ‘settings’ object was used before it was verified against null

public string NavigateURL(int tabID, 
                          bool isSuperTab, 
                          IPortalSettings settings, 
                          ....)
{
  ....
  if (isSuperTab)
  {
    url += "&portalid=" + settings.PortalId;
  }

  TabInfo tab = null;
  if (settings != null)
  {
    tab = TabController.Instance.GetTab(tabID, 
            isSuperTab ? Null.NullInteger : settings.PortalId, false);
  }
  ....
}

The PVS-Studio warning: V3095 The ‘settings’ object was used before it was verified against null. Check lines: 190, 195. DotNetNuke.Library NavigationManager.cs 190

I wonder why at first we access the settings.PortalId instance property, and then we check settings for null inequality. Thus, if settings – null and isSuperTab – true, we get NullReferenceException.

Surprisingly, this code fragment has a second contract that links isSuperTab and settings parameters – the ternary operator: isSuperTab ? Null.NullInteger : settings.PortalId. Note that in this case, unlike ifsettings.PortalId is used when isSuperTab is false.

If isSuperTab is true, the settings.PortalId value is not processed. You may think that it’s just an implicit contract, and everything is fine.

Nope.

The code must be easy to read and understandable – you don’t have to think like Sherlock. If you intend to create this contract, write it explicitly in the code. Thus, the developers, the static analyzer, and you will not be confused. 😉

Please click here to see more bugs from this project.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.