0

Hi I'm trying to write a piece of code for a simple verification method as part of a MVC.

At present the SQL is not written as a prepared statement so obviously it is at risk to a SQL injection so any help in regards to writing the SQL as a prepared statement would be really helpful.

The method which is in the User model.

public boolean getInfo() {

    try {
        DBAccess dbAccess = new DBAccess();

        String sql = "SELECT username, password FROM owner WHERE username = '" + this.username
                + "'AND password = '" + this.password + "';";

        dbAccess.close();dbAccess.executeQuery(sql);
        dbAccess.close();


        return true;
    } catch (Exception e) {
        return false;
    }
}

I want to get the size of the result set which is generated by the SQL query and if the size of it is 1 return true else it's false.

If you need more info on the rest of the MVC just post and I'll get it up here.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
Jimmy
  • 817
  • 4
  • 13
  • 19

4 Answers4

1

Rather select the fields username and password, you could select the count of them and then reference that value.

So your SQL query would be:

SELECT count(*) FROM owner WHERE username = '" + this.username
            + "'AND password = '" + this.password + "';

That would return the number of matched records, where if number is greater than 0, or equals one, validate them.

Tom Dickinson
  • 382
  • 1
  • 8
  • Wow this is much more simple than I thought it would be. Does count(*) FROM owner WHERE username not need to be in "" though? – Jimmy Mar 28 '12 at 23:34
  • Yes sorry, didn't copy the preceding " before hand. So your SQL line would be: String sql = "SELECT count(*) FROM owner WHERE username = '" + this.username + "'AND password = '" + this.password + "';"; – Tom Dickinson Mar 28 '12 at 23:43
1

Without knowing the details of your DBAccess class, we can't tell you how to do this. I'm guessing it returns a List (but it's a guess, nothing more). If that's the case, you could check the size of the List via list.size() , or see if it returned at least 1 result with !list.isEmpty(). Of course, if it's not a list then this won't work.

And you definitely need to switch to prepared statements. For an example, see this SO post.

Side note: if this method is returning a boolean indicating whether a user exists, it shouldn't be called getInfo()! Something like userExists() would make more sense.

Community
  • 1
  • 1
Kaleb Brasee
  • 51,193
  • 8
  • 108
  • 113
1

Just return the result of ResultSet#next(), assuming that there's an UNIQUE constraint on the username. It returns false if there is no next record.

Here's a concrete kickoff example, slightly rewritten to fix potential SQL injection attack hole, resource leaking and threadsafety problems as shown so far in your code. Also, the altered SQL query should force you to MD5-hash the passwords before saving in DB (you don't want to store passwords plaintext in DB).

public boolean exist(String username, String password) throws SQLException {
    Connection connection = null;
    PreparedStatement statement = null;
    ResultSet resultSet = null;
    boolean exist = false;

    try {
        connection = database.getConnection();
        statement = connection.prepareStatement("SELECT id FROM owner WHERE username = ? AND password = MD5(?)");
        statement.setString(1, username);
        statement.setString(2, password);
        resultSet = statement.executeQuery();
        exist = resultSet.next();
    } finally {
        if (resultSet != null) try { resultSet.close(); } catch (SQLException ignore) {}
        if (statement != null) try { statement.close(); } catch (SQLException ignore) {}
        if (connection != null) try { connection.close(); } catch (SQLException ignore) {}
    }

    return exist;
}
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • Thanks for the response, although where will it check to see if the result set is 1 or 0? I can see that it will check the result set size with the resultSet.next(); line, or should I then invoke this method in the servlet and do the if statement there? – Jimmy Mar 29 '12 at 09:46
  • The `ResultSet#next()` returns `false` if there is no one record. – BalusC Mar 29 '12 at 12:13
0

For your question about preventing sql injection, and if you want to start getting your feet wet with an "ORM like" library, you could use myibatis to create prepared statements. Myibatis is a data mapper which you can create a relatively simple ORM from. As you get more brave, you could move to hibernate or JPA.

http://www.mybatis.org/

DavidB
  • 2,064
  • 3
  • 17
  • 17