2
\$\begingroup\$

I have had to do a crash course in JSP, Servlets, and JDBC. I decided to attempt a Login System. I'm unsure if my use of try/catch is appropriate and if I am closing database connections correctly. There is one major flaw in the program which is if a user registering enters an ID that is not unique, then it causes an exception. I'm unsure how to solve this. I realize this is a lot of code to pore through, but any suggestions would be much appreciated.

Index.jsp

<%@ page language="java" contentType="text/html; charset=ISO-8859-1"
    pageEncoding="ISO-8859-1"%>
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
<title>Insert title here</title>
</head>
<body>
<h3> Welcome to the Login Screen</h3>
<br>
<form action = 'login' method = "post">
<input type = "text" name = "userId"/>
<br>
<input type = "password" name = "password"/>
<br>
<input type = "submit"/>
</form>
<br>
<a href="registration.jsp">I dont have an account</a>
</body>
</html>

Profile.jsp

<%@ page language="java" contentType="text/html; charset=ISO-8859-1"
    pageEncoding="ISO-8859-1"%>
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
<title>Profile</title>
</head>
<body>
<h3>Welcome to the profile page</h3>
<br>
<jsp:useBean id="user" class = "com.calgar.service.User" scope = "session"></jsp:useBean>
Welcome <jsp:getProperty property="firstName" name="user"/> <jsp:getProperty property="lastName" name="user"/> 
<br>Your User ID : <jsp:getProperty property="userId" name="user"/>
<br>Your Password is :<jsp:getProperty property="password" name="user"/>
</body>
</html>

Registration.jsp

<%@ page language="java" contentType="text/html; charset=ISO-8859-1"
    pageEncoding="ISO-8859-1"%>
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
<title>Enter Your details here</title>
</head>
<body>
<h3>Enter User ID</h3>
<br>
<form action = "RegistrationServlet" method = "post">
Enter User ID : <input type = "text"  name ="newUserId"/>
<br>Enter your Password : <input type = "password" name ="newUserPassword"/>
<br>Enter your First Name :<input type = "text" name = "newUserFirstName"/>
<br>Enter your Last Name : <input type = "text" name ="newUserLastName"/>
<br><input type = "submit"/>
<br>
<%String name =(String)request.getAttribute("GenericSQLError"); %>
<%=name %>
</form>
</body>
</html>

Login Servlet

package com.calgar.servlet;

import java.io.IOException;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;

import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import com.calgar.service.LoginService;
import com.calgar.service.User;

/**
 * Servlet implementation class LoginServlet
 */
@WebServlet("/login")
public class LoginServlet extends HttpServlet {
    private static final long serialVersionUID = 1L;

    /**
     * @see HttpServlet#HttpServlet()
     */
    public LoginServlet() {
        super();
        // TODO Auto-generated constructor stub
    }

    /**
     * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response)
     */
    protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        String userId,password;
        userId = request.getParameter("userId");
        password = request.getParameter("password");

        Connection connection = null;
        PreparedStatement preparedStatement = null;
        ResultSet resultSet = null;
        boolean authenticated = false;

        LoginService loginService = new LoginService();

        try {
            connection=loginService.getConnection(connection);
            resultSet = loginService.getUserLoginCredentials(preparedStatement, connection, resultSet);
            authenticated = loginService.authenticate(userId,password,resultSet);

            if(authenticated)
            {
                User user = new User();
                user = loginService.getUserDetails(userId, preparedStatement, connection, resultSet);
                request.getSession().setAttribute("user", user);
                response.sendRedirect("profile.jsp");
            }
            else
            {   
                loginService.closeDatabaseConnections(preparedStatement, connection, resultSet);
                response.sendRedirect("index.jsp");
            }

            } catch (ClassNotFoundException | SQLException e) {
                e.printStackTrace();
            }
    }

}

Registration Servlet

package com.calgar.servlet;

import java.io.IOException;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;

import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import com.calgar.service.LoginService;
import com.calgar.service.User;

/**
 * Servlet implementation class RegistrationServlet
 */
@WebServlet("/RegistrationServlet")
public class RegistrationServlet extends HttpServlet {
    private static final long serialVersionUID = 1L;

    /**
     * @see HttpServlet#HttpServlet()
     */
    public RegistrationServlet() {
        super();
        // TODO Auto-generated constructor stub
    }

    /**
     * @see HttpServlet#doGet(HttpServletRequest request, HttpServletResponse response)
     */
    protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        Connection connection = null;
        PreparedStatement  preparedStatement = null;
        String newUserId, newUserPassword, newUserFirstName, newUserLastName;
        ResultSet resultSet = null;

        newUserId = request.getParameter("newUserId");
        newUserPassword = request.getParameter("newUserPassword");
        newUserFirstName= request.getParameter("newUserFirstName");
        newUserLastName = request.getParameter("newUserLastName");
        System.out.println(newUserId);


        try {

            LoginService loginService = new LoginService();
            connection = loginService.getConnection(connection);
            loginService.addNewUser(preparedStatement, connection, newUserId, newUserPassword, newUserFirstName, newUserLastName);
            loginService.closeDatabaseConnections(preparedStatement, connection, resultSet);

            User user = new User();
            user.setUserId(newUserId);
            user.setPassword(newUserPassword);
            user.setFirstName(newUserFirstName);
            user.setLastName(newUserLastName);

            request.getSession().setAttribute("user", user);
            response.sendRedirect("profile.jsp");

        } catch (ClassNotFoundException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

        catch (SQLException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        finally
        {
            if(connection != null)
                try{connection.close();}
                catch(SQLException ignore){}

            if(preparedStatement != null)
                try{preparedStatement.close();}
                catch(SQLException ignore){}

            if(resultSet != null)
                try{resultSet.close();}
                catch(SQLException ignore){}

        }

    }



}

LoginService

package com.calgar.service;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;

public class LoginService {

