0 
try {
    Statement stmt = con.createStatement();
    stmt.executeUpdate("INSERT into emailacc(fname,lname,uname,mail,passwd,passwd2,date,month,year) values('"+fname+","+lname+","+uname+","+mail+","+passwd+","+passwd2+","+date+","+selectMonth+","+year+"')");
    out.println("<h3><font color='green'>Information Added Successfully.</font><br> You Are a registered User now.</h3><br>");
    con.close();
} catch(Exception e) {
    out.println("Exception caught : "+e);   
}

Why is it happening? Last time I did the same but it didn't happen, whats wrong with it?

Improve this question
0

3 Answers 3

Reset to default
4

Well to start with what's wrong with it is that you're including the values directly into your SQL. Don't do that. Ever. Use a parameterized SQL statement via PreparedStatement, and set the parameter values appropriately. That way you don't need to worry about SQL injection attacks, and it'll also be a lot easier to look at what the actual SQL is, without worrying about where the values come from (or rather, separating those two concerns).

I suspect the immediate problem is that you're not quoting any values, so you've got a SQL statement like a longer version of:

INSERT into Foo(name) VALUES (jon)

rather than

INSERT into Foo(name) VALUES ('jon')

... but using parameterized SQL will fix this anyway, so please don't just change the SQL to include single quotes everywhere.

Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

asked 9 mins ago, answered 6 mins ago - you're really fast :)
I just don't think that you should waste yourself in such primitive questions. I don't like the fact that most reputation on this site is being credited for answering numerous primitive questions.
@Oleg: So you don't think the answer is "not useful" (as the tooltip describes) - you just take issue with my freedom to answer what I wish to answer? Wow. The net votes for an answer should be based on the answer's content, not on who posted it. You should be aware that neither your downvote nor any other upvotes this answer gets will affect my reputation, due to the way the rep cap works.
@Oleg: I'm deleting my previous comment now, but I'd just like you to remember the purpose of voting - to indicate the quality of an answer for a specific question. If you really think my answer deserves a downvote on its own merit (without any reference to its authorship) that's fine - but in that case, I'd appreciate comments about the answer rather than about my choice of which questions to answer.
@JonSkeet I'm upvoting because the answer is fine, although I would have swapped the answer with the warning. And even though you and Balus pushed me outta #1 ;)
|
1

Is because you are omitting single quotes, for avoid this mistakes my recommendation is to use PreraredStatement, also in order to proper close connection it mus be in a finally block , you code must look at this:

    try {
        PreparedStatement stmt = con.prepareStatement("INSERT into emailacc(fname,lname,uname,mail,passwd,passwd2,date,month,year) values(?,?,?,?,?,?,?,?,?)");
        stmt.setString(1,fname);
        stmt.setString(2,lname);
        stmt.setString(3,uname);
        stmt.setString(4,mail);
        stmt.setString(5,passwd);
        stmt.setString(6,passwd2);
        stmt.setDate(7,date); //you need convert your date to java.sql.Date if 'date' field of database is of type date. If not setString is fine
        stmt.setInt(8,selectMonth);
        stmt.setInt(9,year);
        stmt.executeUpdate();
        out.println("<h3><font color='green'>Information Added Successfully.</font><br> You Are a registered User now.</h3><br>");
    } catch (Exception e) {
        con.rollback();
        out.println("Exception caught : " + e);
    } finally {
        if (con != null) {
            try {
                con.close();
            } catch(SQLException ex){
                //DO NOTHING
            }
        }
    }

You can learn more of PreparedStatemt in:

http://download.oracle.com/javase/tutorial/jdbc/basics/prepared.html

A final note: PreparedStament are more efficent thant Statement and avoid the SQL Injection hack so PrepararedStatement is more secure. Try use always a PreparedStatement

Improve this answer

Comments

1

Your insert statement is missing quotes between the string insert statement should be:

INSERT into emailacc(fname,lname,uname,mail,passwd,passwd2,date,month,year) values('"+fname+"','"+lname+"','"+uname+"','"+mail+"','"+passwd+"','"+passwd2+"',"+date+","+selectMonth+","+year+")");

every column varchar or text should be between single quotes also double check your date format you might have to use the to_date function : to_date(,'DD-MM-YYYY') just a sample

Improve this answer

4 Comments

This is a bad way of "fixing" the problem. The OP really should be using parameterized SQL instead.
you're right it would be better but we are all missing on a design flaw. The code seems to be written right into a servlet because of the out.println('html code'). This should obviously be extracted to a java class to be precompiled and it would be a much better design "do-it-all" script are not meant to be coded in java
Sure, it's not great design either way - but I'd rather have something badly designed but fundamentally secure than a screaming security hole which is what your answer still leaves (with no warning).
I believe there are different ways of fixing sql injection and proper user input validation is one of them. A good regular expression filter will pick them up every time. But I am not one to dictate how people are to proceed. I agree parameterized sql is better. This is just getting silly

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.