Rules of Safe Coding


This is a generic kind of post about some things you should avoid in your code and how you can do them better: The rules. It is a rather generic kind of post, as I do not have a particular topic in mind. It is not an exhaustive list, but just a few things I have thought about as I deal with problems in the code base I am maintaining (and refactoring).

Test Your Strings

There is a temptation, in coding, to assume that today’s state is the norm for all time and that things will not deviate. We end up, therefore, writing code like this:

private string CleanAddress(string street)
{
    if(street.Substring(0,2) == "0 ")
        return street(2);
    else
        return street;
}

The idea being that 0 Any Street is a bogus address. The problem is one day you might switch from a reverse geocoder that places a 0 in an empty street to one that creates an empty string. The line with Substring(0,2) is now invalid, as there is no length.

If we adopt an assume nothing approach (safe coding), we would test the length of the string before attempting to substring it. Without any real refactoring, this can be as simple as:

private string CleanAddress(string street)
{
    if(street.Length < 2)
         return street;

    if(street.Substring(0,2) == "0 ")
        return street(2);
    else
        return street;
}

I am not here to argue the intent, only that this simple test helps us avoid tons of pain that is unnecessary. Even safer, we would want to also test that the string is not null. As we are probably appending this to another string somewhere, we will return an empty string.

private string CleanAddress(string street)
{
    if(street == null)
         return string.Empty;

    if(street.Length < 2)
         return street;

    if(street.Substring(0,2) == "0 ")
        return street(2);
    else
        return street;
}

This is not the best code in the world, so we should refactor, but it does check the two possible conditions that could error out this particular routine (null string or short string).

The choice of what to do with strings that do not adhere to this rule (raise own exception, ignore, return another value) is up to you, but do not assume that strings will always adhere to your expectations.

Input Checking

Okay, so I am cheating a bit here, as this is what you just did when you checked a string for null values before attempting to do something with it, or checked for length before you ripped it apart. But, the concept applies to more than just strings.

For example, what happens when you divide an integer by zero? You end up with an exception. While you can certainly handle this exception, and there are times you should, you can avoid the overhead of throwing an exception via bad code by checking the value. In fact, assuming we did want to throw an exception, we could do something like this:

private int DivideApples(int apples, int people)
{
     if(b == 0)
         throw new DivideByZeroException();

     return a/b;
}

Not extremely elegant, and I would certainly consider creating my own exception to give a better exception message than divide by zero, like "number of people dividing apples must be greater than zero" exception.

Output Checking

The opposite of input checking is output checking. This is checking a value before it goes out and adjusting output (like throw an exception) when the value is out of range. Many books suggest output checking every routine, but this practice is not extremely common.

Never Trust User Input

This one should go without saying. First, a user might not understand the rules of input and input something that is invalid. If you are going to perform a string operation on a user’s input, you can end up with an empty value that causes problems. This is what was covered in the first section.

Another issue is invalid data. Suppose you have an email field that is required. If you do not check the user input, you can end up with an email address like "joe". Does not blow up the application, but it gives you invalid data in your database and makes it impossible to send your user an email message.

Why talk about user input in a code blog entry? After all, you have the ability to do checking on the user interface through validation controls, right? Yes, but there are ways to circumvent user input. It is not as easy in ASP.NET as it was in earlier programming frameworks, but a clever enough hacker might find a way.

To sum this up, validate input on the server, even if it is checked on the client. It is better to spin a few cycles checking something that is correct than it is to throw an unfriendly exception for a user or let corrupt data make it to your database.

The Point

All of these are the same rule, essentially. Any time you have data that crosses a boundary, whether from user to your application or from layer to layer within your application, you should assume that it might not adhere to the rules your application adheres to. While it is less likely you will end up with issues when you are getting information out of your own database, it is not impossible, unless you are the only one who can EVER put data into the database … AND you remember your rules, no matter how far in the future you are still using the database. If the data was not created in your class, assume it could be incorrect. And, even then, you should consider being safe and coding a check in your code.

Peace and Grace,
Greg

Advertisements

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 )

Twitter picture

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

Facebook photo

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

Google+ photo

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

Connecting to %s

%d bloggers like this: