5

I am using JAVA and MySQL for updating a table with the input parameters coming as JSON input. The problem is I don't know what all parameters would be present and absent in the input, so it's possible to get null values for some parameters. And because of which when I run my update query all the columns in my database store those null values. I have browsed the web and looked at various solution on stack. I found this as a tested solution but it's not working for me. I am new to this so I am open for any other way to achieve this:

 String updateQuery = "UPDATE " + USER_TABLE + " SET " + USER_TABLE_FIRST_NAME 
    + "=IFNULL("+ user.getFirstName() + "," + USER_TABLE_FIRST_NAME + ")," 
    + USER_TABLE_LAST_NAME + "='" + user.getLastName() + "'," 
    + USER_TABLE_ABOUT_ME + "='" + user.getAboutMe() + "',"
    + USER_TABLE_CITY + "='" + user.getCity() + "',"
    + USER_TABLE_DOB + "='" + user.getDateOfBirth() 
    + "' WHERE " + USER_TABLE_ID + "='" + user.getUserId() + "'";

Though IFNULL avoids setting up null values and retains the existing value of the database, its not working the other way around when its doesn't have null value but actual value. I was just testing one column i.e USER_TABLE_FIRST_NAME. And "user" is the JSON object where I am sending the input as

    {
     "firstName":"Rakesh",
     "city":"Jaipur"
    }

It gives me the below error:

 com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Unknown column 'Rakesh' in 'field list'
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
at com.mysql.jdbc.Util.handleNewInstance(Util.java:406)
at com.mysql.jdbc.Util.getInstance(Util.java:381)
at com.mysql.jdbc.SQLError.createSQLException(SQLError.java:1030)
at com.mysql.jdbc.SQLError.createSQLException(SQLError.java:956)
at com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:3491)
at com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:3423)
at com.mysql.jdbc.MysqlIO.sendCommand(MysqlIO.java:1936)
at com.mysql.jdbc.MysqlIO.sqlQueryDirect(MysqlIO.java:2060)
at com.mysql.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2536)
at com.mysql.jdbc.StatementImpl.executeUpdate(StatementImpl.java:1564)
at com.mysql.jdbc.StatementImpl.executeUpdate(StatementImpl.java:1485)
at com.sports.jogar.services.UserService.updateUser(UserService.java:174)
at com.sports.jogar.resources.UserResource.updateUser(UserResource.java:72)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory$1.invoke(ResourceMethodInvocationHandlerFactory.java:81)
at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:151)
at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:171)
at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$TypeOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:195)
at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:104)
at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:387)
at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:331)
at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:103)
at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:269)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:271)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:267)
at org.glassfish.jersey.internal.Errors.process(Errors.java:315)
at org.glassfish.jersey.internal.Errors.process(Errors.java:297)
at org.glassfish.jersey.internal.Errors.process(Errors.java:267)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:297)
at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:252)
at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:1025)
at org.glassfish.jersey.servlet.WebComponent.service(WebComponent.java:372)
at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:382)
at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:345)
at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:220)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:291)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:219)
at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106)
at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502)
at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:142)
at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:617)
at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:518)
at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1091)
at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:668)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1527)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1484)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
at java.lang.Thread.run(Thread.java:745)
Piyush Sharma
  • 53
  • 1
  • 5

2 Answers2

5

You are inserting the user name straight into the SQL without escaping or even quoting. I think you simply missed the apostrophes.

To prevent SQL injection issues, NEVER insert SQL string constants from dynamic data, ALWAYS use PreparedStatement and insert markers.

Alternatively, escape the values, but using markers is much safer, and improves SQL performance by allowing the database to cache the compiled SQL statement.

String updateQuery = "UPDATE " + USER_TABLE +
                       " SET " + USER_TABLE_FIRST_NAME + "=IFNULL(? ," + USER_TABLE_FIRST_NAME + ")," +
                                 USER_TABLE_LAST_NAME + "=?," +
                                 USER_TABLE_ABOUT_ME + "=?," +
                                 USER_TABLE_CITY + "=?," +
                                 USER_TABLE_DOB + "=?" +
                     " WHERE " + USER_TABLE_ID + "=?";
PreparedStatement stmt = conn.prepareStatement(updateQuery);
stmt.setString(1, user.getFirstName());
stmt.setString(2, user.getLastName());
stmt.setString(3, user.getAboutMe());
stmt.setString(4, user.getCity());
stmt.setString(5, user.getDateOfBirth());
stmt.setString(6, user.getUserId());

Note: Answer extended to cover the null check issue.

When you're using simple string injection, "A='" + name + "'" becomes A='Joe' for a non-null value but A='null' for a null value, which is definitely not what you want.

By using parameter markers, the value of ? can be null, which means that IFNULL(?, Name) will give the exact behavior needed, i.e. using the value of ? when it's not null, and the value of NAME when ? is null.

Andreas
  • 154,647
  • 11
  • 152
  • 247
  • Please explain down-vote. All the other code sample here (so far) will fail if the user is named O'Malley. – Andreas Aug 14 '15 at 21:25
  • Escaping the special characters is not the crux of the issue here. OP expects IFNULL to determine if user.getFirstName() is NULL or not. In your answer, line 2 would be `SET USER_TABLE_FIRST_NAME = USER_TABLE_FIRST_NAME` in the scenario when user.getFirstName returns null. That is the my understanding of the problem. – Sarath Chandra Aug 14 '15 at 21:29
  • After Andreas has edited his answer, I think his solution is the way to go. It works and is resistent to sql injection attacks. – beosign Aug 14 '15 at 21:57
0

IFNULL doesn't do a null-check of the variable you're providing. Note that you're string-concatenating your query and not calling the IFNULL on the string variable/object as such.

When you provide a string to the IFNULL, itassumes the STRING provided is a column name in the SELECT. Hence, it is looking for a column named as the string you've provided - 'Rakesh', in this case. Hence, the exception.

IFNULL documentation here.

You cannot expect to invoke the IFNULL function evaluation to happen in your business logic layer (where you are concatenating and preparing the SQL query as a string). The IFNULL function gets executed in the database when the query is executed.

In your case, consider moving the IF-NULL logic to your business logic and out of the query. Evaluate the individual component attributes and compose your SQL string accordingly.

String updateQuery = "UPDATE " + USER_TABLE + " SET " ;
if(null != user.getFirstName()){
    updateQuery += " USER_TABLE_FIRST_NAME = '"+ user.getFirstName() + "'," ; 
}
if(null != user.getLastName() ){
    updateQuery += " USER_TABLE_LAST_NAME = '" + user.getLastName() + "'," ;
}
if(null != user.getAboutMe()){
    updateQuery += " USER_TABLE_ABOUT_ME = '" + user.getAboutMe() + "'," ;
}
if(null != user.getCity()){
    updateQuery += " USER_TABLE_CITY = '" + user.getCity() + "'," ;
}
if(null != user.getDateOfBirth() ){
    updateQuery += " USER_TABLE_DOB ='" + user.getDateOfBirth() + "'"; 
}
updateQuery += " WHERE " + USER_TABLE_ID + "='" + user.getUserId() + "'";
Sarath Chandra
  • 1,850
  • 19
  • 40
  • This is not an answer to the question. The answer is that the SQL is bad because he forgot to quote the user name. Also, please don't perpetuate the bad coding style of injecting string directly into SQL without escaping. – Andreas Aug 14 '15 at 21:23