Loop for downloading file from the server (in client-server application)

Question

In my client server application, client sends some commands and the server gives the results back.

Now my special concern is about the looping for downloading file from the server class, that is downloaded by using the GET filename command.

My question: In my ClientSide there is a while loop for read/downlaod the named file. So how does the loop make sure that the client uses the size sent from the server, to control the receving of file contents? Am I doing that what I am supposed to do in this looping?

This is the complete code:

ClientSide:

package clientside;

import java.io.*;
import java.net.*;
import java.util.Scanner;

public class ClientSide {

    private static Socket socket;
    private static PrintWriter outputToServer;
    private static BufferedReader inputFromServer;
    private static InputStream is;
    private static FileOutputStream fos;
    private static final int PORT = 8000;
    private static final String SERVER = "85.197.159.45";
    boolean Connected;
    DataInputStream serverInput;

    public static void main(String[] args) throws InterruptedException {
        String server = "localhost";
        int port = PORT;

        if (args.length >= 1) {
            server = args[0];
        }
        if (args.length >= 2) {
            port = Integer.parseInt(args[1]);
        }

        new ClientSide(server, port);
    }

    public ClientSide(String server, int port) {

        try {
            socket = new Socket(server, port);
            serverInput = new DataInputStream(socket.getInputStream());
            outputToServer = new PrintWriter(socket.getOutputStream(), true);
            inputFromServer = new BufferedReader(new InputStreamReader(socket.getInputStream()));
            System.out.println("Client is connected! ");
            Connected = true;
            String line = null;

            Scanner sc = new Scanner(System.in);
            System.out.print("Type command: ");

            while (sc.hasNextLine()) {
                String request = sc.nextLine();

                if (request.startsWith("exit")) {
                    outputToServer.println(request);
                    System.out.println("Application exited!");

                    break;
                } else if (request.startsWith("pwd")) {
                    outputToServer.println(request);
                    outputToServer.flush();
                } else if (request.startsWith("list")) {
                    outputToServer.println(request);
                    outputToServer.flush();
                } else if (request.startsWith("GET")) {
                    System.out.print("\r\n");
                    outputToServer.println(request);
                    outputToServer.flush();
                }
                while (Connected) {
                    line = inputFromServer.readLine();
                    System.out.println(line);
                    if (line.isEmpty()) {
                        Connected = false;
                        if (inputFromServer.ready()) {
                            System.out.println(inputFromServer.readLine());
                        }
                    }
                    if (line.startsWith("Status 400")) {
                        while (!(line = inputFromServer.readLine()).isEmpty()) {
                            System.out.println(line);
                        }
                        break;
                    }
                    if (request.startsWith("GET")) {
                        File file = new File(request.substring(4));
                        is = socket.getInputStream();
                        fos = new FileOutputStream(file);

                        byte[] buffer = new byte[socket.getReceiveBufferSize()];
                        serverInput = new DataInputStream(socket.getInputStream());
                        //int bytesReceived = 0;
                        byte[] inputByte = new byte[4000];

                        int length;

/*
The following while loop is to be reviewed
*/
                        while ((length = serverInput.read(inputByte, 0,  inputByte.length)) > 0) {
                            fos.write(inputByte, 0, length);

                        }


                        request = "";
                        fos.close();
                        is.close();
                    }
                }
                System.out.print("\nType command: ");
                Connected = true;
            }
            outputToServer.close();
            inputFromServer.close();
            socket.close();
        } catch (IOException e) {
            System.err.println(e);
        }

    }
}

ServerSide:

package serverside;

import java.io.*;
import java.net.*;
import java.util.Arrays;

public class ServerSide {

    private BufferedReader inputFromClient;
    private PrintWriter outputToClient;
    private FileInputStream fis;
    private OutputStream os;
    private static final int PORT = 8000;
    private ServerSocket serverSocket;
    private Socket socket;

