Multi-threaded web server serving HTML, images, etc

Question

I have a web server that can send websites, images, mp3 and other things and I was wondering how I could improve the code and make the server more efficient.

//this is where you enter the default file directories
char *defaultDir1 = "/Users/rowan/Desktop/webServer(Mac)/website/displaySite/";
char *defaultDir2 = "/Users/rowan/Desktop/webServer(Mac)/";




#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <errno.h>
#include <pthread.h>



struct sockaddr_in sockaddr;
char webFileDir[9000];
char servFileDir[9000];
#define THREAD_POOL_SIZE 200
#define BACKLOG 200

pthread_t thread_pool[THREAD_POOL_SIZE];




//creates a lock and unlock system for enqueue and dequeue
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
//creates a sleep timer for the threads/does something until given a signal or condition.
pthread_cond_t condition_var = PTHREAD_COND_INITIALIZER;

struct node {
    int client_socket;
    struct node *next;
};
typedef struct node node_t;

node_t *head = NULL;
node_t *tail = NULL;

void enqueue(int client_socket) {
    node_t *newnode = malloc(sizeof(node_t)); //create node
    newnode->client_socket = client_socket; //assign client socket
    newnode->next = NULL; //add node to the end of the list
    if (tail == NULL) { //checks if there is nothing on the tail and if so adds a head
        head = newnode;
    } else {
        //if there is a tail then add a new node onto the tail
        tail->next = newnode;
    }
    tail = newnode;
    //this all basically creates a new node then moves it over over and over again until stopped
}

int dequeue() {
    if (head == NULL) return -1;//if the head is empty or nothing has been enqueued then return null


    if (head != NULL) { //else...
        int result = head->client_socket;
        node_t *temp = head;
        head = head->next;
        if (head == NULL) {tail = NULL;}
        free(temp);
        return result;
    }
    //this function removes everything off of the queue and returns the socket
    return 0;
}