    public Connection getConnection(Connection connection) throws ClassNotFoundException, SQLException
    {
        System.out.println("Connecting to database USERS");
        Class.forName("com.mysql.jdbc.Driver");
        connection = DriverManager.getConnection("jdbc:mysql://localhost:3306/login","calgar","password");
        System.out.println("Connection successful");
        return connection;
    }

    public ResultSet getUserLoginCredentials(PreparedStatement ps,Connection connection, ResultSet rs) throws SQLException
    {
        ps =connection.prepareStatement("SELECT userId, password FROM users");
        rs = ps.executeQuery();
        return rs;
    }

    public boolean authenticate(String UserId, String password, ResultSet rs) throws SQLException
    {
        String userIdResultSet,passwordResultSet;
        //Boolean authenticated = false;
        while(rs.next())
        {
            userIdResultSet = rs.getString(1);
            passwordResultSet = rs.getString(2);
             if(userIdResultSet.equals(UserId))
             {
                if(passwordResultSet.equals(password))
                        {
                         return true;
                        }
             }
        }
        return false;

    }

    public User getUserDetails(String userId, PreparedStatement ps,Connection connection, ResultSet rs) throws SQLException
    {
        User user = new User();

        ps = connection.prepareStatement("SELECT userId, password, userFirstName, userLastName FROM users WHERE  userId = ?" );
        ps.setString(1, userId);
        rs = ps.executeQuery();

        while(rs.next())
        {

            user.setUserId(rs.getString(1));
            user.setPassword(rs.getString(2));
            user.setFirstName(rs.getString(3));
            user.setLastName(rs.getString(4));
        }

        return user;
    }

    public void addNewUser(PreparedStatement ps, Connection connection, String newUserId, String newUserPassword,String newUserFirstName,String newUserLastname) throws SQLException
    {
        if (newUserId != null && "".equals(newUserId.trim())){
                newUserId = null;
        }
            if ((newUserPassword != null) && (newUserPassword.trim().equals(""))){
                newUserPassword = null;
            }
        ps = connection.prepareStatement("INSERT INTO users (userId ,password , userFirstName, userLastName)VALUES(?,?,?,?)");
        ps.setString(1, newUserId);
        ps.setString(2, newUserPassword);
        ps.setString(3, newUserFirstName);
        ps.setString(4, newUserLastname);

        ps.executeUpdate();

    }

    public void closeDatabaseConnections(PreparedStatement ps,Connection connection, ResultSet rs) throws SQLException
    {
        if(ps != null)
        {
        ps.close();
        }
        if(connection!=null)
        {
        connection.close();
        }
        if(rs != null)
        {
        rs.close();
        }

    }

}

User

package com.calgar.service;

public class User {

    private String userId;
    private String password;
    private String firstName;
    private String lastName;

    public String getUserId() {
        return userId;
    }
    public void setUserId(String userId) {
        this.userId = userId;
    }
    public String getPassword() {
        return password;
    }
    public void setPassword(String password) {
        this.password = password;
    }
    public String getFirstName() {
        return firstName;
    }
    public void setFirstName(String firstName) {
        this.firstName = firstName;
    }
    public String getLastName() {
        return lastName;
    }
    public void setLastName(String lastName) {
        this.lastName = lastName;
    }
}
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Most of the functions in LoginService take unnecessary parameters. For example, getConnection() takes a connection parameter, but never uses it. It just reassigns and returns it. It could be written like this instead:

public Connection getConnection() throws ClassNotFoundException, SQLException {
    System.out.println("Connecting to database USERS");
    Class.forName("com.mysql.jdbc.Driver");
    Connection connection = DriverManager.getConnection("jdbc:mysql://localhost:3306/login","calgar","password");
    System.out.println("Connection successful");
    return connection;
}

Same thing with ps and rs in getUserLoginCredentials(), etc.

The database resources are not being closed properly. Make sure you wrap the use of each resource in a try and close it in the finally. This goes for all database resources, including Connection, PreparedStatement and ResultSet.

Connection connection = getConnection();
try {
    // use connection
}
finally {
    connection.close()
}

If you are using Java 7, you should check out the new try-with-resources statement. The above could be written like this:

try (Connection connection = getConnection()) {
    // use connection
} // connection will automatically be closed here

Rather than declaring all variables at the top of a function, consider moving the variable declarations closer to where they are used. For example, rather than this:

String newUserId, newUserPassword, newUserFirstName, newUserLastName;
...

newUserId = request.getParameter("newUserId");
newUserPassword = request.getParameter("newUserPassword");
newUserFirstName= request.getParameter("newUserFirstName");
newUserLastName = request.getParameter("newUserLastName");

Do this:

String newUserId = request.getParameter("newUserId");
String newUserPassword = request.getParameter("newUserPassword");
String newUserFirstName= request.getParameter("newUserFirstName");
String newUserLastName = request.getParameter("newUserLastName");

This will eliminate several lines of code and make it a little easier to read.

You could also get rid of the constructors in the servlet classes. They will be generated by the compiler automatically.

Improve this answer
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.