Simple webserver in C

Question

My first learning project in C. Looking for general feedback e.g. gotchas, coding standards, formatting, naming etc. I'm here to learn!

webserver.h

#ifndef __Webserver__webserver__
#define __Webserver__webserver__

#include <stdio.h>

struct webserver_connection {
    int status;
    int socket;
    char *path;
};

struct webserver_connection webserver_connect(int port_number, char *root);
void webserver_listen(struct webserver_connection);
void webserver_disconnect(struct webserver_connection);

#endif /* defined(__Webserver__webserver__) */

webserver.c

#include "webserver.h"
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/select.h>
#include <sys/un.h>
#include <sys/stat.h>
#include <netinet/in.h>
#include <string.h>
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <fcntl.h>

struct webserver_request {
    enum {REQUEST_POST, REQUEST_GET} method;
    char *uri;
    char *host;
};

struct webserver_response {
    int status;
    char *body;
    unsigned long body_length;
    char *content_type;
    char *content_encoding;
};

int webserver_parse_lines(int client_socket, struct webserver_request *request);
int webserver_parse_line(char request_line[], struct webserver_request *request);
int webserver_accept_connection(struct webserver_connection server_connection);
int webserver_handle_connection(struct webserver_connection, int client_socket);
int webserver_respond(int client_socket, struct webserver_response *response);
void webserver_respond_with_error(int client_socket, int http_status);
int webserver_http_message(int http_status, char **http_message);
int webserver_get_content(char* root_path, struct webserver_request *request, char **file_content, unsigned long *file_length);
char* webserver_content_type_from_filepath(char *file_path);
void webserver_log_request(struct webserver_request *request, int http_status);

struct webserver_connection webserver_connect(int port_number, char *root) {

    struct webserver_connection server_connection;
    server_connection.status = 0;
    server_connection.socket = socket(PF_INET, SOCK_STREAM, 0);

    if (setsockopt(server_connection.socket, SOL_SOCKET, SO_REUSEADDR, &(int) { 1 }, sizeof(int)) < 0) {

        server_connection.status = -1;
        return server_connection;
    }

    server_connection.path = malloc(strlen(root) + 1);
    strcpy(server_connection.path, root);

    struct sockaddr_in socket_address;
    socket_address.sin_family = AF_INET;
    socket_address.sin_port = htons(port_number);
    socket_address.sin_addr.s_addr = htonl(INADDR_ANY);

    int length = sizeof(socket_address);
    if (bind(server_connection.socket, (struct sockaddr*) &socket_address, length) < 0) {

        server_connection.status = -1;
        return server_connection;
    }

    if (listen(server_connection.socket, 5) < 0) {
        server_connection.status = -1;
    }

    return server_connection;
};

void webserver_disconnect(struct webserver_connection server_connection) {
    close(server_connection.socket);
}

void webserver_listen(struct webserver_connection server_connection) {

    printf("Starts listening for new connections...\n");

    while (1) {

        struct timeval timeout;
        timeout.tv_sec = 1;
        timeout.tv_usec = 0;
        fd_set set;

        FD_ZERO(&set);
        FD_SET(server_connection.socket, &set);

        int status = select(server_connection.socket + 1, &set, NULL, NULL, &timeout);
        if (status > 0) {
            webserver_accept_connection(server_connection);
        }
    }
}

int webserver_accept_connection(struct webserver_connection server_connection) {

    int client_socket;
    struct sockaddr_in client_address;
    socklen_t client_length;

    client_socket = accept(server_connection.socket, (struct sockaddr*) &client_address, &client_length);
    if (client_socket < 0) {
        return -1;
    }

    pid_t pid = fork();
    if (pid == 0) {

        webserver_handle_connection(server_connection, client_socket);
        exit(0);
    } else {
        close(client_socket);
    }

    return 0;
}

int webserver_handle_connection(struct webserver_connection server_connection, int client_socket) {

    struct webserver_request request;
    int status;

    status = webserver_parse_lines(client_socket, &request);
    if (status == 0) {

        char *file_content;
        unsigned long *file_length = malloc(sizeof(unsigned long));

        status = webserver_get_content(server_connection.path, &request, &file_content, file_length);

        if (status == 0) {

            status = 200;
            char *content_type = webserver_content_type_from_filepath(request.uri);

            struct webserver_response response;
            response.status = status;
            response.body = file_content;
            response.body_length = *file_length;
            response.content_type = content_type;
            response.content_encoding = NULL;

            if (strcmp(content_type, "text/html") == 0) {
                response.content_encoding = strdup("utf-8");
            }

            webserver_respond(client_socket, &response);

            free(content_type);
        } else if (status > 0) {
            webserver_respond_with_error(client_socket, status);
        } else {
            webserver_respond_with_error(client_socket, 500);
        }

        free(file_content);
        free(file_length);
    }

    webserver_log_request(&request, status);

    close(client_socket);

    return 0;
}

