Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question

marked as duplicate by Mason Wheeler, gnat, Martijn Pieters, greyfade, World Engineer Mar 16 '13 at 0:07

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

    
personally I prefer redundancy in these kind of things, (like what happens if you migrate your DB over to another server/vendor and forget some constraints (or they just can't be supported) –  ratchet freak Mar 15 '13 at 20:45
add comment

2 Answers

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.

share|improve this answer
    
Well, I think you'd just name all your constraints, then when the DB gives you violated username_valid_char, you lookup friendly_errors['username_valid_char']. –  eegg Mar 15 '13 at 21:19
add comment

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.

share|improve this answer
3  
How would you perform an SQL injection attack using invalid characters if the developer is correctly using parameterized queries? –  Heinzi Mar 15 '13 at 21:10
    
So basically you're saying that some things can't be validated using DB constraints, right? –  Robert Harvey Mar 15 '13 at 21:11
    
You're right about the password issue! My silly example was fortuitous for finding that example. I don't buy the injection argument; obviously that should just be properly escaped. –  eegg Mar 15 '13 at 21:15
2  
@eegg properly escaping is very hard to do correctly (even library escapers can get it wrong) it's better to parameterize –  ratchet freak Mar 15 '13 at 22:24
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.