int *handleClient(int pclientSock) {
    FILE *fpointer;
    char *findExtention;
    char *clientIP;
    char *endDir;
    int freadErr;
    int fsize;
    int bin;
    int readSock;
    char recvLine[2];
    char fileDir[9000];
    char recvLineGET[60];
    char httpResponseReport[1000];
    char *fileLine;
    char httpResponse[1000];
    char *endOfPost;
    char fullResp[100000];
    int checkSend;
    char *startDir;
    char *getEnd;
    int responseCode = 200;
    
    
    bzero(fileDir, sizeof(fileDir));
    bzero(recvLineGET, sizeof(recvLineGET));


    //get client ip
    int acceptSock = pclientSock;
    socklen_t addrSize = sizeof(struct sockaddr_in);
    getpeername(acceptSock, (struct sockaddr *)&sockaddr, &addrSize);
    clientIP = inet_ntoa(sockaddr.sin_addr);


    
    fullResp[0] = '\0';
    //handles reading client http request
    while (1) {
        if ((readSock = read(acceptSock, recvLine, 1)) != 1) {
            perror("error reading from socket");

            //error 500: internal server error
            send(acceptSock, "HTTP/1.1 500\r\n\r\n", 16, MSG_NOSIGNAL);

            bzero(recvLine, sizeof(recvLine));
            bzero(fullResp, sizeof(fullResp));

            close(acceptSock);
            return 0;
        }
        recvLine[1] = '\0';
        strcat(fullResp, recvLine);

        if ((endOfPost = strstr(fullResp, "\n")) != NULL) {
            break;
        } else if (readSock < 0) {
            //perror("endOfHttpBody error");

            bzero(fullResp, sizeof(fullResp));

            close(acceptSock);
            return 0;
        }
        bzero(recvLine, sizeof(recvLine));
    }





    strcpy(fileDir, webFileDir);



    if ((getEnd = strstr(fullResp, "HTTP")) == NULL) {
        perror("error reading from socket");

        bzero(fullResp, sizeof(fullResp));
        //error 400: bad request
        send(acceptSock, "HTTP/1.0 400\r\n\r\n", 16, MSG_NOSIGNAL);
        
        close(acceptSock);
        return 0;
    }

    strcpy(getEnd, "");

    if ((startDir = strchr(fullResp, '/')) == NULL) {
        perror("this shouldnt happen .-.");
        printf("startDir: %s\n", startDir);

        //error 400: bad request
        send(acceptSock, "HTTP/1.0 400\r\n\r\n", 16, MSG_NOSIGNAL);

        bzero(fullResp, sizeof(fullResp));

        close(acceptSock);
        return 0;
    }



    //handles the file retrieving
    if ((endDir = strchr(startDir, ' ')) == NULL) {
        perror("endDir error");

        //error 500: internal server error
        send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

        bzero(fileDir, sizeof(fileDir));
        bzero(fullResp, sizeof(fullResp));

        close(acceptSock);
        return 0;
    }


    strcpy(endDir, "");


    
    //checks for requested directory
    if (strcmp(startDir, "/") == 0) {
        findExtention = ".html";
        strcpy(fileDir, webFileDir);
        strcat(fileDir, "index.html");
        responseCode = 200;
    } else {
        if ((findExtention = strchr(startDir, '.')) == NULL) {
            perror("invalid webpage");

            findExtention = ".html";
            strcpy(fileDir, servFileDir);
            strcat(fileDir, "err404.html");
            responseCode = 404;
        }

        strcat(fileDir, startDir);
    }


    if ((fpointer = fopen(fileDir, "rb")) != NULL) {
        fseek(fpointer, 0L, SEEK_END);
        fsize = ftell(fpointer);
    } else if (strcmp(startDir-1, "/favicon.ico") == 0 && access(fileDir, F_OK) != 0) {
        strcpy(fileDir, servFileDir);
        strcat(fileDir, "favicon.ico");

        if (access(fileDir, F_OK) != 0) {
            printf("\n\n\nERROR: please do not delete the default favicon.ico file. the program will not work properly if it is deleted\n\n\n");

            //error 500: internal server error
            send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

            exit(1);
        }
        
        if ((fpointer = fopen(fileDir, "rb")) == NULL) {
            perror("fopen error");

            //error 500: internal server error
            send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

            bzero(fullResp, sizeof(fullResp));

            close(acceptSock);
            return 0;
        }

        fseek(fpointer, 0L, SEEK_END);
        fsize = ftell(fpointer);
    } else if (access(fileDir, F_OK) != 0) {
        perror("webpage doesnt exist");
        
        findExtention = ".html";
        strcpy(fileDir, servFileDir);
        strcat(fileDir, "err404.html");
        responseCode = 404;
        if ((fpointer = fopen(fileDir, "r")) == NULL) {
            perror("fopen error");

            //error 500: internal server error
            send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

            bzero(fullResp, sizeof(fullResp));

            close(acceptSock);
            return 0;
        }
        fseek(fpointer, 0L, SEEK_END);
        fsize = ftell(fpointer);
    }


    fclose(fpointer);



    //sets the server http response
    if ((strcmp(findExtention, ".jpeg")) == 0 || (strcmp(findExtention, ".jpg")) == 0) {
        bin = 1;
        sprintf(httpResponse, "HTTP/1.0 %d\r\nConnection: close\r\nContent-Type: image/jpeg\r\nContent-Length: %d\r\nContent-Transfer-Encoding: binary\r\n\r\n", responseCode, fsize);
    } else if ((strcmp(findExtention, ".png")) == 0) {
        bin = 1;
        sprintf(httpResponse, "HTTP/1.0 %d\r\nConnection: close\r\nContent-Type: image/png\r\nContent-Length: %d\r\nContent-Transfer-Encoding: binary\r\n\r\n", responseCode, fsize);
    } else if ((strcmp(findExtention, ".ico")) == 0) {
        bin = 1;
        sprintf(httpResponse, "HTTP/1.0 %d\r\nConnection: close\r\nContent-Type: image/x-icon\r\nContent-Length: %d\r\nContent-Transfer-Encoding: binary\r\n\r\n", responseCode, fsize);
    } else if ((strcmp(findExtention, ".mp3")) == 0) {
        bin = 1;
        sprintf(httpResponse, "HTTP/1.0 %d\r\nConnection: close\r\nContent-Type: audio/mpeg\r\nContent-length: %d\r\nContent-Transfer-Encoding: binary\r\n\r\n", responseCode, fsize);
    } else if ((strcmp(findExtention, ".html")) == 0) {
        bin = 0;
        sprintf(httpResponse, "HTTP/1.0 %d\r\nConnection: close\r\nContent-Type: text/html\r\nContent-Length: %d\r\n\r\n", responseCode, fsize);
    } else {
        strcpy(fileDir, "err404.html");
        if ((fpointer = fopen(fileDir, "r")) == NULL) {
            perror("fopen error");

            bzero(fullResp, sizeof(fullResp));

            close(acceptSock);
            return 0;
        }
        fseek(fpointer, 0L, SEEK_END);
        fsize = ftell(fpointer);
        fclose(fpointer);
        bin = 0;
        sprintf(httpResponse, "HTTP/1.0 200 OK\r\nContent-Type: text/html\r\nContent-Length: %d\r\n\r\n", fsize);
    }

    strcpy(httpResponseReport, httpResponse);



    if (readSock < 0) {
        perror("readsock is less than 0");

        //error 500: internal server error
        send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

        bzero(fullResp, sizeof(fullResp));
        bzero(fileDir, sizeof(fileDir));
        bzero(httpResponseReport, sizeof(httpResponseReport));
        bzero(httpResponse, sizeof(httpResponse));

        close(acceptSock);
        return 0;
    }


    //checks if i need to read plaintext or binary
    if (bin == 0) {
        fpointer = fopen(fileDir, "r");
    } else if (bin == 1) {
        fpointer = fopen(fileDir, "rb");
    }


    if (fpointer == NULL) {
        perror("file open error");

        //error 500: internal server error
        send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

        bzero(fullResp, sizeof(fullResp));
        bzero(fileDir, sizeof(fileDir));
        bzero(httpResponseReport, sizeof(httpResponseReport));
        bzero(httpResponse, sizeof(httpResponse));

        close(acceptSock);
        return 0;
    }

    



    //sends server http response to client
    fseek(fpointer, 0L, SEEK_END);
    fsize = ftell(fpointer);
    fclose(fpointer);




    //checks if i need to read plaintext or binary (again)
    if (bin == 0) {
        fpointer = fopen(fileDir, "r");
    } else if (bin == 1) {
        fpointer = fopen(fileDir, "rb");
    }

    if (fpointer == NULL) {
        perror("fopen error");

        //error 500: internal server error
        send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

        bzero(fullResp, sizeof(fullResp));
        bzero(fileDir, sizeof(fileDir));
        bzero(httpResponse, sizeof(httpResponse));
        bzero(httpResponseReport, sizeof(httpResponseReport));

        close(acceptSock);
        return 0;
    }

    fileLine = malloc(fsize * sizeof(char));

    

    while ((freadErr = fread(fileLine, fsize, fsize, fpointer)) > 0);
    
    if (feof(fpointer) < 0) {
        perror("fread error");

        //error 500: internal server error
        send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

        bzero(fullResp, sizeof(fullResp));
        bzero(fileDir, sizeof(fileDir));
        bzero(httpResponseReport, sizeof(httpResponseReport));
        bzero(httpResponse, sizeof(httpResponse));

        close(acceptSock);
        return 0;
    }



    //set 5 second socket timeout
    struct timeval timeout;
    timeout.tv_sec = 15;
    timeout.tv_usec = 0;

    if (setsockopt(acceptSock, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout)) < 0) {
        perror("setsockopt error");

        //error 500: internal server error
        send(acceptSock, "HTTP/1.0 500\r\n\r\n", 16, MSG_NOSIGNAL);

        close(acceptSock);
        return 0;
    }


    while (1) {
        if ((checkSend = send(acceptSock, httpResponse, strlen(httpResponse), MSG_NOSIGNAL)) == -1) {
            break;
        }

        //send full response
        if ((checkSend = send(acceptSock, fileLine, fsize, MSG_NOSIGNAL)) == -1) {
            break;
        }

        sleep(1);
    }

    

    bzero(httpResponse, sizeof(httpResponse));
    bzero(fullResp, sizeof(fullResp));


    fclose(fpointer);
    close(acceptSock);
    free(fileLine);



    //handles the clearing of variables and logs client ip and requested directory
    printf("\nclient with the ip: %s has requested %s.\n", clientIP, fileDir);//added server response but i don't think i need it anymore

    bzero(fileDir, sizeof(fileDir));
    bzero(httpResponseReport, sizeof(httpResponseReport));
    return 0;
}



