Turning a Java console app into a server I'm not familiar with Java at all, so I'd like to get some feedback on this piece of code.  

I made it by looking at a few Hello World-esque Java server examples, so it may have drawbacks I'm not aware of.

My task was turning an existing Java console app into a single-threaded server listening on some port. Each connection should “launch” what previously was the console app, where actual “arguments” are passed by the client. Anything that was written by console app to stdout, should go into process' stdout; but what was previously written to stderrm needs to be redirected to the client. Finally, when the processing has finished, if there was no error, the client should receive "OK" and get disconnected. (And the next client should be processed.)

Here's the original and my version side by side:

// Change: we want to run this as a server instead

// public static void main(String [] args) {
//     (new Main(args)).execute();
//     System.exit(0);
// }

public static void main(String args[]) throws Exception {
    int port = Integer.parseInt(args[0]);
    ServerSocket welcomeSocket = new ServerSocket(port);

    System.out.format("Listening on %d\n", port);
    PrintStream defaultStderr = System.err;

    while (true) {
        // Establish a connection
        Socket connectionSocket = welcomeSocket.accept();
        BufferedReader input = new BufferedReader(new InputStreamReader(connectionSocket.getInputStream()));
        DataOutputStream output = new DataOutputStream(connectionSocket.getOutputStream());

        // Hook System.err
        System.setErr(new PrintStream(output));

        try {
            // Run Batik as if we ran console app, but without the launch overhead
            (new Main(input.readLine().split(" "))).execute();
        } catch (Throwable t) {
            // Normally, we shouldn't get here (as Batik should not crash).
            // If we *do* get here, print to stderr, which we redirect to output stream anyway.
            t.printStackTrace();
        }

        // Let the client know how it went
        if (output.size() == 0) {
            output.writeBytes("OK");
        }

        output.close();

        // Restore stderr
        System.setErr(defaultStderr);
    }
}


What can I make better? I want to clarify I do not want to change any other method in the app to keep it easy to merge upstream changes.
General:

Commandline handling in any application is a PITA.... My recommendation is that you change both the client side, and server side, to put each argument on a different line so that you don't have to 'split' it on the Server. This would be relatively simple to do.

OOP:

This is a 'for future improvement' thing. It is a common pattern in Java for servers to handle different 'clients' in different threads. It is really easy if you have the basic infrastructure in place. Simply creating a class called "ClientManager" or something is a start, and having the following:

class ClientManager {
    private final Socket socket;
    ClientManager(Socket socket) {
        this.socket = socket;
    }

    public void processSocket() {

        // do all your things for the socket in here.

    }
}


With this simple class, your Java server becomes:

while (true) {
    // Establish a connection
    ClientManager client = new ClientManager(welcomeSocket.accept());
    client.processSocket();
}


This has some advantages. The first is that it becomes easier to separate Server problems from client problems, and the second is that the client.processSocket() can be easily moved off to any thread we want to use.

Right now, using a single thread is probably fine, but, the Error handling would be much better with this distinction. Consider the following:

try {
    while (true) {
        // Establish a connection
        ClientManager client = new ClientManager(welcomeSocket.accept());
        try {
            client.processSocket();
        } catch (Exception e) {
            // this is unusual to catch `Exception` but it is done intentionally!
            // Any exception in the client code is specific to **that** client.
            // Clients should be better behaved and manage their own exceptions
            // but no client is perfect.
            // Log the issue, and wait for the next customer.
            e.printStackTrace();
        }
    }
} catch (IOException serverexception) {
    // this is an exception in the **server** code, and thus the server dies:
    serverexception.printStackTrace();
}


OK, so we have a Server that is well isolated from the client code it runs.

The client handler

The client code is handled by a ClientManager class. What happens in there.... this is a copy/paste of the relevant code:

    BufferedReader input = new BufferedReader(new InputStreamReader(connectionSocket.getInputStream()));
    DataOutputStream output = new DataOutputStream(connectionSocket.getOutputStream());

    // Hook System.err
    System.setErr(new PrintStream(output));

    try {
        // Run Batik as if we ran console app, but without the launch overhead
        (new Main(input.readLine().split(" "))).execute();
    } catch (Throwable t) {
        // Normally, we shouldn't get here (as Batik should not crash).
        // If we *do* get here, print to stderr, which we redirect to output stream anyway.
        t.printStackTrace();
    }

    // Let the client know how it went
    if (output.size() == 0) {
        output.writeBytes("OK");
    }

    output.close();

    // Restore stderr
    System.setErr(defaultStderr);


Now, this client/server setup is simple. It is a call-response type only, with a 'success' indicated by 'OK'. Anything else is a problem.

The protocol can be set up that the client pushes the arguments, and then closes that stream.... and then waits for results. Closing the stream has the advantage that we can read the arguments nicely (wait for end-of-stream):

List&lt;String&gt; arguments = new ArrayList&lt;&gt;();
String line = null;
while ((line = input.readLine()) != null) {
    arguments.add(line);
}


Let's introduce the Java7 try-with-resources system. This has a number of advantages, but, in this case it makes sure that, if there's a problem, the client side of the process knows immediately instead of hanging around waiting for a socket to die (get garbage collected).

Note, never catch Throwable.... it will catch all sorts of things you don't want to, like ThreadKillError, etc.

First up, we manage the input/output streams:

PrintStream defaultStderr = System.err;
try (  BufferedReader input = new BufferedReader(
           new InputStreamReader(socket.getInputStream()));
       DataOutputStream output = new DataOutputStream(socket.getOutputStream());
       PrintStream stderr = new PrintStream(output);
) {
    // at this point, the input/output is guaranteed to close
    List&lt;String&gt; arguments = new ArrayList&lt;&gt;();
    String line = null;
    while ((line = input.readLine()) != null) {
        arguments.add(line);
    }       

    // Hook System.err
    System.setErr(stderr);

    try {
        // Run Batik as if we ran console app, but without the launch overhead
        Main batik - new Main(arguments.toArray(new String[arguments.size]));
        batik.execute();
    } catch (Exception t) {
        // Normally, we shouldn't get here (as Batik should not crash).
        // If we *do* get here, print to stderr, which we redirect to output stream anyway.
        t.printStackTrace();
    }

    // Let the client know how it went
    if (output.size() == 0) {
        output.writeBytes("OK");
    }
    output.flush();
} finally {
    // note, we close the socket explicitly when we are done.
    // even if the input and output are closed, you should still close the socket.
    this.socket.close();

    // Restore stderr
    System.setErr(defaultStderr);
} 


note that this handling of System.err is a problem.... you will never be able to multi-thread until you fix this.

All the best, I hope this makes sense to you.