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

Implement HTTP Server with persistent connection

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

Problem

I am trying to implement a HTTP Server in Java, I want the server to use a persistent connection per thread for request and response. After some research on Google, this is how my program looks like. It runs fine without any exceptions but I am unable to tell if it is doing what I want it to do, that is connection per thread policy. Can some one please take a look at it and tell me if there any changes to be made or if there are any flaws in my program?

```
public class TinyHttpd4ServerThread extends Thread{
Socket client;

String line = null;
String httpVersion = null;
boolean connectionKeepAlive;
private ArrayList files = new ArrayList();
ArrayList lineArray = new ArrayList();
ArrayList tokens = new ArrayList();
ArrayList requestHeaderLines;
StringTokenizer st = null;
ReentrantLock lock = new ReentrantLock();
TinyHttpd4ServerThread(Socket socket)
{
this.client = socket;
}

public void run()
{
try
{
lock.lock();
executeCommand(client);
}
catch(Exception e)
{
e.printStackTrace();
}
finally
{
lock.unlock();
}

}

private void executeCommand( Socket client ){
try {
try {

String fileName = "index.html";
String token = null;
client.setSoTimeout(30000);
BufferedReader in = new BufferedReader( new InputStreamReader( client.getInputStream() ) );
PrintStream out = new PrintStream( client.getOutputStream() );
System.out.println( "I/O setup done" );
String line = in.readLine(); //Get first line of request to check the request command GET or HEAD
StringTokenizer tokenLine = new StringTokenizer(line);
String reqCommand = tokenLine.nextToken();

Solution

I do not believe this code is doing what you expect... and, some of the more important parts of the code are not included here.

There are a number of standard models that are used in Java to run Network-based servers. Your code does not follow any pattern I am familiar with.

First, some low-level concepts:

Thread

Do not extend Thread class. It is commonly done, but, it is not the right object-model to use. What you have is something that is Runnable, your class is not a Thread, but a Thread runs it. So, you want something to run..., then you want to run that runnable on the Thread. Your entire class should be renamed from public class TinyHttpd4ServerThread extends Thread{... to be:

public class TinyHttpdSocketHandler implements Runnable {


And, then when you create it, you should have the same Socket-based constructor:

TinyHttpdSocketHandler(Socket socket)
{
    this.client = socket;
}


Now, with this runnable, you have to add it to a Thread. You do not include the code that does this for you, but, it should look something like:

Runnable handler = new TinyHttpdSocketHandler(socket);
Thread socketThread = new Thread(handler, "Thread for " + socket.toString());
socketThread.setDaemon(true);
socketThread.start();


Note how I have called setDaemon(true). This allows your application to exit in a sane way.

Encoding

The encoding in the data is determined by the protocol values. The actual headers and stuff in the HTTP protocol are 8-bit ASCII (I believe, check me on that). Your are loading up a default InputStreamReader on your InputStream and this may be messing with the encoding. You should probably keep things as ASCII, and as a Stream. Manually finding the newline and parsing from there...

BufferedInputStream bis = new BufferedInputStream(socket.getInputStream());
BufferedOutputStream bos = new BufferedOutputStream(socket.getOutputStream());


etc.

Game Loop

When designing games, they have what's called a "Game Loop". This is a system where the game iterates through and manages things, even when there is no input, etc. The system works well for network socket handling too.

You want to take the socket, expose the parts you need, then loop on it, and wait for activity. Your code is not doing that. It has no loop.

The request/response nature of the HTTP protocol should be very obvious in the loop. Something like:

public void run() {
    try (
        BufferedInputStream bis = new BufferedInputStream(socket.getInputStream());
        BufferedOutputStream bos = new BufferedOutputStream(socket.getOutputStream());) {

        boolean alive = true;
        while (alive) {
            alive = false;
            try {
                Response response = null;
                try {
                    Request req = parseRequest(bis);
                    response = processRequest(req);
                } catch (BadParseException bpe) {
                    response = newExceptionStatus(HTTP_400_BAD_REQUEST, bpe);
                } catch (BadProcessException bqe) {
                    response = newExceptionStatus(HTTP_500_INTERNAL_ERROR, bqe);
                }
                if (response == null) {
                    response = newExceptionStatus(HTTP_500_INTERNAL_ERROR, bqe);
                }
                returnRespone(response, bos);
                alive = true;                    
            } catch (Exception e) {
                // log this locally, we can't return it to the client....
                alive = false;
            }
        }
    }
}


Conclusion

With your system not having a decent game loop, with the socket being passed around in a funny way, and with the Thread being 'unusual', there are a lot of infrastructure things that should be changed. I recommend you read up on Java Network servers

Code Snippets

public class TinyHttpdSocketHandler implements Runnable {
TinyHttpdSocketHandler(Socket socket)
{
    this.client = socket;
}
Runnable handler = new TinyHttpdSocketHandler(socket);
Thread socketThread = new Thread(handler, "Thread for " + socket.toString());
socketThread.setDaemon(true);
socketThread.start();
BufferedInputStream bis = new BufferedInputStream(socket.getInputStream());
BufferedOutputStream bos = new BufferedOutputStream(socket.getOutputStream());
public void run() {
    try (
        BufferedInputStream bis = new BufferedInputStream(socket.getInputStream());
        BufferedOutputStream bos = new BufferedOutputStream(socket.getOutputStream());) {

        boolean alive = true;
        while (alive) {
            alive = false;
            try {
                Response response = null;
                try {
                    Request req = parseRequest(bis);
                    response = processRequest(req);
                } catch (BadParseException bpe) {
                    response = newExceptionStatus(HTTP_400_BAD_REQUEST, bpe);
                } catch (BadProcessException bqe) {
                    response = newExceptionStatus(HTTP_500_INTERNAL_ERROR, bqe);
                }
                if (response == null) {
                    response = newExceptionStatus(HTTP_500_INTERNAL_ERROR, bqe);
                }
                returnRespone(response, bos);
                alive = true;                    
            } catch (Exception e) {
                // log this locally, we can't return it to the client....
                alive = false;
            }
        }
    }
}

Context

StackExchange Code Review Q#49746, answer score: 7

Revisions (0)

No revisions yet.