//assign work to each thread
void *giveThreadWork() {
    while (1) {
        int pclient;

        pthread_mutex_lock(&mutex);
        //makes thread wait until signaled
        while ((pclient = dequeue()) == -1) {
            pthread_cond_wait(&condition_var, &mutex);
        }
        pthread_mutex_unlock(&mutex);
        
        handleClient(pclient);
    }
}


int main() {
    int clientSock;
    int sock;
    int portNum;
    int defaultOrNo;




    printf("\n\n\nWeb Server\nBy: Rowan Rothe\n\n\n");

    printf("would you like to use default directories (1 = yes, 0 = no): ");
    scanf("%d", &defaultOrNo);

    if (defaultOrNo == 0) {
        printf("enter the directory of the files to be served (with '/' at the end): ");
        scanf("%s", webFileDir);

        printf("enter the directory of the web server folder (with '/' at the end): ");
        scanf("%s", servFileDir);
    } else if (defaultOrNo == 1) {
        strcpy(webFileDir, defaultDir1);
        strcpy(servFileDir, defaultDir2);
    }

    printf("what port would you like to host the site on?: ");
    scanf("%d", &portNum);

    sock = socket(AF_INET, SOCK_STREAM, 0);

    if (sock < 0) {
        perror("sock error");
    }



    sockaddr.sin_family = AF_INET;
    sockaddr.sin_addr.s_addr = htonl(INADDR_ANY);
    sockaddr.sin_port = htons(portNum);

    if ((bind(sock, (struct sockaddr *) &sockaddr, sizeof(sockaddr))) < 0) {
        perror("bind error");
        exit(1);
    }

    printf("socket bind success\n");

    //create all the threads for the thread pool
    for (int i = 0; i < THREAD_POOL_SIZE; i++) {
        pthread_create(&thread_pool[i], NULL, giveThreadWork, NULL);
    }

    printf("created thread pool of size %d successfully\n", THREAD_POOL_SIZE);

    if ((listen(sock, BACKLOG)) < 0) {
        perror("listen error");
    }

    printf("listen success\n");




    while (1) {
        //checks for client connection
        if ((clientSock = accept(sock, NULL, NULL)) < 0) {
            perror("accept error");
        } else {
            pthread_mutex_lock(&mutex);
            enqueue(clientSock);
            pthread_cond_signal(&condition_var);
            pthread_mutex_unlock(&mutex);
        }
    }

    return 0;
}

