Send and receive file with socket

Question

This part of code is used by a client when receiving a file:

void do_retr_cmd(int f_sockd){
  int fd;
  ssize_t nread = 0;
  uint32_t fsize, fsize_tmp, total_bytes_read, size_to_receive;
  char *filename = NULL, *conferma = NULL, *filebuffer = NULL;
  char buf[256], dirp[256], t_buf[256];

  memset(dirp, 0, sizeof(dirp));
  memset(buf, 0, sizeof(buf));
  memset(t_buf, 0, sizeof(t_buf));
  printf("Write the name of file to download: ");
  fgets(dirp, BUFFGETS, stdin)
  filename = NULL;
  filename = strtok(dirp, "\n");
  sprintf(buf, "RETR %s", dirp);
  if(send(f_sockd, buf, strlen(buf), 0) < 0){
    perror("Errore durante l'invio del nome del file");
    onexit(f_sockd, 0, 0, 1);
  }
  fsize = 0;
  recv(f_sockd, t_buf, sizeof(t_buf), 0)
  fsize = atoi(t_buf);
  fd = open(filename, O_CREAT | O_WRONLY, 0644);
  fsize_tmp = fsize;
  filebuffer = (char *)malloc(fsize);
  total_bytes_read = 0;
  nread = 0;
  for(size_to_receive = fsize; size_to_receive > 0;){
    nread = read(f_sockd, filebuffer, size_to_receive);
    if(nread < 0){
      perror("read error on retr");
      onexit(f_sockd, 0, 0, 1);
    }
    if(write(fd, filebuffer, nread) != nread){
      perror("write error on retr");
      onexit(f_sockd, 0, 0, 1);
    }
    size_to_receive -= nread;
  }
  close(fd);
  fflush(stdout);
  fflush(stdin);
  memset(buf, 0, sizeof(buf));
  recv(f_sockd, buf, 21, 0)
  printf("%s", buf);
  memset(buf, 0, sizeof(buf));
  memset(t_buf, 0, sizeof(t_buf));
  memset(dirp, 0, sizeof(dirp));
  free(filebuffer);
}

And this part of code is used by a server when sending a file:

void do_server_retr_cmd(f_sockd, m_sockd){
  int fd, rc;
  uint32_t fsize, size_to_send;
  char *filename = NULL, *other = NULL;
  char buf[512], t_buf[256];
  off_t offset;
  struct stat fileStat;

  memset(buf, 0, sizeof(buf));
  memset(t_buf, 0, sizeof(t_buf));
  recv(f_sockd, buf, sizeof(buf), 0)
  other = NULL;
  filename = NULL;
  other = strtok(buf, " ");
  filename = strtok(NULL, "\n");

  if(strcmp(other, "RETR") == 0){
    printf("Ricevuta richiesta RETR\n");
  } else /* do something */

  fd = open(filename, O_RDONLY);

  memset(&fileStat, 0, sizeof(fileStat));
  fileStat.st_size = 0;
  fstat(fd, &fileStat)
  fsize = fileStat.st_size;
  snprintf(t_buf, 255, "%" PRIu32, fsize);
  send(f_sockd, t_buf, sizeof(t_buf), 0)
  offset = 0;
  for (size_to_send = fsize; size_to_send > 0; ){
    rc = sendfile(f_sockd, fd, &offset, size_to_send);
    if (rc <= 0){
      perror("sendfile");
      onexit(f_sockd, m_sockd, fd, 3);
    }
    size_to_send -= rc;
  }
  close(fd);
  fflush(stdout);
  fflush(stdin);
  memset(buf, 0, sizeof(buf));
  strcpy(buf, "226 File transfered\n");
  send(f_sockd, buf, strlen(buf), 0)
  memset(buf, 0, sizeof(buf));
  memset(t_buf, 0, sizeof(t_buf));
}

--> Errors checking have been omitted <--
I have a big problem with these 2 pieces of code. When i start the main program i have to write:
1. retr and then i press enter
2. Write the filename to download: i write the filename and then i press enter
The problem is that sometimes the file is downloaded correctly and sometimes it is not downloaded but a part of it is displayed on stdout (on the terminal).
I don't understand why i got this strange behavior.
PS: i know my code is ugly but i a C-newbie!

I'm developing on Ubuntu amd64 and using GCC-4.6.3 (C language).

Answer 1

The previous version of my answer wasn't quite right, but here's why you are seeing the strange behaviour. You send the file size as

snprintf(t_buf, 255, "%" PRIu32, fsize);

You then receive with

recv(f_sockd, t_buf, sizeof(t_buf), 0)

but that's not guaranteed to actually read sizeof(t_buf) bytes. Then atoi will sometimes return an incorrect size, and the rest of the file is treated as the status message, which is printed at the end (up to the first null char).

Because recv may not return all of the data you ask for at once, you have to check its return value and potentially repeat the call to recv:

size_t to_recv = sizeof(t_buf);
size_t rcvd = 0;
while (to_recv > 0) {
    ssize_t r = recv(f_sockd, t_buf + rcvd, sizeof(t_buf) - rcvd, 0);
    if (r < 0) {
        //error
    }
    else {
        to_recv -= r;
        rcvd += r;
    }
}

Obviously you either have to know how much data to expect, or come up with a better protocol as suggested in the other answers (for example, look for a terminator to determine when you have read the size).

Answer 2

recv(f_sockd, buf, 21, 0)
printf("%s", buf);

This printf prints a ton of random junk because there's no actual protocol implemented to control what it receives and what it prints. For example, how does the printf know how many bytes to print?

Answer 3

TCP connections gives you a reliable bi-directional stream of bytes, but the boundaries of your "application messages" are not preserved, meaning one send() can be received in multiple recv()s on the other side, and the other way around, several send()s can be collapsed into one recv() (and you can receive part of the last chunk you sent). The good thing is you do receive bytes you sent, in the order you sent them.

The line recv(f_sockd, buf, sizeof(buf), 0); in the server code assumes that you read the file name here, while in fact you can get up to 256 bytes of whatever your client sent.

You need to impose some sort of an application-level protocol on top of bare TCP. A very simplistic one would be to send a textual header in front of your file content in the form:

file-size file-name\n

So your server can look up the first newline, split the line on the first space and have the number of bytes to expect, and the file name to save those bytes to. Do not ignore the rest of the receive buffer after that newline, save it to the file. This also gives you a possibility of re-using that connection for multiple file transfers.

Hope this helps.