10
\$\begingroup\$

Why did I write this code

This has a fun cartoon describing the problem of file transfer nowadays. And it's utterly true.

image

There are already some webRTC based P2P file transfer services online. But I want something solely point-to-point, and I like command line and native app.

What does the program do

The program reads a file and listens to a socket. Then it sends the file to anyone connect to that socket. It supports continuous downloading.

My question

Since I just learnt C (and HTTP and socket), I'm not very familiar about handling errors in C. Please give me some advice to make the program as robust as possible.

But keep code fast and clean is more important than handle EVERY possible error properly. For example the program will not respond a bad header if received a post request, it will simply drop the connection. (Is this OK? I'm not sure)

P.S. I know this only works on public IP, I'm working on NAT traversal. But that will be another question.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <wordexp.h>
#include <netdb.h>
#include <netinet/in.h>
#include <sys/stat.h>
#include <unistd.h> /* for close() */
#ifdef __linux__
#include <sys/sendfile.h>
#endif
#ifdef __APPLE__
#include <sys/socket.h>
#include <sys/uio.h>
#include <sys/types.h>
#endif



#define RIO_BUFSIZE 1024
#define BUFFER_SIZE 1024
#define MAXLINE 1024
#define MAX_FILENAME 512

#define REQUIRE_RANGE_TRUE 1
#define  REQUIRE_RANGE_FALSE 0

typedef struct
{
  int RIOfd;                 /* descriptor for this buf */
  off_t RIOrest;                /* unread byte in this buf */
  char *RIObufferPTR;           /* next unread byte in this buf */
  char RIObuffer[RIO_BUFSIZE];  /* internal buffer */
} riobuffer_t;

typedef struct {
  off_t offset;              /* for support Range */
  off_t end;
  int requireRange;
} httpRquest;

void RIOreadInitBuffer(riobuffer_t *rp, int fd)
{
  rp->RIOfd = fd;
  rp->RIOrest = 0;
  rp->RIObufferPTR = rp->RIObuffer;
}

ssize_t RIOread(riobuffer_t *rp, char *usrbuf, ssize_t n)
{
  int rest;

  while(rp->RIOrest <= 0) /* refill if buffer is empty */
  {
    rp->RIOrest = read(rp->RIOfd, rp->RIObuffer, sizeof(rp->RIObuffer));
    if (rp->RIOrest < 0) /* Interrupted by sig handler return */
    {
      if (errno != EINTR) return -1;
      else if (rp->RIOrest == 0) return 0; /* EOF */
      else rp->RIObufferPTR = rp->RIObuffer; /* reset buffer ptr */
    }
  }

  /* Copy min(n, rp->rio_cnt) bytes from internal buf to user buf */
  rest = n;
  if (rp->RIOrest < n) rest = rp->RIOrest;
  memcpy(usrbuf, rp->RIObufferPTR, rest);
  rp->RIObufferPTR += rest;
  rp->RIOrest -= rest;
  return rest;
}

ssize_t RIOreadlineB(riobuffer_t *rp, void *usrbuf, size_t maxlen)
{
  int n;
  char c, *bufferPTR = usrbuf;

  for (n = 1; (size_t) n < maxlen; n++)
  {
    int readCount;
    if ((readCount = RIOread(rp, &c, 1)) == 1)
    {
      *bufferPTR++ = c;
      if (c == '\n') break;
    }
    else if (readCount == 0)
    {
      if (n == 1) return 0; /* EOF, no data read */
      else break;    /* EOF, some data was read */
    }
    else return -1;    /* error */
  }
  *bufferPTR = 0;
  return n;
}

ssize_t RIOwriteN(int fd, void *usrbuf, size_t n)
{
  size_t nleft = n;
  char *bufferPTR = usrbuf;

  while (nleft > 0)
  {
    ssize_t numberWritten;
    if ((numberWritten = write(fd, bufferPTR, nleft)) <= 0)
    {
      if (errno == EINTR)  /* interrupted by sig handler return */
        numberWritten = 0;    /* and call write() again */
      else return -1;       /* errorno set by write() */
    }
    nleft -= numberWritten;
    bufferPTR += numberWritten;
  }
  return n;
}


/* find file name from path */
void findFilename(char *filepath, char* filename)
{
  size_t pt;
  size_t filenamePt = 0;

  for (pt = 0; pt < strlen(filepath); pt++)
  {
    char ch;
    ch = filepath[pt];
    filename[filenamePt] = ch;
    filenamePt++;
    if(ch == '/')
    {
      bzero(filename, sizeof(&filename));
      filenamePt = 0;
    }
  }

  filename[filenamePt + 1] = '\0';
}

void errorExit(char* text)
{
  perror(text);
  exit(0);
}

void expandFilePath(char* filepath)
{
  wordexp_t wordExpand;

  if (wordexp(filepath, &wordExpand, 0) != 0)
    errorExit("expanding file path failed");
  filepath = wordExpand.we_wordv[0];
}


void readHeaderFromClient(int socketFD, httpRquest *request)
{
  char buffer[MAXLINE], method[MAXLINE], url[MAXLINE], version[MAXLINE];

  riobuffer_t rioBuffer;
  RIOreadInitBuffer(&rioBuffer, socketFD);
  RIOreadlineB(&rioBuffer, buffer, MAXLINE);
  sscanf(buffer, "%s %s %s", method, url, version);

  if (strcmp(method, "GET"))
  {
    printf("not GET request\n");
    close(socketFD);
    exit(0);
  }

  request->requireRange = REQUIRE_RANGE_FALSE;
  request->offset = 0;
  request->end = 0;
  while (buffer[0] != '\n' && buffer[1] != '\n') /* end of header */
  {
    RIOreadlineB(&rioBuffer, buffer, MAXLINE);
    if (buffer[0] == 'R' && buffer[1] == 'a' && buffer[2] == 'n') /* find "Range" field */
    {
      request->requireRange = REQUIRE_RANGE_TRUE;
      if (sscanf(buffer, "Range: bytes=%lld-%lld", &request->offset, &request->end) <= 0)
        {
          fprintf(stderr, "failed to read Range: %s\n", buffer);
          request->requireRange = REQUIRE_RANGE_FALSE;
          continue;
        }
      /* bad range request */
      if (request->offset < 0 || request->end < 0 || request->offset > request->end ||
          (request->offset == 0 && request->end == 0))
        /* "Range: bytes=0-" will make both number 0 */
        {
          fprintf(stderr, "bad Range request or not supported: %s\n", buffer);
          request->requireRange = REQUIRE_RANGE_FALSE;
        }
    }
  }
}


off_t getFileLength(char *filepath)
{
  off_t fileLength;
  struct stat statBuffer;

  if (stat(filepath, &statBuffer) != 0 || (!S_ISREG(statBuffer.st_mode)))
    errorExit("checking status of file failed, is it a regular file?");

  fileLength = statBuffer.st_size;
  return fileLength;
}

void composeHeader(char *header, httpRquest *request, char *filepath)
{
  /* file name have to be shorter than MAX_FILENAME(1024) characters*/
  char filename[MAX_FILENAME];
  findFilename(filepath, filename);

  off_t fileLength;
  fileLength = getFileLength(filepath);
  if (fileLength < 0) errorExit("getting file length failed");

  if (request->requireRange == REQUIRE_RANGE_FALSE)
  {
    sprintf(header,
            "HTTP/1.1 200 OK\r\n"
            "Accept-Ranges: bytes\r\n"
            "Content-Disposition: attachment; filename=\"%s\"\r\n"
            "Content-Length: %lld\r\n"
            "\r\n", filename, fileLength);
  } else if (request->requireRange == REQUIRE_RANGE_TRUE)
  {
    sprintf(header,
            "HTTP/1.1 206 Partial\r\n"
            "Accept-Ranges: bytes\r\n"
            "Content-Disposition: attachment; filename=\"%s\"\r\n"
            "Content-Range: bytes %lld-%lld/%lld\r\n"
            "Content-Length: %lld\r\n"
            "Content-Type: multipart/byteranges\r\n"
            "\r\n", filename, request->offset, request->end, fileLength, request->end - request->offset + 1);
    printf("offset: %lld\nend: %lld\n", request->offset, request->end);
  }
  puts(header);
}

#ifdef __APPLE__
void sendFile(char *filepath, int clientSocketFD, httpRquest *request)
{
  FILE *file = fopen(filepath, "rb");
  if (!file) errorExit("failed to open file");

  if (request->requireRange == REQUIRE_RANGE_FALSE)
    request->end = getFileLength(filepath);

  off_t totalSize = request->end - request->offset;
  off_t offset = request->offset;
  off_t totalBytesSent = 0;
  off_t len = BUFFER_SIZE;

  /* send file in chunks */
  while (totalBytesSent < totalSize)
  {
    int ret;
    if ((totalSize - totalBytesSent) < BUFFER_SIZE)
      len = totalSize - totalBytesSent;

    ret = sendfile(fileno(file), clientSocketFD, offset, &len, NULL, 0);
    if (ret < 0) errorExit("failed to send file");
    totalBytesSent += len;
    offset += len;
  }
}
#endif

#ifdef __linux__
void sendFile(char *filepath, int clientSocketFD, httpRquest *request)
{
  FILE *file = fopen(filepath, "rb");
  if (!file) errorExit("failed to open file");

  if (request->requireRange == REQUIRE_RANGE_FALSE)
    request->end = getFileLength(filepath);

  off_t offset = request->offset;
  off_t bytesLeftToSend = 0;
  off_t bytesSentInOneAction = 0;
  size_t len = BUFFER_SIZE;
  int fileNo = fileno(file);

  bytesLeftToSend = request->end - offset + 1;
  while (bytesLeftTosend > 0)
  {
    if (bytesLeftToSend < BUFFER_SIZE) len = bytesLeftToSend;

    bytesSentInOneAction = sendfile(clientSocketFD, fileNo, &offset, len);

    if (bytesSentInOneAction < 0) errorExit("failed to send file");

    offset += bytesSentInOneAction;
    bytesLeftToSend -= bytesSentInOneAction;
  }
}
#endif

void serveFile(int clientSocketFD, char *filepath)
{
  httpRquest request;
  readHeaderFromClient(clientSocketFD, &request);

  char header[BUFFER_SIZE];
  composeHeader(header, &request, filepath);
  RIOwriteN(clientSocketFD, header, strlen(header));
  sendFile(filepath, clientSocketFD, &request);
}

void initListening(int socketFD, struct sockaddr_in *address, int port)
{
  /* create socket */
  if (socketFD < 0)
    errorExit("creating socket failed");

  /* configure socket */

  address->sin_family = AF_INET;
  address->sin_port = htons(port); /* host to network short */
  address->sin_addr.s_addr = INADDR_ANY;

  /* bind & listen */
  if(bind(socketFD, (struct sockaddr*) address, sizeof(*address)) != 0)
    errorExit("binding failed");

  if (listen(socketFD, 16)!=0) errorExit("listening failed");
}

void serve(char* filepath, int port)
{
  int socketFD = socket(PF_INET, SOCK_STREAM, 0);
  struct sockaddr_in address;
  initListening(socketFD, &address, port);

  /* accept client */
  for (;;)
  {
    socklen_t size = sizeof(address);
    int clientSocketFD = accept(socketFD, (struct sockaddr*) &address, &size);

    puts("client connected");

    /* if (fork() == 0) */
    /* { */
    /*   close(socketFD); */
    /*   serveFile(clientSocketFD, filepath); */
    /*   /\* close(clientSocketFD); *\/ */
    /*   /\* exit(0); *\/ */
    /* } */
    /* else */
    /* { */
    /*   close(clientSocketFD); */
    /* } */
    serveFile(clientSocketFD, filepath);
    sleep(5);
    close(clientSocketFD);
  }
}

int main(int argc, char *argv[])
{
  if (strcmp(argv[1], "-h") == 0)
  {
    puts("format: pass <filepath> <port>");
    puts("file name(not path) must not exceed 1024 characters");
    puts("keyboard interrupt to kill");
    return 0;
  }

  if (argc < 3 || argc > 3)
  {
    fprintf(stderr, "needs 2 arguments: file path & port\n");
    return -1;
  }

  char *filepath = argv[1];
  int port = atoi(argv[2]);

  printf("serving  %s  on port  %d\n"
         "keyboard interrupt to kill\n", filepath, port);

  expandFilePath(filepath);
  serve(filepath, port);
}
\$\endgroup\$
5
\$\begingroup\$

Types

Nice use of typedef with struct. But try not to add _t endings to your type names. Those are reserved by POSIX.

Although it is common in C there is no need to drop random characters from type and variable names like in httpRquest (missing e).

Also, your type names don't follow any particular convention. Stick to one like type_name or TypeName. I will use the latter.

Example type declaration:

typedef struct {
    off_t offset; /* HTTP "Range" support */
    off_t end;
    int requireRange;
} HTTPRequest;

exitError function

It is common practice to use some sort of die function to write an error message and terminate the program.

You may consider calling exit(EXIT_FAILURE) instead of exit(0) to terminate your program with a status other than 0.

Some quotes from exit man page:

The C standard specifies two constants, EXIT_SUCCESS and EXIT_FAILURE, that may be passed to exit() to indicate successful or unsuccessful termination, respectively.

and

The use of EXIT_SUCCESS and EXIT_FAILURE is slightly more portable (to non-UNIX environments) than the use of 0 and some nonzero value like 1 or -1. In particular, VMS uses a different convention.

die function may look like this:

void die(const char *fmt, ...) {
    va_list ap;

    va_start(ap, fmt);
    vfprintf(stderr, fmt, ap);
    va_end(ap);

    if (fmt[0] && fmt[strlen(fmt)-1] == ':') {
        fputc(' ', stderr);
        perror(NULL);
    } else {
        fputc('\n', stderr);
    }

    exit(EXIT_FAILURE);
}

Example usage:

if (rand() > RAND_MAX/2)
    die("%s, you ran out of luck, try again later", "Dude");

/* If last non-NULL char is `:`, `die` will call `perror`: */
if (NULL == fopen("/no/such/file", "r"))
    die("Failed to read file:");

Okay, let's jump right to the bottom and check your main function.

Use of argc and argv

Woops, I see some random memory access. Right on the first line of main where you check for -h argument.

Lets see in detail, how your argv and argc look like.

I will use this simple C code to test it:

#include <stdio.h>
#include <string.h>

int main(int argc, char **argv) {
    printf("argc: %d\n", argc);
    for (int i = 0; i < argc; i++) {
        printf("%d: \"%s\"\n", i, argv[i]);
    }
}

Some test results:

$ ./arguments-test
argc: 1
0: "./arguments-test"
$
$ ./arguments-test -h
argc: 2
0: "./arguments-test"
1: "-h"
$
$ ./arguments-test code review
argc: 3
0: "./arguments-test"
1: "code"
2: "review"
$
$ ./arguments-test "code review"
argc: 2
0: "./arguments-test"
1: "code review"

When your program is executed without any arguments (like in ./arguments-test) argv will still have argv[0], which stores your path to the executed program, but nothing apart that.

On the very first line of main you are testing argv[1] against -h but if your program has no arguments it will access out-of-bounds array index and probably be terminated by SIGSEGV. So, before using argv you must check argc value like this:

int main(int argc, char **argv) {
    if (argc == 2 && strncmp(argv[1], "-h", sizeof("-h")) == 0)) {
        /* Printing program usage */
        return EXIT_SUCCESS;
    }

    if (argc == 3) {
        /* Parsing filepath name and port number, running */
        return EXIT_SUCCESS;
    }

    /* Probably wrong program usage */
    return EXIT_FAILURE;
}

