1

first of all, I know there are already a couple of posts here about similar issues, but I've gone through them and their problems were not mine nor their solutions solve my problem. If anyone has a post which solves my question, please reply with the link and I'll go for it.

Now, the problem - I've got a client and a server. The client requests a file, the server sends it and then the client receives it, or at least that's how it should be. But it is not. What happens instead? The client receives the first 1024 bytes, then the next 1024 bytes, and then receives 436 bytes (I'm always using the same file so I always get the same results) and ends because it has received less than 1024 bytes and therefore it has deducted than it was the last read it had to perform, but it shouldn't be, because the server, when reading the file from its FileInputStream, reads much more bytes (much more pieces of 1024 bytes). I've tried everything I've imagined, but I've got no positive result: Always, the third piece, instead of being whole received, is just received up to the byte 436 (maybe what happens is that, from the third piece up to the penultimate are skipped for some reason, and then the last one is 436 bytes and is received, althought I don't think this is the case since the whole file isn't read in the server because the client closes the connection as soon as it thinks the transmission has finished, provoking an exception on the server that interrupts it from reading anything).

And here goes what I think is the key code:

Server

OutputStream put;
        try {
            ServerSocket serverSocket;
            try {
                serverSocket = new ServerSocket(DesktopClient.PORT_FOR_FILES);
            } catch (IOException ex) {
                JOptionPane.showMessageDialog(null, ex.toString(), "Error", JOptionPane.ERROR_MESSAGE);
                return false;
            }
            Socket socket;
            try {
                socket = serverSocket.accept();
                ActionWindow.println("Connection for files transference stablished - " + socket);
            } catch (Exception ex) {
                JOptionPane.showMessageDialog(null, ex.toString(), "Error", JOptionPane.ERROR_MESSAGE);
                return false;
            }
            put = socket.getOutputStream();
            BufferedReader requestedVideoBufferedReader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
            String requiredVideo = requestedVideoBufferedReader.readLine();
            while (!requiredVideo.matches("finish") && !requiredVideo.matches("errored")) {
                ActionWindow.println("The screen in the router " + ActionWindow.currentConn + " is asking for " + requiredVideo);
                if (new File(ActionWindow.pathToVideosFolder + requiredVideo + ActionWindow.videoFilesExtension).exists()) {
                    try (InputStream in = new FileInputStream(ActionWindow.pathToVideosFolder + requiredVideo + ActionWindow.videoFilesExtension)) {
                        byte[] buf = new byte[1024];
                        int len;
                        System.out.println("About to write");
                        while (true) {
                            len = in.read(buf);
                            System.out.println("len: " + len);
                            if (len > 0) {
                                put.write(buf, 0, len);
                            } else {
                                break;
                            }
                        }
                    }
                    ActionWindow.println("File " + requiredVideo + ActionWindow.videoFilesExtension + " sent.");
                } else {
                    ActionWindow.println("File " + requiredVideo + ActionWindow.videoFilesExtension + " couldn't be found in the local index.");
                    put.close();
                    socket.close();
                    serverSocket.close();
                    return false;
                }
                requiredVideo = requestedVideoBufferedReader.readLine();
            }
            if (requiredVideo.matches("errored")) {
                JOptionPane.showMessageDialog(null, "Se produjo un error en la transmisión de archivos. Los datos del servidor no han sido modificados.", "Error", JOptionPane.ERROR_MESSAGE);
            }
            put.close();
            socket.close();
            serverSocket.close();
            ActionWindow.println("Connecion finished.");
        } catch (IOException ex) {
            if (!(ex instanceof SocketException)) { //This means that the exception has not been caused by finishing the transference.
                JOptionPane.showMessageDialog(null, ex.toString(), "Error", JOptionPane.ERROR_MESSAGE);
                return false;
            } else {
                return true;
            }
        }
        return true;

Client

fileRequester.println(x);
            ServerDaemon.println("Requested " + x);
            File destFile = new File(ServerDaemon.pathToVideosFolder + x + ServerDaemon.videoFilesExtension);
            byte[] buf = new byte[1024];
            OutputStream streamForWritingToFile;
            try {
                streamForWritingToFile = new FileOutputStream(destFile);
            } catch (FileNotFoundException ex) {
                ServerDaemon.println(ex.toString());
                return false;
            }
            int len;
            try {
                while (true) {
                    len = streamFromServer.read(buf);

                    if (len > 0) {
                        streamForWritingToFile.write(buf, 0, len);
                    } else {
                        break;
                    }

                    if (len != 1024) {
                        break;
                    }
                }
            } catch (IOException ex) {
                ServerDaemon.println(ex.toString());
                if (ex instanceof SocketException) {
                    ServerDaemon.disconnect();
                }
                return false;
            }

            ServerDaemon.println("Received file " + x);

EDIT: As a temporal solution, I'm sending the file size through UDP so the receiver know how much data must it wait for. However, it's higly preferred to have it working through TCP, but it doesn't work (take a look at the comments in @PeterLawrey 's answer). This is what I've tried in order to make it work through TCP:

Receiver:

BufferedReader inFromClient;
int fileSize;
String receivedSizeMsg = null;
try {
      inFromClient = new BufferedReader(new InputStreamReader(socket.getInputStream()));
      receivedSizeMsg = inFromClient.readLine();
    } catch (IOException ex) {
      ServerDaemon.println(ex.toString());
    }
fileSize = Integer.parseInt(receivedSizeMsg.substring(4));

Sender:

DataOutputStream outToServer;
outToServer = new DataOutputStream(socket.getOutputStream());
outToServer.writeBytes("size" + new File(file+"\n");

SECOND EDIT: With TCP:

Receiver:

DataInputStream inFromClient;
int fileSize = 0;
inFromClient = new DataInputStream(socket.getInputStream());
fileSize = Integer.parseInt(inFromClient.readUTF().substring(4));

Sender:

DataOutputStream outToServer;
outToServer = new DataOutputStream(socket.getOutputStream());
outToServer.writeUTF("size" + new File(ActionWindow.pathToVideosFolder + requiredVideo + ActionWindow.videoFilesExtension).length());
  • TCP's a streaming protocol. It doesn't do messages. If your server does one 1k write, the client could receive the data in any number of chunks. Relying on the size of the `read` on the client will never work reliably. – Mat Aug 02 '12 at 12:57

2 Answers2

3

You have a problem here.

                if (len != 1024) {
                    break;
                }

This assumes that data will arrive with the same length it is sent with which is not true. A blocking read() is only guaranteed to read one byte. Take this out of your code and it may work now.

For example, TCP often combines data into packets of around 1500 bytes. When you are reading the data, you can read it faster than it is sent so you might expect to read a block of 1024 bytes followed by 476 bytes (minus any TCP header)


EDIT: a simple send file and receive file method.

public static void sendFile(DataOutputStream out, String fileName) throws IOException {
    FileInputStream fis = new FileInputStream(fileName);
    try {
        byte[] bytes = new byte[8192];
        int len;
        out.writeInt((int) fis.getChannel().size());
        while ((len = fis.read(bytes)) > 0)
            out.write(bytes, 0, len);
    } finally {
        fis.close();
    }
}

public static void recvFile(DataInputStream in, String fileName) throws IOException {
    FileOutputStream fos = new FileOutputStream(fileName);
    try {
        byte[] bytes = new byte[8192];
        int left = in.readInt();
        int len;
        while(left > 0 && (len = in.read(bytes, 0, Math.min(left, bytes.length))) > 0) {
            fos.write(bytes, 0, len);
            left -= len;
        }
    } finally {
        fos.close();
    }
}
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • I was guessing that could have something to see here. However, that piece of code is not there for nothing: I had thought that I could use it to detect the end of the file, because otherwise the client wouldn't be able to know when the file transmission is finished. By reading @mike 's answer, I'm thinking that maybe I could use a Timer object to decide when the transmission has ended, al although I keep doubting there's nothing easier. Anyway, I'll test removing this snippet tomorrow when back to work since I can't do it in home. – Jorge Antonio Díaz-Benito Aug 02 '12 at 16:05
  • You close the connection when the end of the file is sent so you just need to detect for `if (len < 0)` which is what you do already. – Peter Lawrey Aug 02 '12 at 16:23
  • Actually, the complete program doesn't necessary close the connection when a file is received, but it may request a new one by resetting the corresponding streams, so the client can't wait for the server to disconnect in order to understand it as an end-of-file signal, because it will be just valid in case there's only one file to be transmitted. – Jorge Antonio Díaz-Benito Aug 02 '12 at 16:41
  • In that case, you can send the length of the file first and when you have read that many bytes you know you have reached the end. You might also want to send the file name if you have multiple files waiting. Youc an use DataOutptuStream/DataInputStream to `writeInt/readInt` and `writeUTF/readUTF` – Peter Lawrey Aug 02 '12 at 16:49
  • But by doing that I wouldn't be able to know how much of the received data corresponds to the size of the file. But in order to solve that, I could use an UDP message since the client is also listening for other matters of the program, and then send an ACK of that message back to the server, which wouldn't send the file until receiving the ACK. What about this? – Jorge Antonio Díaz-Benito Aug 02 '12 at 18:31
  • You only needs to count how much data you have read to keep track of where you are up to in the file. Using UDP is a very bad idea, much harder to get right. – Peter Lawrey Aug 02 '12 at 19:28
  • Ok I'll try the TCP approach first. – Jorge Antonio Díaz-Benito Aug 02 '12 at 20:33
  • One of the features of UDP is that you can't be sure if a packet will be delivered. The sender and receiver has no idea it has failed. – Peter Lawrey Aug 02 '12 at 20:36
  • Just don't try to bodge this with extra messaging on other channels, UDP or not. A timer approach will also be unreliable. Use a protocol, connect/disconnect or some other protocol that has less-than-dire performance. If you bodge this, it will be unreliable and fail frequently. 'although I keep doubting there's nothing easier' - there is nothing to replace a protocol that works, correctly assembling the byte-stream into protocol-units, nothing at all. – Martin James Aug 02 '12 at 22:48
  • Although I agree both of you in that the best solution is saying the file size by TCP, doing it drives me to a new problem: somehow the file size becomes 2048 bytes more than it should, and then the receiver gets stucked since it is waiting for more bytes to be send but the sender won't send any more bytes because there are no more bytes to send. So TCP doesn't yet look like a solution. I'll implement the file size sending through UDP solution until I have some time to get the TCP equivalent to work. – Jorge Antonio Díaz-Benito Aug 03 '12 at 09:13
  • If the size is wrong, sending it via UDP won't fix this. Use the `new file(filename).length()` or `FileChannel.getLength()` to get the length. The size of the file could change if it is being modified while you read it, but its unlikely to get smaller. – Peter Lawrey Aug 03 '12 at 09:33
  • For both my TCP test and the UDP working implementation I've used the `new file(filename).length()` approach (see the main post edit I've added in order to check my TCP test). However, the file size is right when I send it by UDP but not when I send it by TP, although it's being obtained the same way. That dives me to think of that the fle size is maybe readed twice, the first with `inFromClient.readLine();` and the second one with `streamFromServer.read(buf);` , although I don't really think that's what is actually happening. The problem is that I've no clue on what may actually be happening. – Jorge Antonio Díaz-Benito Aug 03 '12 at 09:44
  • Receiver - `BufferedReader inFromClient; int fileSize; String receivedSizeMsg = null; try { inFromClient = new BufferedReader(new InputStreamReader(socket.getInputStream())); receivedSizeMsg = inFromClient.readLine(); } catch (IOException ex) { ServerDaemon.println(ex.toString()); } fileSize = Integer.parseInt(receivedSizeMsg.substring(4));` Sender - `DataOutputStream outToServer; outToServer = new DataOutputStream(socket.getOutputStream()); outToServer.writeBytes("size" + new File(file+"\n");` – Jorge Antonio Díaz-Benito Aug 03 '12 at 10:08
  • **Never mixed text and binary readers unless you enjoy confusion**. The BufferedReader will read a whole buffer e.g. 2 KB even if that is after the line. – Peter Lawrey Aug 03 '12 at 10:11
  • That would explain why I got an extra file size of 2048 bytes in my test. I should use writeUTF [link](http://docs.oracle.com/javase/7/docs/api/java/io/DataOutputStream.html#writeUTF(java.lang.String)) I guess. Gonna test it right now. – Jorge Antonio Díaz-Benito Aug 03 '12 at 10:17
  • Sending writeInt() is much simpler and more efficient. Use writeUTF if you want to send the filename. – Peter Lawrey Aug 03 '12 at 10:26
  • 1
    Not necessary the file name, but I want to send the 'size' keyword before the size number. Anyway, I've finally got it to work with TCP. Thank you very much, indeed. I'm going to add it to the main post for other people with this problem. – Jorge Antonio Díaz-Benito Aug 03 '12 at 10:42
0

The others have given you the answer, basically, you aren't guaranteed to get data in a single packet. In networking, there are so many variables such as configuration (tcp_nodelay), packet size, etc. that it's better to do defensive programming.

Build a little protocol. Send say a 15 byte header that contains the size of the file and maybe a checksum. Any client who connects has to send that 15 byte header. Then your server can just sit in a loop and wait for data. You can even poll the socket (select) and time out if you don't get any more data for N number of seconds.

At the end, you can use the checksum to see if the file transmitted successfully.

Java World had an article about how to check the socket without blocking, but if your just doing a homework assignment or something small, it's probably not worth it.

Mike
  • 3,186
  • 3
  • 26
  • 32
  • Implementing a custom protocol maybe is taking too much risk taking into account the deadline I have for this, since I've never done it before and therefore it could drive me to too many time-consuming tasks. However, your idea of the time-out looks nice (take a look at my comment at Peter Lawrey's post). – Jorge Antonio Díaz-Benito Aug 02 '12 at 16:07
  • Implementing a protocol on top of TCP is the only thing that will work reliably. Disconnecting after sending a file is a protocol of sorts, though it will have poor performace because of the connect/disconnect handshake latencies. Any attempt to control the messages with timers will fail, eventually. You should do this correctly, else.. – Martin James Aug 02 '12 at 22:43