int webserver_parse_lines(int client_socket, struct webserver_request *request) {

    char request_line[1024];

    while (1) {

        bzero(request_line, 1024);
        if (read(client_socket, request_line, 1023) < 0) {
            return -1;
        }

        int status = webserver_parse_line(request_line, request);

        if (status > 0) {
            webserver_respond_with_error(client_socket, status);
        } else {
            return status;
        }
    }

    return 0;
}

int webserver_parse_line(char request_line[], struct webserver_request *request) {

    char *token;
    int token_counter = 0;
    token = strtok(request_line, " ");

    while (token != NULL) {

        if (token_counter == 0) {

            if (strcmp(token, "GET") == 0) {
                request->method = REQUEST_GET;
            } else {
                return 400;
            }
        } else if (token_counter == 1) {

            request->uri = malloc(strlen(token));
            strcpy(request->uri, token);
        }

        token = strtok(NULL, " ");
        token_counter++;
    }

    free(token);

    return 0;
}

int webserver_http_message(int http_status, char **http_message) {

    *http_message = malloc(30);

    switch (http_status) {
        case 200:

            strcpy(*http_message, "OK");
            break;

        case 201:

            strcpy(*http_message, "Created");
            break;

        case 400:

            strcpy(*http_message, "Bad Request");
            break;

        case 401:

            strcpy(*http_message, "Unauthorized");
            break;

        case 403:

            strcpy(*http_message, "Forbidden");
            break;

        case 404:

            strcpy(*http_message, "Not Found");
            break;

        case 405:

            strcpy(*http_message, "Method Not Allowed");
            break;

        case 500:

            strcpy(*http_message, "Internal Server Error");
            break;

        default:

            return -1;
            break;
    }

    return 0;
}

int webserver_respond(int client_socket, struct webserver_response *response) {

    char *http_status_message;
    char response_headers[200];

    webserver_http_message(response->status, &http_status_message);
    size_t content_type_length = strlen(response->content_type + 11);
    if (response->content_encoding != NULL) {
        content_type_length += strlen(response->content_encoding);
    }

    char *content_type = malloc(content_type_length);
    strcpy(content_type, response->content_type);

    if (response->content_encoding != NULL) {

        strcat(content_type, "; charset=");
        strcat(content_type, response->content_encoding);
    }

    sprintf(
        response_headers,
        "HTTP/1.0 %d %s\nContent-Type: %s\nContent-Length: %lu\nServer: Webserver C\r\n\r\n",
        response->status,
        http_status_message,
        content_type,
        response->body_length
    );

    write(client_socket, response_headers, strlen(response_headers));
    write(client_socket, response->body, response->body_length);
    write(client_socket, "\r\n", strlen("\r\n"));

    free(http_status_message);
    free(content_type);

    return 0;
}

void webserver_respond_with_error(int client_socket, int http_status) {

    char *message;
    char *html = malloc(100);

    webserver_http_message(http_status, &message);
    sprintf(html, "<html><body>%s</body></html>", message);

    struct webserver_response response;
    response.status = http_status;
    response.body = html;
    response.body_length = strlen(html);
    response.content_type = strdup("text/html");
    response.content_encoding = strdup("utf-8");

    webserver_respond(client_socket, &response);

    free(message);
    free(html);
}

int webserver_get_content(char *root_path, struct webserver_request *request, char **file_content, unsigned long *file_length) {

    char *file_path = malloc(strlen(root_path) + strlen(request->uri) + 1);
    strcpy(file_path, root_path);
    strcat(file_path, request->uri);

    *file_content = malloc(0);

    if (access(file_path, F_OK) == -1) {
        return 404;
    }

    struct stat s;
    if (stat(file_path, &s) == 0) {
        if (s.st_mode & S_IFDIR) {
            return 404;
        }
    } else {
        return 404;
    }

    FILE *file;
    file = fopen(file_path, "rb");
    if (!file) {
        return -1;
    }

    fseek(file, 0, SEEK_END);
    *file_length = ftell(file);
    fseek(file, 0, SEEK_SET);

    free(*file_content);
    *file_content = malloc(*file_length + 1);
    fread(*file_content, 1, *file_length, file);

    fclose(file);

    return 0;
}