Tips on -h

It is nice to have standalone usage function, that will output help message on -h argument or program misuse (wrong number of arguments, bad arguments).

So our strategy will be:

  • on -h write help message to stdout and exit
  • on program misuse write it to stderr and exit with an error

Let's write one:

/* `argv0` is used to print actual program name in `usage`,
   must be set in `main` function */
static const char *argv0;

/* Will print program help and exit */
static void usage(FILE *stream) {
    fprintf(
        stream,
        "usage:\n"
        "  %s -h\n"
        "  %s [path to file] [port]",
        argv0, argv0
    );
}

int main(int argc, char **argv) {

    /* Remember `argv[0]` to print in `usage` */
    argv0 = argv[0];

    if (argc == 2 && strncmp(argv[1], "-h", sizeof("-h")) == 0) {
        /* Printing program usage */
        usage(stdout);
        return EXIT_SUCCESS;
    }

    if (argc == 3) {
        /* Parsing arguments and running */
        return EXIT_SUCCESS;
    }

    /* Probably wrong program usage */
    usage(stderr);
    return EXIT_FAILURE;
}

Port parsing

You use atoi to parse your port from argv[2]. Let's see man page for this function:

The atoi() function converts the initial portion of the string pointed to by nptr to int. The behavior is the same as strtol(nptr, NULL, 10) except atoi() does not detect errors.

