File transfer from Server to Client in C programming

Question

for some reason, it's not transferring content over. I had strlen(file) in the send() function and it was working for txt file. It won't work with bin files though, so I changed it to bytes_read. Now nothing's transferring properly. I'm getting blank files on the client side. Also, in debugging with print statements, I noticed the bin file is skipping the read() while loop. Any ideas?

Here's my client:

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>
#include "packetErrorSend.h"
#include <sys/time.h>
#define MAX_LINE 1000

ssize_t recvx(int sockfd, void *buf, size_t len) {
  int var = recv(sockfd, buf, len, 0);
  if(var != -1)
  {
    return var;
  } else {
    printf("%s \n","Did not receive.");
    exit(1);
  }
}

int main(int argc, char *argv[])
{
  char buf[MAX_LINE];
  struct addrinfo hints;
  struct addrinfo *rp, *result;
  int bytes_received =  1;
  int s;
  char *server;
  char *port;
  char *file;
  int fd = -1; //file descriptor
  int bytes_written = 1;

  if (argc==4)
  {
    server = argv[1];
    port = argv[2];
    file = argv[3];
  }
  else
  {
    fprintf(stderr, "invalid # of arguments\n");
    exit(1);
  }

  /* Translate host name into peer's IP address */
  memset(&hints, 0, sizeof(hints));
  hints.ai_family = AF_INET;
  hints.ai_socktype = SOCK_STREAM;
  hints.ai_flags = 0;
  hints.ai_protocol = 0;

  if ((s = getaddrinfo(server, port, &hints, &result)) != 0 )
  {
    fprintf(stderr, "%s: getaddrinfo: %s\n", argv[0], gai_strerror(s));
    exit(1);
  }

  /* Iterate through the address list and try to connect */
  for (rp = result; rp != NULL; rp = rp->ai_next)
  {
    if ((s = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol)) == -1 )
    {
      continue;
    }
    if (connect(s, rp->ai_addr, rp->ai_addrlen) != -1)
    {
      break;
    }
    close(s);
  }

  if (rp == NULL)
  {
    perror("stream-talk-client: connect");
    exit(1);
  }
  freeaddrinfo(result);

  /*send lines of text */
  send(s, file, sizeof(file)+30, 0);
  int status = recv(s,buf,1,0);

  if(status == 0 || buf[0] == 'e')
  {
    fprintf(stderr, "Server Error: unable to access file %s \n", file);
    close(s);
    exit(0);
  }

  if(buf[0] == 's')
  {

    while(bytes_received != 0)
    {
      bytes_received = recvx(s, buf, 20);
      if(bytes_received == -1)
      {
        fprintf(stderr, "Client Error: Error receiving file \n");
        exit(0);
      } else {
        if(fd == -1)
        {
          fd = open(file, O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
          if(fd == -1)
          {
            fprintf(stderr,"Client Error: Open failed \n");
            break;
          }
          bytes_written = write(fd,buf,bytes_received);
          if(bytes_written == -1)
          {
            fprintf(stderr,"%s \n", "Client Error: Write error");
            break;
          }
        } else {
          bytes_written = write(fd,buf,bytes_received);
          if(bytes_written == -1)
          {
            fprintf(stderr,"%s \n", "Client Error: Write error");
            break;
          }
        }
      }
    }
  }
  if(close(fd) != 0)
  {
    printf("%s \n", "Client Error: File did not close successfully");
    exit(0);
  }


  close(s);

  return 0;

}

Server:

#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/sendfile.h>
#include "packetErrorSend.h"

#define SERVER_PORT "5432"
#define MAX_LINE 2000
#define MAX_PENDING 5

int main(int argc, char *argv[])
{
  struct addrinfo hints;
  struct addrinfo *rp, *result;
  char file[MAX_LINE];
  int s, new_s;
  int bytes_transferred = 0;
  int fd; //file descriptor
  char status[1];
  int bytes_read = 0;

  /* Build address data structure */
  memset(&hints, 0, sizeof(struct addrinfo));
  hints.ai_family = AF_INET;
  hints.ai_socktype = SOCK_STREAM;
  hints.ai_flags = AI_PASSIVE;
  hints.ai_protocol = 0;
  hints.ai_canonname = NULL;
  hints.ai_addr = NULL;
  hints.ai_next = NULL;

  /* Get local address info */
  if ((s = getaddrinfo(NULL, argv[1], &hints, &result)) != 0 )
  {
    fprintf(stderr, "%s: getaddrinfo: %s\n", argv[0], gai_strerror(s));
    exit(1);
  }

  /* Iterate through the address list and try to perform passive open */
  for (rp = result; rp != NULL; rp = rp->ai_next)
  {
    if ((s = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol)) == -1 )
    {
      continue;
    }

    if (!bind(s, rp->ai_addr, rp->ai_addrlen))
    {
      break;
    }
    close(s);
  }
  if (rp == NULL)
  {
    perror("stream-talk-server: bind");
    exit(1);
  }
  if (listen(s, MAX_PENDING) == -1)
  {
    perror("stream-talk-server: listen");
    close(s);
    exit(1);
  }

  freeaddrinfo(result);

  /* Wait for connection, then receive and print text */
  while(1)
  {
    for(int i = 0; i < sizeof(file); i++)
    {
      file[i] = '\0';
    }

    if ((new_s = accept(s, rp->ai_addr, &(rp->ai_addrlen))) < 0)
    {
      perror("stream-talk-server: accept");
      close(s);
      exit(0);
    }
    while(bytes_transferred == recv(new_s,file,sizeof(file),0))
    {
      if(bytes_transferred == -1)
      {
        close(new_s);
        exit(0);
      }
    }
    fd =open(file,O_RDONLY);
    if(fd < 0)
    {
      status[0] = 'e';
      send(new_s,status,sizeof(status),0);
      close(new_s);
      exit(0);

    }
    else
    {
      status[0] = 's';
      send(new_s,status,sizeof(status),0);
      while(bytes_read == read(fd,file,sizeof(file)) > 0)
      {
        if(bytes_read < 0)
        {
          close(new_s);
          exit(0);
        }
      }

      while((bytes_transferred = send(new_s,file,bytes_read,0)) > 0)
      {
        if(bytes_transferred == -1)

        {
          close(new_s);
          exit(0);
        }
      }
      if(close(fd) != 0)
      {
        close(new_s);
        exit(0);
      }
      else{
        break;
      }
    }
  }
  close(new_s);

  return 0;
}

