CODEWITHSUNDEEP
javamysql
Argus
Argus

Reputation: 150

Java Web App Inserting into mysql, best way

So I'm building a register/login form on java web app and I want to make it nearly perfect. By that I mean secure, fast, clean code.

So right now my code is like that. I have a private structure of User, with setters and getters and a method of registration:

db dbconn = new db();
Connection myconnection = dbconn.Connection();
String sqlString = "INSERT INTO users (name, password) VALUES ('"+Name+"','"+Password+"')";
try {
    Statement myStatement = null;
    myStatement.executeUpdate(sqlString);
    myStatement.close();
    myconnection.close();
} 
catch (SQLException e) {
    System.err.println(e);
}

But the thing is I heard ( not sure ) that just running SQL queries like that is bad example and I shouldn't do that? So I'm just wondering, what is the best way to register a user into my MySQL database? Using procedures? Or is using Statement like that is valid as well?

Upvotes: 0

Views: 847

Answers (2)

nsawaya
nsawaya

Reputation: 623

  • Move

db dbconn = new db(); Connection myconnection = dbconn.Connection();

To a utility class.

  • As proposed in the comments, use PreparedStatement to avoid SQL injections and String concatenations (use ? and don't try to concatenate)
  • I would put all SQL statements in a util class as public static final String

  • See some real world examples on how to do it, for example use a finally block to close the statement and the connection (it will look ugly with the try/catch also inside the finally block)

  • Here's a link to a JDBC best practices refcard (although a bit advanced)

  • A couple reference small tutorials:

    Vogella.com

    Oracle's official docs

Upvotes: 0

Thomas
Thomas

Reputation: 88727

If you just use JDBC (and not an OR mapper) you should use prepared statements due to security reasons. The way you're doing it allows for SQL injection.

Problem: if you concatenate parameter values like you do one could alter the query.

Example: if Name had the value x','y');DELETE FROM users;SELECT * FROM users WHERE name in ('whatever

the query would become:

INSERT INTO users (name, password) VALUES ('x','y');DELETE FROM users;SELECT * FROM users WHERE name in ('whatever', 'password')

Now execute that query and you might wonder where all the users are gone.

Using a PreparedStatement the statement would look like this:

PreparedStatement pst = myconnection.prepareStatement("INSERT INTO users (name, password) VALUES (?,?)");
pst.setString(1, Name); //btw, Java coding conventions state it should be "name" instead of "Name"
pst.setString(2, Password); //your password should be hashed and salted btw!
pst.executeUpdate();

Note how it's not necessary to use single quotes in the statement. PreparedStatement will handle that for you and also escape the values to prevent injection.

A second thought (from a design point of view): you might think about separating user registration etc. from the user objects holding the data (see Single responsibility principle). This is just meant to be a suggestion and there surely are others that don't agree (with valid arguments, often depending on the situation though).

Upvotes: 4

Related Questions