Answer 1

Calling strcat in a loop is inefficient

The function strcat works the following way:

It must first search the entire destination string until it finds the terminating null character at the end. It then appends the source string.

Therefore, calling the function strcat in a loop in order to append one character at a time is highly inefficient. This is because whenever you append a single character, it must search the entire destination string again for the terminating null character. (See Schlemiel the Painter's Algorithm for more information.)

If you want to add a single character at a time, then you should not use the function strcat, but should rather remember the position of the end of the string (for example using a pointer or by remembering the length of the string) and add the new character yourself, by overwriting the terminating null character with that character. You can add a new terminating null character at the end of the string after you have finished adding all characters. Or, if you need the character sequence to be a valid null-terminated string after every intermediate step, then you can add a new null-terminating character after every character added.

System calls are expensive

The function read is not a simple function call. It is a system call which must be handled by the operating system's kernel. The overhead of this system call is probably somewhere between 1000 and 3000 CPU cycles. See this question for more information. Also, see the comments section of my answer for some further comments regarding recent increase in system call overhead due to Meltdown/Spectre mitigations.

Therefore, it is a big waste of CPU resources to call this function once for every single character, instead of attempting to read as much as you can at once, using a single system call.

Zeroing large amounts of memory is inefficient

You are using the function bzero to zero out large amounts of memory in several places in your program. It seems that you are zeroing out more than 100 KB of memory for every HTTP request.

While it is not much of a problem to do this once at program startup, doing this for every HTTP request seems excessive.

Also, it is unclear why you are zeroing out memory, in particular whether you are doing this

1. to ensure that all strings are null-terminated, or
2. to erase sensitive data, such as passwords.

If it is #1, then there are other more efficient ways to ensure that strings are always null-terminated.

If it is #2, then you should be using explicit_bzero instead of bzero, because it is possible that bzero will be optimized away by the compiler.

Not guarding against buffer overflows

You don't seem to be guarding against buffer overflows in your program. This means that a malicious client could crash your server by sending a request in which the first line is larger than 100 KB.

This loop is inefficient

For the reasons stated in the previous 4 sections, this loop is inefficient and prone to a buffer overflow:

fullResp[0] = '\0';
//handles reading client http request
while (1) {
    if ((readSock = read(acceptSock, recvLine, 1)) != 1) {
        //handle error (code omitted)
    }
    recvLine[1] = '\0';
    strcat(fullResp, recvLine);
    if ((endOfPost = strstr(fullResp, "\n")) != NULL) {
        break;
    } else if (readSock < 0) {
        //handle error (code omitted)
    }
    bzero(recvLine, sizeof(recvLine));
}

Also, after every character added, you are searching the entire string again for the newline character, although it would only be necessary to check the last character added.

However, since you are only processing a single line with this inefficient loop, it probably won't matter much. But if a (possibly malicious) client sends you a very long line, your server will spend a significant amount of time processing this line. This vulnerability could be used in a DoS attack against your server.

Also, the variable readSock does not seem appropriately named, as it implies that it contains the value of a socket file descriptor.

An efficient version of the loop, which also protects against buffer overflow, would look like this:

//fullResp[0] = '\0'; //this line is no longer needed
size_t offset = 0;

do {
    char *p_read_buf = fullResp + offset;
    size_t to_read = sizeof fullResp - offset - 1;

    if ( to_read == 0 ) {
        //handle error (code omitted)
    }

    if ( ( readSock = read( acceptSock, p_read_buf, to_read ) ) <= 0 ) {
        //handle error (code omitted)
    }

    offset += readSock;

} while ( ( endOfPost = memchr( p_read_buf, '\n', readSock ) ) == NULL );

//add null terminating character
fullResp[offset] = '\0';

Rewind files instead of closing and reopening them

You seem to be opening files, in order to determine their length, and then closing them again, only to reopen them again shortly afterwards. This does not seem meaningful. You can use the function rewind to reset the file position indicator to the start of the file.

This loop seems to be buggy

The following loop appears to be buggy:

while (1) {
    if ((checkSend = send(acceptSock, httpResponse, strlen(httpResponse), MSG_NOSIGNAL)) == -1) {
        break;
    }

    //send full response
    if ((checkSend = send(acceptSock, fileLine, fsize, MSG_NOSIGNAL)) == -1) {
        break;
    }

    sleep(1);
}

It is unclear what this loop is attempting to accomplish.

You seem to be first sending the HTTP response header, then the HTTP response body. However, it is unclear why you are doing this in an infinite loop.

Also, it should not be necessary to call sleep(1) in this loop. If your program does not behave properly without that call, then this is a sign of a bug in your program.

My guess is that you need this wait in order to give the client time to close the connection, so that the first call to send in the second iteration of the loop fails, which causes the loop to break. Otherwise, if the client does not close the connection, you will send the HTTP response to the client in an infinite loop. However, this is not the proper way of solving the problem.

The proper way to solve this is to not use an infinite loop:

//send HTTP response header
size_t len = strlen( httpReponse );
if ( ( checkSend = send( acceptSock, httpResponse, len, MSG_NOSIGNAL) ) != len ) {
    //handle error (code omitted)
}

//send HTTP response body
if ( ( checkSend = send(acceptSock, fileLine, fsize, MSG_NOSIGNAL ) ) != fsize ) {
    //handle error (code omitted)
}

Your usage of fread is wrong

The line

while ((freadErr = fread(fileLine, fsize, fsize, fpointer)) > 0);

is wrong. You are instructing fread to attempt to read fsize items each of size fsize bytes, i.e. a total of fsize*fsize bytes. Also, the while loop does not make sense.

You should write either

if ( ( freadErr = fread( fileLine, fsize, 1, fpointer ) ) != 1 )
{
    //handle error (code omitted)
}

or this:

if ( ( freadErr = fread( fileLine, 1, fsize, fpointer ) ) != fsize )
{
    //handle error (code omitted)
}

Your usage of feof is wrong

The line

if (feof(fpointer) < 0) {

does not make sense.

The function feof will return a nonzero value (i.e. true) if the end-of-file indicator has been set on the stream, otherwise zero (i.e. false). Therefore, comparing the returned value with < 0 does not make sense.

Also, the function feof does not check the file position indicator itself. Instead, it merely checks whether the end-of-file indicator on the stream has been set. This end-of-file indicator will only be set if a previous I/O function attempted to read past the end of the file. The end-of-file indicator will not be set if the file position indicator has merely reached the end of the file, so that the next input operation will fail.

For this reason, it does not make sense to call feof. You should check the return value of the previous I/O operation instead (which you are already doing). Only if that I/O operation reports an error or does not read all of the requested input, will it make sense to call feof or ferror to determine the cause.

Redundant if statement

In the function

int dequeue() {
    if (head == NULL) return -1;//if the head is empty or nothing has been enqueued then return null


    if (head != NULL) { //else...
        int result = head->client_socket;
        node_t *temp = head;
        head = head->next;
        if (head == NULL) {tail = NULL;}
        free(temp);
        return result;
    }
    //this function removes everything off of the queue and returns the socket
    return 0;
}

the second if statement is redundant and the return 0 in the last line is unreachable. You can simply write:

int dequeue() {
    if (head == NULL) return -1;//if the head is empty or nothing has been enqueued then return null

    int result = head->client_socket;
    node_t *temp = head;
    head = head->next;
    if (head == NULL) {tail = NULL;}
    free(temp);
    return result;
}

Answer 2

I didn't read the whole code thing, just looked at some parts. Also, Andreas already mentioned some of the major things, like making one system call per byte being a performance disaster.

---

Some of those local arrays, especially char fullResp[100000];, are a bit large to keep on the stack. Most mainstream systems give you at least 1MiB of stack space (e.g. Windows, with Linux defaulting to a ulimit -s of 8MiB which pthreads uses when allocating thread stacks). So 100k is fine here, where that's the only really big thing, and you don't have a large stack depth of other function calls.

Since this is multithreaded, you can't just make it static, although you could in theory make it thread-local static like static __thread char fullResp[100000];. But if you want to avoid stack space, probably better to dynamically allocate it with calloc/free. But you'd probably want to avoid alloc/free on every request, perhaps allocating the buffer in giveThreadWork and passing it as an arg to handleClient (although that's a pretty ugly optimization). 100k is small enough that it's not painful for each thread to keep that much allocated. (If you never make a huge response, some of those pages will never be dirtied, if you avoid naive bzero of the whole thing.)

---

strstr(fullResp, "\n") is very silly. strchr is specifically optimized for searching for a single char. Or better, keep track of the used length in the buffer so you can avoid str functions that scan from the beginning every time. Actually, this is inside your loop that reads and appends 1 char at a time inefficiently (as discussed in @Andreas Wenzel's answer) with strcat, so you could just be checking recvLine[0] == '\n'.

You assign the strstr return value to endOfPost but never use that variable (except in the if( (endOfPost = strstr()) ) where it's assigned), so it should be removed.

For cases where you do actually want to concatenate multiple strings whose length isn't known to be 1, you can use POSIX / GNU stpcpy.

Unfortunately ISO C str functions all suck and don't help you avoid redundant work scanning for a terminating 0 in the string. e.g. strcpy and strcat return the start of the string, not the new end. And strlen + memcpy would scan the source string twice. See strcpy() return value for some C ancient history about where those poorly designed functions came from, and details about stpcpy.

---

    //handles the file retrieving
    if ((endDir = strchr(startDir, ' ')) == NULL) {
        perror("endDir error");

strchr doesn't set errno; there's no meaningful strerror() error string that perror can append to this. It will either be ": Success" or some old error code.

---

It's redundant to check access(fileDir, F_OK) if you're about to open it (like in the favico fallback path), or after a failed fopen(). If you want to know why an open failed, check errno. On POSIX systems, fopen() failure is guaranteed to set errno as open() would have (cppreference). So use strerror or copy errno early, before doing other system calls that might overwrite errno again.

Your use of access introduces a TOCTOU problem (Time-of-check vs. Time-of-use). The file could be deleted, created, or change permission, between fopen() and access().

If you're checking before open, opening can still fail. That makes it hard to test the failed-open path of execution because it normally doesn't happen. Also as discussed on SO, access uses your "real" UID/GID, not the current effective UID/GID, which matters for setuid root programs. (Such as a web server that opens port 80 and then drops root privileges.)

---

Instead of fseek/ftell (+ rewind) to find file length, use POSIX fstat (With a file descriptor from fileno if you want to keep using C stdio). Your code is already using POSIX pthreads and the POSIX socket API, so other POSIX functions are fine. (Although arguably you could keep the threading part OS-specific or switch to C11 threads.h and keep the rest of the code using pure ISO C as much as possible.)

But fseek/ftell isn't portably guaranteed to give you file lengths. (That SO link correctly explains why, despite not being the answer to the question it's posted under.) ftell on a binary stream isn't guaranteed to be the length you can actually read, and ftell on a non-binary stream is not guaranteed to have any particular meaning.
It does happen to work (or is in fact well defined) on most systems, and is a common idiom.

However, ftell returns a long which is 32-bit on some systems (including 32-bit systems running Linux, and x86-64 Windows), so it won't handle large files correctly there. ftello returns an off_t, but as per other discussion on SO, ftello isn't available under that name on Windows despite being specified by ISO C.

---

stat(2) / fstat(2) work fine on regular files, and you can probably assume you aren't serving up block device files (which do have a size, unlike a pipe or socket, but not one that stat reports.) Note that using fstat avoids some TOCTOU problems and is more efficient (only one path lookup by the kernel), but if the file length is changing while the server is running, reads could still eventually get more data from the file than the reported size.

See How can I get a file's size in C? on Stack Overflow.

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>

    struct stat buf;
    fstat(fileno(fp), &buf);
    off_t size = buf.st_size;

Answer 3

Never do this:

    node_t *newnode = malloc(sizeof(node_t)); //create node
    newnode->client_socket = client_socket; //assign client socket

If malloc() is unable to allocate memory (e.g. if the process's limit is reached), it will return a null pointer. Dereferencing that pointer with -> is Undefined Behaviour: if you're lucky, it will just crash your service and only be a denial of service attack. If you're less lucky, absolutely anything could be the result.

You should be much more defensive with this (and the other allocation):

    node_t *newnode = malloc(sizeof *newnode);
    if (!newnode) { return false; } /* and make sure the caller checks this! */
    newnode->client_socket = client_socket;

This level of robustness is vital in any network-facing code, because it's exposed to the most hostile of input!