Open source Pastebin tool made in JSP and servlet

Question

I'm a student and still studying. I just made my own Pastebin tool to generate source code. I've also contributed this project as an open source. You could see this project from here.

If you wish to see the demo, you could visit here.

Now after this, I've made this project using JSP and servlet. Actually, when the user presses "Generate source codes", the URL looks something like this:

http..//viewCode.jsp?id=10428

Please contribute your opinions here. Here is my source code to generate the URL like this:

<%@ page language="java" contentType="text/html; charset=ISO-8859-1"
    pageEncoding="ISO-8859-1"%>

<%@ page language="java" import="java.sql.*" %>
<%@ page language="java" import="java.util.*" %>
<%@ page language="java" import="org.apache.commons.lang3.StringEscapeUtils" %>
<%@ page language="java" import="org.apache.*" %>

<%

    String submit_code1=request.getParameter("paste_code");
    String title_code=request.getParameter("title_code");

    Random random = new Random(System.currentTimeMillis());
    int gen_key=10000 + random.nextInt(20000);

    String submit_code2 = StringEscapeUtils.escapeHtml4(submit_code1);


    try{
    Class.forName("com.mysql.jdbc.Driver");
    Connection con=DriverManager.getConnection("jdbc:mysql://localhost:3306/db_name", "root", "1234");
    PreparedStatement pstmt=con.prepareStatement("insert into paste_tool(gen_key, title, source_codes) VALUES(?, ?, ?)");

    pstmt.setInt(1, gen_key);
    pstmt.setString(2, title_code);
    pstmt.setString(3, submit_code2);

    pstmt.executeUpdate();

    pstmt.close();

    response.sendRedirect("viewCode.jsp?id="+gen_key);

}
catch(Exception e){
    e.printStackTrace();
}
%>

Answer 1

On the upside of your code, you kept it as simple as possible. But there are a few flaws - and the upside doesn't come without consequences.

What's also good is that you already use PreparedStatement.

Security Risk

Hard-coding the username and password is not a good idea for another reason. A misconfigured webserver could easily expose these critical credentials by accident.

Layered Architecture

We say that often, an architecture consists of 3+1 layers: UI, Business Logic / App, Persistence, plus Main.

In your case this is all mixed up. The upside is that in your case it keeps things seemingly simple. But this is a trap.

Someone needs to change the source code of the application in order to connect with a different password, different username, or to use a different SQL RDBMS than MySQL, or to use a different type of DBMS altogether.

ID Collisions likely

The way how you get your ID means that ID collisions are quite likely. There are better ways to generate a unique ID. In case of MySQL, and most other RDBMS for that matter, you can actually let the DBMS generate an ID for you. In MySQL, the id column would need to be defined like this: id INT PRIMARY KEY AUTO_INCREMENT.

Usually, we say that using auto-incremental IDs is a security issue. However, if pastes are public anyway, that's a different topic. The pastes are not access-protected anyway. So incremental IDs are fine in this case.

Otherwise, universally unique ids or hashes with unlikely collisions are far better user ids. One might still want to go for id INT PRIMARY KEY AUTO_INCREMENT hidden in the DBMS for relations and use a uuid VARCHAR(...) UNIQUE for the id visible outside the DBMS.

Error handling

You want to close your pstmt even in case of error, and you want to close your con in any case.

You can use Java 7 try-with-resources for that:

try (final Connection con = DriverManager.getConnection(dbUrl, dbUsername, dbPassword)) {
    try (final PreparedStatement pstmt = con.prepareStatement("INSERT INTO paste_tool(gen_key, title, source_codes) VALUES(?, ?, ?)")) {
        // ...
    }
}

This will close your PreparedStatement and your Connection automatically, regardless of whether they were successful or threw exceptions.

Code Conventions

In Java, we only use _ in names in these rare cases:

• The name implements an API defined elsewhere and there the name already contains _.
• The name is a compile time constant, as in public static final short MAX = 32767;
• The _ replaces a comma in natural language, as in givenConnectedChatClient_whenSendingMessage_thenReceivesMessage().

In your case, you should rename things:

• gen_key -> genKey
• submit_code1 -> submitCode1
• title_code -> titleCode

Why is it named submit_code1 anyway? Is there also non-submitted code? Is there also a submit_code0? A submit_code2? And why is it named submit_code1, when in the form it is named paste_code?

I think it that submitCode1 should actually be named pasteCode. And submitCode2 should be renamed to htmlEscapedPasteCode - or removed, see next point.

When to replace the special characters

You replace the special characters in the paste code before storing the paste in the DBMS. While this gives you a little bit of performance - you have to replace only once, upon storage, not many times upon each retrieval, I think it's not the best way how to handle it.

Consider you want to extend this with different interfaces. Maybe you want to implement raw retrieval, or PDF export, or the facility that a user can fetch all his pastes as one big zip file? Then you'd have to reverse the replacement of the special characters.

From an architecture perspective, the special characters are a presentation topic - it's because you use HTML as presentation. Therefore it should be in the presentation layer, not in the persistence layer. If you then add more presentation modules, they will not have to reverse presentations of other modules that are otherwise unrelated to them. It would look awkward in a raw module or PDF module to first have to reverse the HTML special characters from what they get from the database.

So, I'd put the StringEscapeUtils.escapeHtml4() call in viewCode.jsp instead.

Error Handling 2

What happens in case of an ID collision (gen_key already used)?

Error Handling 3

What happens in case the form is submitted empty?

ISO-8859-1 vs. UTF-8

I'd really use UTF-8. Always. Everywhere. Unless there's a really good reason not to. And I don't think that here there is.

ASP-style JSPs vs. XML-style JSPs

I'd rather go for XML-style JSPs than ASP-style JSPs. For many it may just be a matter of taste. But to me there's the advantage that I can XSLT process the source of the JSP.