Answer 1

Usual copy loop problem:

while((bytes_transferred == send(new_s,file,sizeof(file),0)) > 0)

should be

while((bytes_transferred = send(new_s,file,bytes_read,0)) > 0)

Detailed commentary on your code:

send(s, file, sizeof(file)+30, 0);

should be

send(s, file, strlen(file)+1, 0);

Then:

while(bytes_received != 0)
{
  bytes_received = recvx(s, buf, 20);
  if(bytes_received == -1)
  {
    fprintf(stderr, "Client Error: Error receiving file \n");
    exit(0);
  } else {
    if(fd == -1)
    {
      fd = open(file, O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
      if(fd == -1)
      {
        fprintf(stderr,"Client Error: Open failed \n");
        break;
      }
      bytes_written = write(fd,buf,bytes_received);
      if(bytes_written == -1)
      {
        fprintf(stderr,"%s \n", "Client Error: Write error");
        break;
      }
    } else {
      bytes_written = write(fd,buf,bytes_received);
      if(bytes_written == -1)
      {
        fprintf(stderr,"%s \n", "Client Error: Write error");
        break;
      }
    }
  }
}

A much simpler way to write all this would be:

fd = open(file, O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
if(fd == -1)
{
    fprintf(stderr,"Client Error: Open failed %s\n", strerror(errno));
   break;
}
while((bytes_received = recvx(s, buf, sizeof buf)) > 0)
{
      bytes_written = write(fd,buf,bytes_received);
      if(bytes_written == -1)
      {
        fprintf(stderr,"%s %s\n", "Client Error: Write error", strerr(errno));
        break;
      }
    }
  }
}
if(bytes_received == -1)
{
    fprintf(stderr, "Client Error: Error receiving file %s\n", strerr(errno));
    exit(0);
}

In the server, your code inside the accept loop is totally bizarre. It can be reduced to:

// receive a target file name
if ((bytes_transferred = recv(new_s,file,sizeof(file),0) > 0)
{
    fd = open(file,O_RDONLY);
    if(fd < 0)
    {
        perror("open");
        status[0] = 'e';
        send(new_s,status,1,0);
        close(new_s);
        exit(0);
    }
    status[0] = 's';
    send(new_s,status,1,0);

    while ((bytes_read = read(fd, file, sizeof file)) > 0)    
    {
        if (send(new_s,file,bytes_read,0) < 0)
        {
            perror("send");
            break;
        }
    }
    if (bytes_transferred == 0)
    {
       break;
    }
    else if(bytes_transferred == -1)
    {
        perror("read");
        close(new_s);
        exit(0);
    }
}
close(new_s);

E&OE

Note that you don't have to length-check send() in blocking mode. Posix requires that it blocks until all data has been transferred.

Answer 2

In your code, sizeof(file) is always MAX_LINE, hence the garbage at the end. Because sizeof cares nothing about what you put into variables; it only cares about their in-memory size:

#define MAX_LINE 2000
// ...
char file[MAX_LINE]; // sizeof(file) == 2000

Also, this line is writing beyond the length of the file array, and probably zeroing out some other variable that you care about:

file[sizeof(file)]='\0'; // very bad idea

To fix your initial problem, measure what your are sending (use strlen, or even better, strnlen) - and send only those bytes, including the terminating '\0' , instead of always sending MAX_LEN.

---

EDIT: after multiple downvotes, apparently due to not warning against the use of strlen & co. when used on non-textual inputs, I will now warn against the use of strlen for non-textual inputs:

• do not use strlen unless your input is 100% text.

• do not use it anyway, even with text: strnlen is safer, as it will never overflow the buffer (as long as the maximum-length parameter is correctly set):

  char line[MAX_LINE]; // ... read into line somehow int line_length = strnlen(line, MAX_LINE-1); // make sure that it is 0-terminated line[line_length] = 0;

To send binary data, you need to measure it first. File sizes can be measured using fseek and ftell. Do not try to use string functions (such as strnlen) or print (printf) binary data, as many bytes will map to control characters and mess up the display. If you need to print binary data, use code similar to this one.