3

At work we have a lot of code like this (pseudocode):

response form_submit(string username, string password) {
  if (  username == ""
     || username.contains(invalid_chars)
     || password.length < 5 
     || ...
     ) {
    return "errors: blah blah";
  } else {
    try {
      db("insert into users(username, password) values (?, ?)", username, password);
      return "success";
    }
    catch (DBException e) {
      return "errors: blah blah";
    }
  }
}

Some of those are enforced as constraints on the DB, some aren't. It makes more sense to me to enforce everything as DB constraints and just do

response form_submit(string username, string password) {
  try {
    db("insert into users(username, password) values (?, ?)", username, password);
    return "success";
  }
  catch (DBException e) {
    return "errors: blah blah";
  }
}

My opinion is obvious here, but what's yours: is this kind of application-level data integrity check bad practise?

jameshfisher
  • 750

2 Answers2

4

Well, all performance and redundancy considerations aside, unless .net database exceptions are more sophisticated than in every other language I've ever worked with, the simple fact of needing to construct an error message readable by non-programmers sort of necessitates the first method, unless you prefer error messages that read something like:

Error #0x4b13: Violated length constraint '>=5' for table 'Users', field 'password'.

What you'll end up doing is recreating the validation code anyway to produce a decent error message, and if you're going to recreate that code anyway, you may as well do it before you incur the expense of a database call.

Karl Bielefeldt
  • 148,830
1

In the database, you won't be able to validate the password length at all (since it's hashed and not plaintext, right? RIGHT?).

Testing for invalid characters in the username must be done before you write your INSERT statement, otherwise you're wide open to a SQL injection attack. Which would be much less likely if you used a parameterized query instead, but it's a risk you must always consider.

Redundancy in your checks is valuable, as ratchet freak pointed out.

alroc
  • 473