Re: JSP, Servlets & AJAX username validation, Image verification
amitatgroups@gmail.com wrote:
--------------------- JSP ----------------
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://
www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=iso-8859-1" />
<title> - User Login</title>
[code sample snipped]
This example doesn't follow best practices.
It's got scriptlet in the JSP and HTML in the Java source code.
The Java code has hard-coded Strings and public non-final non-static members:
public String DBUrl = "jdbc:mysql://127.0.0.1:3306/databasename";
It uses System.out.println() and System.err.println() in Web code. (Use
logging, not console output.)
It's vulnerable to SQL injection attack because it doesn't use PreparedStatement:
String querySelctId = "select count(*)user from userinfo where UserId='"+userID+"'AND UserPassword='"+userPass+"';";
(Consider a user entry for the userID of
' or 1=1 --
where the single-quotes are very significant.)
The HTML contains tables nested within cells within tables.
(As a side note, one should not include embedded TABs in Usenet source-code
postings.)
The "log" call from the UserLogin servlet's init() method reads:
System.out.println("UserLogin called");
This message is misleading, since the usual interpretation of "calling" a
servlet is to call its service method, which isn't what's happening there.
It'd be more useful for the message to identify /which/ method in the servlet
was called. (And to be a logging call instead of a console output.)
Many variables are redundantly initialized:
String userID=null,userPass=null,currentDate=null;
userID = req.getParameter("userID");
userPass = req.getParameter("userPass");
HttpSession session = req.getSession(true);
It uses sendRedirect() where the JSP error-page mechanism would work better
(if the presentation were coming from a JSP as it should be), or failing that,
at least a RequestDispatcher.forward(), thus preventing the unnecessary
round-trip to the browser and concomitant loss of diagnostic information.
res.sendRedirect("/servlets/ErrorPage.htm");
(And "htm" as the suffix? Aside from the fact that the error page should be a
JSP, what's wrong with the suffix "html"?)
I'm very dubious about the lines:
pool.releaseConnection(con);
rs.close();
stm.close();
Most DB connection pools do not require the code to explicitly know of the
pooled nature of the connections, but just have the connection call its
close() method. The closing of the Connection would close the ResultSet and
the (not Prepared!) Statement. If you do close them explicitly, close the
ResultSet first, then the (Prepared!) Statement, then the connection.
The catch-all Exception catch blocks would be better handled by the error-page
mechanism.
Database logic should have its own layer, as should business logic. Mingling
presentation, logic, data access and navigation all in one is not robust.
--
Lew