A code-quality improving contest!

This is about what I found in EPiServer and why it concerns *you*, and a contest for you to enter in. Not only for the glory, but for the fine prizes!

My wife is very understanding, she let’s me ramble on about my everyday nerdy problems and says nothing if I pull a half-nighter (I don’t do full-nighters any more, the privilege of age aka experience) digging really deep into that memory dump and not even replying when spoken to.

The other day, I received an e-mail from a Swedish on-line forum with the message being essentially: “We’ve been hacked and our password storage is not safe. You should change your password on other sites if it’s also used here”. I went off in a spin, telling her about the simple fact that we’ve known for 40 years how to store passwords securely and irreversibly!

She then says: ‘You’re scaring me. You always go on about simple things like that and how the same mistakes keep being made over and over. It sounds like an industry full of programmers who don’t know their profession.’

I could not really say anything to that. The evidence is that it’s true. Then again I guess it’s human to err. But still… It is scary.

Shameless plug: If you want a tool to simply your life when a sites’ password database gets hacked, or if you just want help with managing all your passwords check out Xecrets on-line password generator and store.

Now to the real topic of this post and another even simpler rule than how to store passwords.

Do not ever catch unspecified exceptions silently.

That’s not a very hard rule to follow. But it’s getting violated every day, hundreds if not thousand times even as you read this.

On a site that I am involved in we started getting reports about intermittent behavior concerning logins.

I asked for logs, then increased logging levels, and more logs. (This was greatly simplified by the consolidated logs produced by the remote logging service.)

After about 4 hours of pouring over the logs, my own code and reflecting on the EPiServer code – I found the following:

public override MembershipUser GetUser(object providerUserKey, bool userIsOnline)
{
    foreach (MembershipProvider provider in this.ActiveMembershipProviders)
    {
        try
        {
            MembershipUser user = provider.GetUser(providerUserKey, userIsOnline);
            if (user != null)
            {
                this.CurrentMembershipUser = user;
                this.CurrentProvider = provider;
                return user;
            }
            continue;
        }
        catch
        {
            continue;
        }
    }
    return null;
}

 

This causes *any* exception to return as-if the user was not found! This in turn causes rather unpleasant effects in our case, since the calling code now assumes that the user does not exist. And of course it turns out we can get a SQL Server deadlock here!

But the real point is not that I found a bug in EPiServer. My code has bugs too. Lots of them. But… I should hope there are no silent catch-alls!

Everyone now, do a ‘find in files’ in Visual Studio with something like the following regular expression on your code base:

(catch~(:b*\())|(catch:b*\(:b*Exception.*\))

Then, check each and every one and in *all* cases add at least logging! Do not *ever* eat an exception silently.

There is never any reason to consume an exception silently. Never. Let’s repeat that. Never. Ok, just to make things clear: Never. Got it? Never…

Unfortunately this has been said so many times before, and still…

So, let’s have a contest!

Use the above regular expression or whatever tools you have to find and remove silent consuming of unspecified exceptions. The one who posts the most found and fixed will get a computer related book of their choice (within reason) from me personally. The contest lasts for 30 days, and requires at least 3 entrants and > 10 fixes total for the price to be sent. There’s no shame in fixing bugs! I’m going to do the same on my own code base, who knows…

Oh, and I’ll buy a coffee mug for anyone who can provide a convincing argument for a single situation where it’s appropriate to silently consume an unspecified exception.

PS – The contest is open for EPiServer too!