Some possible pitfalls:

  • int type is guaranteed to store [−32767, +32767], but port numbers are [1, 65535]
  • atoi converts initial portion of the string, so garbage like 8080garbage is allowed
  • it does not detect errors

I suggest you to use strtol:

if (argv[2] < '0' || argv[2] > '9') {
    /* Port argument must start with digits */
    usage(stderr);
    return EXIT_FAILURE;
}

char *end;
long int port = strtol(argv[2], &end, 10);

if (end != NULL || port < 0 || port > 65535) {
    /* Port argument must not contain garbage and must represent a valid port number */
    usage(stderr);
    return EXIT_FAILURE;
}

Filepath argument

Your second argument is a path to file to be sent. Let's check expandFilePath function.

Hah, I see, you use wordexp, and I bet you are getting it wrong. Your shell will expand filepath before the program starts. No need to do that once again.

There is one possible issue in the current implementation: wordexp call must be followed with wordfree, otherwise it is a memory leak.

Also, It worth to test if the file is readable before actual socket binding.

if (!access(filepath, R_OK)) {
    die("Provided file doesn't exist or is not readable:");
}

I will write about other parts of your program a bit later. Feel free to comment or edit this answer.

\$\endgroup\$
  • 1
    \$\begingroup\$ I heard that comments are not for saying thanks but thanks! \$\endgroup\$ – Yuan Fu Apr 17 '18 at 20:40
  • \$\begingroup\$ @YuanFu, You are welcome (: \$\endgroup\$ – sineemore Apr 17 '18 at 21:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.