Sending a file over a socket with AES encryption

I'm not sure what's the best way to deal with streams that need to be closed whenever I'm done using them? Usually the need to also catch the exception inside the finally statement is very, very annoying. Am I doing this right?

```
public class FileSender {

public static final String KEY = "00000000000000000000000000000000";
public static final String ALGORITHM = "AES";
public static final String FILE = "C:\\Users\\Andres\\Desktop\\file.txt";
public static final String HASH_ALG = "MD5";

private String filePath;
private String serverMachine;
private int serverPort;

public FileSender(String filePath, String serverMachine, int serverPort){
this.filePath = filePath;
this.serverMachine = serverMachine;
this.serverPort = serverPort;
}

public void sendFile(){
CipherOutputStream cipherOut = null;
Socket client = null;
String checkSum;
try {
System.out.println("Running client ...");
client = connectToServer();
checkSum = getHashChecksum();
cipherOut = initCipherStream(client);
transferData(cipherOut, checkSum);
} catch (Exception e) {
System.out.println(e.getMessage());
} finally {

}
}

private String getHashChecksum() {
FileInputStream fis = null;
try {
MessageDigest md = MessageDigest.getInstance(HASH_ALG);
fis = new FileInputStream(this.filePath);
byte[] byteArray = new byte[1024];
int bytesCount = 0;
while ((bytesCount = fis.read(byteArray)) >= 0)
md.update(byteArray, 0, bytesCount);
byte[] hash = md.digest();
fis.close();
return String.format("%032x",new BigInteger(1, hash));
} catch (Exception e) {
e.printStackTrace();
throw new RuntimeException("Error in checksum");
} finally {
try {

try-with-resources

FileInputStream fis = null;
        try {
            MessageDigest md = MessageDigest.getInstance(HASH_ALG);
            fis = new FileInputStream(this.filePath);
            byte[] byteArray = new byte[1024];
            int bytesCount = 0;
            while ((bytesCount = fis.read(byteArray)) >= 0)
                md.update(byteArray, 0, bytesCount);
            byte[] hash = md.digest();
            fis.close();
            return String.format("%032x",new BigInteger(1, hash));
        } catch (Exception e) {
            e.printStackTrace();
            throw new RuntimeException("Error in checksum");
        } finally {
            try {
                fis.close();
            } catch (IOException e) {
                throw new RuntimeException("Error in checksum. Provided file can't be closed.");
            }
        }

In a normal run, you try to close the stream twice. You do not need to call close() before the return. The finally block will take care of it.

There is also an issue. If there is an exception in the getInstance call, you will attempt to dereference a null pointer in the finally block.

You can rewrite this

try (FileInputStream fis = new FileInputStream(this.filePath)) {
            MessageDigest md = MessageDigest.getInstance(HASH_ALG);

            byte[] bytes = new byte[1024];
            for (int bytesCount = fis.read(bytes); bytesCount >= 0; bytesCount = fis.read(bytes)) {
                md.update(byteArray, 0, bytesCount);
            }

            return String.format("%032x", new BigInteger(1, md.digest()));
        } catch (Exception e) {
            e.printStackTrace();
            throw new RuntimeException("Error in checksum");
        }

This uses a try-with-resources so that you don't have to manually close the resource.

I prefer names like bytes to byteArray as being simpler and more focused on what the data means logically than how it is stored physically. You can read more about this in discussions of Hungarian notation. This is somewhat controversial. I mostly mention it because the code was scrolling when it was named the Hungarian notation way.

Using the for loop to replace the while reduces the scope of the bytesCount variable. Other than that, both do the the same thing.

I added some vertical whitespace to break things up.

It's not necessary to assign the result of md.digest() to a variable. We can pass it directly to the BigInteger constructor. Either way works though.

StackExchange Code Review Q#163092, answer score: 6