char* webserver_content_type_from_filepath(char *file_path) {

    char *file_path_copy = strdup(file_path);
    char *token;
    char *extension;
    char *content_type = malloc(20);

    extension = malloc(0);
    while ((token = strsep(&file_path_copy, ".")) != NULL) {

        free(extension);
        extension = strdup(token);
    }

    free(file_path_copy);
    free(token);

    if (strcmp(extension, "htm") == 0 || strcmp(extension, "html") == 0) {
        strcpy(content_type, "text/html");
    } else if (strcmp(extension, "jpg") == 0) {
        strcpy(content_type, "image/jpeg");
    } else if (strcmp(extension, "png") == 0) {
        strcpy(content_type, "image/png");
    } else if (strcmp(extension, "gif") == 0) {
        strcpy(content_type, "image/gif");
    }

    return content_type;
}

void webserver_log_request(struct webserver_request *request, int http_status) {

    char method[5];

    switch (request->method) {
        case REQUEST_GET:

            strcpy(method, "GET");
            break;

        case REQUEST_POST:

            strcpy(method, "POST");
            break;
    }

    printf("%s %s %d\n", method, request->uri, http_status);
}

main.c

#include "webserver.h"
#include <signal.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>

int main(int argc, const char * argv[]) {

    struct webserver_connection connection;
    connection = webserver_connect(8888, "/path/to/www-root");

    if (connection.status != 0) {

        perror("Webserver");
        return EXIT_FAILURE;
    }

    webserver_listen(connection);

    return EXIT_SUCCESS;
}

Answer 1

Are you sure that &(int) { 1 } does what you want it to do? I'm not sure it does.

Using malloc()/strcpy() is probably better handled with strdup() (which you've used elsewhere, so you're already okay with that).

In webserver_handle_connection, you're using malloc() to allocate space for an unsigned long. Why? Declare a normal local variable, and pass its address to webserver_get_content.

In webserver_parse_line, you have a free(token); statement in there. That statement doesn't do anything because token is already known to be NULL at the point it is reached. Also, there's no need to free anything when using strtok() because that function returns pointers into your buffer, not newly allocated memory. (This error occurs in a couple of places.)

In webserver_respond, you're sending an extra "\r\n" after the HTTP response body. This is a violation of the HTTP protocol and should not be sent.

In webserver_respond_with_error, you are using sprintf() with a (possibly) unknown string, printing into a fixed size buffer. What happens if the resulting string is 100 characters or more? You should first use snprintf() instead so you don't overwrite more than your buffer has allocated, and second you should work out how much space you actually need to allocate in the first place based on the message.

In webserver_get_content, often when a webserver gets a request for a directory name without a trailing slash, it will return an HTTP Redirect so that the client will resubmit the request with a trailing slash, so that relative links in the returned HTTP page go to the correct place.

In webserver_get_content, you're calling malloc(0) and then freeing that pointer later. That doesn't seem to make sense. Why? Also, loading the entire contents of a file into memory is reasonable for small files, but quite unreasonable for large files.

There's a data leak race condition when reading the file data - if the file is changed (say, truncated) between the time you get the size and the time you read the data, then you'll end up sending the (uninitialised) contents of memory space allocated by malloc(). This could have unintended security consequences.

In webserver_content_type_from_filepath, you've again got that pattern of malloc(0), then free(...), then malloc(...) again. Why?

In webserver_log_request, there is some code that is indicative of a pattern I've seen throughout your code. You've got a local buffer method (fine), which you strcpy() some data into and then use. There's no need to do this unnecessary copying. You can instead do:

const char *method = NULL; // initialise the pointer (good practice)

switch (request->method) {
    case REQUEST_GET:
        method = "GET";
        break;

Then, you can use method as usual. I've declared it as const char *, which is the type of a literal string (assuming you're using a reasonably modern C compiler). (I notice that you're not using any const at all. Your code would definitely benefit from that.)

In other places in your code (such as the handling of content_encoding), you do something similar where you strdup("utf-8") when you could be just assigning a pointer. Also, you fail to free() the content_encoding anywhere so you have a memory leak.

Answer 2

There's no documentation whatsoever in your code. I know being a web server is self-explanatory but might not be for everyone. Also, it is recommended that all 'struct' definitions to go into the header file. The header file should have enough information so a that you don't even need to look into its '.c' counterpart/header-implementation.

Answer 3

In addition to the malloc() etc related stuff in Greg's answer, there's actually no need whatsoever to use select() when you're only handling a single (blocking) file descriptor.