    public static void main(String[] args) {
        int port = PORT;
        if (args.length == 1) {
            port = Integer.parseInt(args[0]);
        }
        new ServerSide(port);
    }

    private boolean fileExists(File[] files, String filename) {
        boolean exists = false;
        for (File file : files) {
            if (filename.equals(file.getName())) {
                exists = true;
            }
        }
        return exists;
    }

    public ServerSide(int port) {
        // create a server socket
        try {
            serverSocket = new ServerSocket(port);
        } catch (IOException ex) {
            System.out.println("Error in server socket creation.");
            System.exit(1);
        }

        while (true) {
            try {

                socket = serverSocket.accept();

                outputToClient = new PrintWriter(socket.getOutputStream());
                inputFromClient = new BufferedReader(new InputStreamReader(socket.getInputStream()));

                while (true) {

                    String request = inputFromClient.readLine();

                    if (!request.startsWith("exit") && !request.startsWith("pwd") && !request.startsWith("list") && !request.startsWith("GET")) {
                        outputToClient.println("Wrong request\r\n"
                                + "\r\n");
                    } else if (request.startsWith("exit")) {
                        break;
                    } else if (request.startsWith("pwd")) {
                        File file = new File(System.getProperty("user.dir"));
                        outputToClient.print("Status OK\r\n"
                                + "Lines 1\r\n"
                                + "\r\n"
                                + "Working dir: " + file.getName() + "\r\n");
                    } else if (request.startsWith("list")) {
                        File file = new File(System.getProperty("user.dir"));
                        File[] files = file.listFiles();
                        outputToClient.print("Status OK\r\n"
                                + "Files " + files.length + "\r\n"
                                + "\r\n"
                                + Arrays.toString(files).substring(1, Arrays.toString(files).length() - 1) + "\r\n");
                    } else if (request.startsWith("GET")) {
                        String filename = request.substring(4);
                        File file = new File(System.getProperty("user.dir"));
                        File[] files = file.listFiles();

                        if (fileExists(files, filename)) {
                            file = new File(filename);
                            int fileSize = (int) file.length();

                            outputToClient.printf("Status OK\r\nSize %d Bytes\r\n\r\nFile %s Download was successfully\r\n",
                                    fileSize, filename);
                            outputToClient.flush();
                             //private FileInputStream fis;
                            try (FileInputStream fis = new FileInputStream(file)) {
                                OutputStream  os = socket.getOutputStream();
                                byte[] buffer = new byte[(1 << 7) - 1];
                                int bytesRead = 0;



                                while ((bytesRead = fis.read(buffer)) != -1) {
                                    os.write(buffer, 0, bytesRead);
                                }

                            }



                        } else {
                            outputToClient.print("Status 400\r\n"
                                    + "File " + filename + " not found\r\n"
                                    + "\r\n");
                            outputToClient.flush();
                        }
                    }
                    outputToClient.flush();
                }
            } catch (IOException e) {
                System.err.println(e);
            }
        }
    }
}

Answer 1

The loop that you asked for specifically and marked in your post with a comment, seems fine: it reads everything from the input stream and writes everything to the output stream. Nothing really suspicious there to me.

But you have a couple of much bigger problems in this code. A couple of pointers to get you started:

1. Review all your static variables. Take a hard look and check if they really need to be static (hint: most of them don't)

2. Try to make the content of all the try blocks smaller. Don't cover large blocks of code with a single catch. When many things can go wrong at many points, it's impossible to know the failure point, and it's impossible to recover gracefully. Try to handle each failure point individually.

3. Reorganize the code to not exit from anywhere other than the main method. Code with exit statements in constructors or non-toplevel methods is a very bad sign.

4. Avoid unnecessary operations. For example, the file exists method will iterate over all items, even after it already found a match. A better way is to return true immediately once you found a match. If the loop reaches the end, return false. No need for a boolean state variable.

5. The convention is to use camelCase for variable names, which the "Connected" variable violates.