HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

User registration Servlet

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
userregistrationservlet

Problem

I have a servlet that processes user registration to a website. It takes inputs from an HTML form like username, password, email, etc.

MySQL Table:

CREATE TABLE IF NOT EXISTS user(
    user_id VARCHAR(255),
    user_password VARCHAR(255) NOT NULL,
    user_last_name VARCHAR(255),
    user_first_name VARCHAR(255),
    user_email VARCHAR(255) UNIQUE NOT NULL,
    user_type TINYINT UNSIGNED NOT NULL, /* VALUES: 0 - Guest, 1 - Admin, 2 - User */   
    PRIMARY KEY(user_id)
);


Servlet:

```
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
String userId = request.getParameter("username");
String userFirstName = request.getParameter("firstname");
String userLastName = request.getParameter("lastname");
String userEmail1 = request.getParameter("email1");
String userEmail2 = request.getParameter("email2");
String userPassword1 = request.getParameter("pass1");
String userPassword2 = request.getParameter("pass2");
String captchaAnswer = request.getParameter("answer");

try {
// simple captcha
HttpSession session = request.getSession(true);
Captcha captcha = (Captcha) session.getAttribute(Captcha.NAME);
request.setCharacterEncoding("UTF-8");

boolean isCaptchaCorrect = captcha.isCorrect(captchaAnswer);
session.setAttribute("isCaptchaCorrect", isCaptchaCorrect);

session.setAttribute("userId", userId);
session.setAttribute("userFirstName", userFirstName);
session.setAttribute("userLastName", userLastName);
session.setAttribute("userEmail1", userEmail1);
session.setAttribute("userEmail2", userEmail2);

if(isCaptchaCorrect) {
// put database entries into a String[]
DatabaseManipulator dm = new DatabaseManipulator();
String[] usernameArray = dm.dbEntriesToArray("user_id");
String[] emailArray = dm.dbEntriesToArray("user_emai

Solution

What's even worse is all this:

try {
     throw new SomeException();
} catch (SomeException uaee) {
     response.sendRedirect("some-result.jsp");
}


It would be better to just do

response.sendRedirect("some-result.jsp");


Directly. There is really no need to throw an exception just to catch the same exception on the next line.

if(hasDuplicateUsername) {
    response.sendRedirect("register-result.jsp");
} else if(hasDuplicateEmail) {
    response.sendRedirect("register-result.jsp");
} else if(!isEmailMatch) {
    response.sendRedirect("register-result.jsp");
} else if(!isPasswordMatch) {
    response.sendRedirect("register-result.jsp");
}


As for the NullPointerException, I assume that it is one of these that are null:

String userId = request.getParameter("username");
String userFirstName = request.getParameter("firstname");
String userLastName = request.getParameter("lastname");
String userEmail1 = request.getParameter("email1");
String userEmail2 = request.getParameter("email2");
String userPassword1 = request.getParameter("pass1");
String userPassword2 = request.getParameter("pass2");
String captchaAnswer = request.getParameter("answer");


The fix for this is easy, check if they are null before using them:

if (userId == null || userFirstName == null || userLastName == null ||
  userEmail1 == null || ...) {
    response.sendRedirect("index.jsp");
    return;
}

Code Snippets

try {
     throw new SomeException();
} catch (SomeException uaee) {
     response.sendRedirect("some-result.jsp");
}
response.sendRedirect("some-result.jsp");
if(hasDuplicateUsername) {
    response.sendRedirect("register-result.jsp");
} else if(hasDuplicateEmail) {
    response.sendRedirect("register-result.jsp");
} else if(!isEmailMatch) {
    response.sendRedirect("register-result.jsp");
} else if(!isPasswordMatch) {
    response.sendRedirect("register-result.jsp");
}
String userId = request.getParameter("username");
String userFirstName = request.getParameter("firstname");
String userLastName = request.getParameter("lastname");
String userEmail1 = request.getParameter("email1");
String userEmail2 = request.getParameter("email2");
String userPassword1 = request.getParameter("pass1");
String userPassword2 = request.getParameter("pass2");
String captchaAnswer = request.getParameter("answer");
if (userId == null || userFirstName == null || userLastName == null ||
  userEmail1 == null || ...) {
    response.sendRedirect("index.jsp");
    return;
}

Context

StackExchange Code Review Q#67205, answer score: 8

Revisions (0)

No revisions yet.