HTTP server that evaluates arithmetic expressions

Question

I made a simple web server in C, and I would like to have your opinion on my code quality. This would help me to improve.

#include "signal.h"
#include "string.h"
#include "sys/types.h"
#include "unistd.h"
#include "sys/socket.h"
#include "netinet/in.h"
#include "arpa/inet.h"
#include "netdb.h"
#include "stdlib.h"
#include "ctype.h"
#include "stdio.h"
#include "utility.h"
#include "route.c"
#include "tokenize.c"
#include "bridge.c"




#define PORT 4775
#define ADDRESS "127.0.0.1"
#define START  "start"
#define  STOP  "stop"
#define LISTENQ 4

typedef struct sockaddr SA;
//#define INET6_ADDRSTRLEN 3000
requested_data *re_data;

int switch_ch;
struct sockaddr_in serveraddr;

int start_server(int port){
    int listenid;
    int optval=1;
    typedef struct sockaddr SA;
    if((listenid = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
        error_message("Error in startServer <Failed to create socket>");
    }
    if (setsockopt(listenid, SOL_SOCKET, SO_REUSEADDR,
                   (const void *)&optval , sizeof(int)) < 0)
            return -1;
    memset(&serveraddr, 0, sizeof(serveraddr));
    serveraddr.sin_family = AF_INET;
    serveraddr.sin_addr.s_addr = htonl(INADDR_ANY);
    serveraddr.sin_port = htons((unsigned short)port);
    if(bind(listenid, (SA*)&serveraddr, sizeof(serveraddr)) < 0) {
        error_message("Error in startServer <Failed to bind socket>");
    }
    if(listen(listenid, LISTENQ) < 0) {
        error_message("Error in startServer <Failed to listen>");
    }
    
    
    return listenid;
}
    

void *get_in_addr(struct sockaddr *sa)
{
    if (sa->sa_family == AF_INET) {
        return &(((struct sockaddr_in*)sa)->sin_addr);
    }

    return &(((struct sockaddr_in6*)sa)->sin6_addr);
}

void process_server(int listenid){
    char remoteIP[INET6_ADDRSTRLEN];
    int fd;
    int read_size;
    char *client_message = (char *)malloc(sizeof(char) * 1000);
    struct sockaddr_storage remoteaddr; // client address
    socklen_t addrlen;
    addrlen = sizeof remoteaddr;
    char *gen;
    int gen_int = 0;
    int supplied_answer;
    while((fd = accept(listenid, (struct sockaddr *)&remoteaddr, &addrlen))) {
        while((read_size = recv(fd, client_message, 1000, 0)) > 0) {
            printf("%s", client_message);
            re_data = router(client_message, read_size);
            printf("Method %s --url::%s=+++\n", re_data->method, re_data->url);
            switch_ch = 1;
            break;
                
        }
        printf("DE server: new connection from %s on "
                            "socket %d\n",
                            inet_ntop(remoteaddr.ss_family,
                                get_in_addr((struct sockaddr*)&remoteaddr),
                                remoteIP, INET6_ADDRSTRLEN),fd);
        str_query *rem_data;
        str_query *header = re_data->meta_data;
         while(header){
            if(strncasecmp(re_data->method, "PUT", 3) == 0){
                if(strncasecmp(re_data->url, "/answer", 7) == 0){
                    if(strlen(re_data->url) == 7){
                        if(strncasecmp(header->req_name, "question", 8) == 0){
                            
                            int res_int = parse_input(header->req_value);
                            
                            if(gen_int == res_int && supplied_answer == res_int){
                                send(fd, "HTTP status 200 OK" , 18, 0);
                            } else {
                                send(fd, "400 Bad Request" , 15, 0);
                            }
                            
                            printf("\n Resulted from computation  of %s  result :: %d\n", header->req_name, res_int);
                            
                        }
                    }
                    
                }
            }
            if(switch_ch){
                if(strncasecmp(re_data->url, "/question" , 9) == 0){
                    if(strncasecmp(re_data->method, "GET", 3) == 0){
                        gen = pmain();
                        send(fd, gen, strlen(gen), 0);
                        gen_int = parse_input(gen);
                        switch_ch = 0;
                        printf("\n Resulted from generated computation  of %s  result :: %d\n", header->req_name, gen_int);
                        free(gen);
                    }
                }
            }
            if(strncasecmp(header->req_name, "answer", 8) == 0){
                supplied_answer = atoi(header->req_value);
                
            }
            
            printf("\nname %s    value %s\n", header->req_name, header->req_value);
            rem_data = header;
            header = header->next;
            free(rem_data);
         
        }       
        free(re_data);
        close(fd);
    }
    return ;
    
}


int main(int argc, char **argv) {
    int port, pid, server_conn = 1;
    int internet_address_len = 15;
    char serAdd[internet_address_len];

    char strAction[7];
    if(argc == 1) {
        port = PORT;
        strcpy(serAdd, ADDRESS);
    } else if(argc == 2){
        port = atoi(argv[1]);
        strcpy(serAdd, ADDRESS);
    } else if(argc == 3) {
        port = atoi(argv[1]);
        int n = strlen(argv[2]);
        strncpy(serAdd, argv[2], n);
        if(n >= internet_address_len){
            error_message("Internet address is incorrect");
        }
        serAdd[n] = '\0';
    }
    printf("\n\tDE is a basic server built to carry out simple things\n"
            "\t-----    -----\n"
            "\t|    |   |\n"
            "\t|    |   -----\n"
            "\t|    |   |\n"
            "\t-----    -----\n"
           "\tMethods implemented [Get & Put]\n"
           "\tTo start type start\n"
            "\tTo stop type stop\nDE]");
    scanf("%s", strAction);
    if (strcmp(strAction, START) == 0){
        printf("Server %sed\n", START);
        int listenid = start_server(port);
        printf("listen on port %d & listening is %d\nDE]", port,listenid);
        signal(SIGPIPE, SIG_IGN);
        while(server_conn) {
            pid =fork();
            if(pid == 0) {
                process_server(listenid);
                break;              
            }
            scanf("%s", strAction);
            if (strcmp(strAction, STOP) == 0) {
                close(listenid);
                return 0;
            }

        }
        
    }

    return 0;
    
}

---

#include "string.h"
#include "stdio.h"
#include "stdlib.h"
#include "ctype.h"


#define ADD 1
#define MINUS 2
#define MULTIPLE 3 
#define REMAINDER 4
#define DIVIDE 5
#define OP 10
#define NUM 11
#define ERROR_NUM -10000

typedef  struct token{
    int type;
    int value;
    struct token *next;  
} token;

token *token_head = NULL;
token *token_tail = NULL;
token *t_k;
int change_error = 0;

static token *make_token(int val, int typ){
    t_k = (token *)malloc(sizeof(token));
    t_k->type = typ;
    t_k->value = val;
    t_k->next = NULL;
    return t_k;
}
    

static void add_to_list(token *tk){
    if(!token_head){
        token_head = tk;
        token_tail = tk;
    } else{
        token_tail->next = tk;
        token_tail = token_tail->next;
    }
}

/*
* private function 
* @params: takes a string
* @returns: nothing
* loops through the string and build tokens based on expression gramma
*/

static void  tokenize(char* source){
    char charTokens[] = "+-x/%";
    while(*source != '\0' ){
        if(strchr(charTokens,*source ) != NULL){
            if ( *source == '+'){
                add_to_list(make_token(ADD ,OP));
                source++;
            } else if(*source == '-'){    
                add_to_list(make_token(MINUS,OP)); 
                source++;
            } else if(*source == 'x'){
                add_to_list(make_token(MULTIPLE,OP));
                source++;
            } else if (*source == '/'){
                add_to_list(make_token(DIVIDE, OP));
                source++;
            } else if (*source == '%'){
                add_to_list(make_token(REMAINDER, OP));
                source++;
            }
        } else if (strncasecmp(source , "ADD", 3) == 0){
            add_to_list(make_token(ADD ,OP));
            source = source + 3;
        } else if (strncasecmp(source , "SUBSTRACT", 9) == 0){
            add_to_list(make_token(MINUS ,OP));
            source = source + 9;
        } else if (strncasecmp(source , "DIVIDE", 6) == 0){
            add_to_list(make_token(DIVIDE ,OP));
            source = source + 6;
        } else if (strncasecmp(source , "MULTIPLE", 8) == 0){
            add_to_list(make_token(MULTIPLE ,OP));
            source = source + 8;
        } else if(isspace(*source)){
            source++;
            while(isspace(*source)){ source++;}
        } else if (isdigit(*source)){
            int num = 0;
            num = num + *source - '0';
            source++;
            while(isdigit(*source)){
                num = num * 10 + *source - '0';
                source++;
            }
            add_to_list(make_token(num, NUM));
        } else if(isalpha(*source)) {
            source++;
            while(isalpha(*source)){ 
                source++;
            }
        } else if (*source == '?'){
            source++;
        } else {
            source++;
        }
        
    }

}

/*
 *Private function
 * @Params :list of tokens
 * @Return : integer
 * 
 * Add 3 to 56?
 * Sum 6 and 6
 * This will only demand a bit of adjustment of solve_expression
*/


static int solve_expression(token **itokens){
    token *tokens = *itokens;
    token *test_tk;
    test_tk = tokens;
    tokens = tokens->next;
    int left;
    if(test_tk->type == 11){
        left = test_tk->value;
        test_tk = tokens;
        tokens = tokens->next;
        switch(test_tk->value){
            case ADD:
                test_tk = tokens;
                return left + test_tk->value;
            case MINUS:
                test_tk = tokens;
                return left - test_tk->value;
            case MULTIPLE:
                test_tk = tokens;
                return left * test_tk->value;
            case REMAINDER:
                test_tk = tokens;
                return left % test_tk->value;
            case DIVIDE:
                test_tk = tokens;
                return left / test_tk->value;
            default:
                change_error = 1;
                return ERROR_NUM;
        }
        
    } else if(test_tk->type == 10){
        int opcode = test_tk->value;
        //left = test_tk->value;
        test_tk = tokens;
        tokens = tokens->next;
        switch(opcode){
            case ADD:
                return test_tk->value + tokens->value;
            case MINUS:
                return test_tk->value - tokens->value;
            case MULTIPLE:
                return test_tk->value * tokens->value;
            case REMAINDER:
                return test_tk->value / tokens->value;
            case DIVIDE:
                return test_tk->value % tokens->value;
            default:
                change_error = 1;
                return ERROR_NUM;
        }
        
        
    }
    
    change_error = 1;       
    return ERROR_NUM;   
        
}


/*
* Params: string
* Examples of acceptle expressions
* Add 3 to 56?
* Sum 6 and 6
* What is 80 x 8?
* Returns integer which is result of expression computation
*/
        

int parse_input(char *statement){
    tokenize(statement);
    token *dd;
    int ans = solve_expression(&token_head);
    dd = token_head;
    while(token_head){
        token_head = token_head->next;
        free(dd);
        dd = token_head;
    }
    
    return ans;
}           

---

#include "utility.h"
#include "stdlib.h"
#include "string.h"
#include "ctype.h"
#include "derror.c"


str_query *head = NULL;
str_query *tail = NULL;

str_query *header; 
str_query *tailer; 

str_query *ntemp;

int stag_set = 0;
void add_message(str_query *message) {
    if(!head){
        head = message;
        tail = message;
    } else {
        tail->next = message;
        tail = tail->next;
    }
    return;
}

void add_header_message(str_query *message) {
    if(!header){
        header = message;
        tailer = message;
        stag_set = 1;
    } else {
        tailer->next = message;
        tailer = tailer->next;
    }
    return;
}   

void create_message(const char *name, const char *value, int nlen, int vlen, int choice){
    ntemp = (str_query *)malloc(sizeof(str_query));
    ntemp->req_name = (char *)malloc(sizeof(char) * nlen);
    ntemp->req_value = (char *)malloc(sizeof(char) * vlen);
    strncpy(ntemp->req_name, name, nlen);
    ntemp->req_name[nlen] = '\0';
    strncpy(ntemp->req_value, value, vlen);
    ntemp->req_value[vlen] = '\0';
    if(!choice){
        add_header_message(ntemp);
    } else{
        add_header_message(ntemp);
    }
     
    return;
    
}

/*
 * @params: string and its size (integer)
 * @return: a pointer to struct
 * Resolves request header and url
*/


requested_data *router(char *message, int siz){
    char *temp;
    int j = 0;
    char c; 
    header = NULL;
    tailer = NULL;
    str_query *query;
    int method_size = 3;
    requested_data *ndata = (requested_data *)malloc(sizeof(requested_data));
    //ndata->dmessage = NULL;
    ndata->meta_data = NULL;
    int i = 0;
    while((c = *message ) != '\0'){
        while(isspace(*message )) { message++;}
        if(strncasecmp(message, "GET", method_size) == 0){
            strncpy(ndata->method, "GET", method_size);
            ndata->method[method_size] = '\0';
            message = message + 3;
            i = i + 3;
            continue;
        }
        if(strncasecmp(message, "PUT", method_size) == 0) {
            strncpy(ndata->method, "PUT", method_size);
            ndata->method[method_size] = '\0';
            message = message + 3;
            i = i + 3;
            continue;
        }
        if(c == '/') {
            temp = (char *)malloc(sizeof(char)*200);
            temp[j++] = c;
            i++;
            message++;
            while((c = *message) != ' '){
                if(isalnum(c)) {
                    temp[j++] = c;
                } else if (c == '?'){
                    strncpy(ndata->url, temp, j);
                    ndata->url[j] = '\0';
                    j = 0;
                    break;
                }
                message++;
                i++;
            }
            if(j != 0){
                strncpy(ndata->url, temp, j);
                ndata->url[j] = '\0';
                j = 0;
                continue;
            }
        }
        if(c == '?'){
            temp = (char *)malloc(sizeof(char)* 100);
            i++;
            int state_check = 0;
            message++;
            j = 0;
            query = (str_query *)malloc(sizeof(str_query));
            while((c = *message) != ' '){
                if(isalnum(c)) {
                    temp[j++] = c;
                } else if(c == '='){
                    query->req_name = (char *)malloc(sizeof(char) * j);
                    strncpy(query->req_name, temp, j);
                    query->req_name[j] = '\0';
                    temp = (char *)malloc(sizeof(char)* 200);
                    j = 0;
                    state_check = 1;
                } else if(c == '&' && state_check){
                    query->req_value = (char *)malloc(sizeof(char) * j);
                    strncpy(query->req_value, temp, j);
                    query->req_value[j] = '\0';
                    temp = (char *)malloc(sizeof(char)* 200);
                    add_message(query);
                    state_check = 0;
                    j = 0;
                    query = (str_query *)malloc(sizeof(str_query));
                    
                }
                message++;
                i++;
                
            }
            if(j != 0){
                query->req_value = (char *)malloc(sizeof(char) * j);
                strncpy(query->req_value, temp, j);
                query->req_value[j] = '\0';
                add_message(query);
                j = 0;
                continue;
            }
            
        } 
        
        if(strncasecmp(message, "HTTP", 4) == 0){
            temp = (char *)malloc(sizeof(char) * 100);
            strncpy(temp, "HTTP", 4);
            message = message + 4;
            c = *message;
            int n = 4;
            while(c != '\n'){
                temp[n++] = c;
                c = *message++;
            }
            create_message("proto", temp, 5, n, 0);
            free(temp);
        }
        if(strncmp(message, "Host", 4) == 0){
            temp = (char *)malloc(sizeof(char) * 100);
            message = message + 4;
            c = *message++;
            int n = 0;
            if (c == ':'){
                c = *message++;
                while(c == ' ') {c = *message++;}
                while(!isspace(c)){
                    temp[n++] = c;
                    c = *message++;
                }
                create_message("Host", temp, 4, n, 0);
                free(temp);
            }
        }
        if(strncasecmp(message, "User-Agent", 10) == 0){
            temp = (char *)malloc(sizeof(char)* 100);
            message = message + 10;
            c = *message++;
            j = 0;
            if(c == ':'){
                c = *message++;
                while(isspace(c)){ c = *message++;}
                while(!isspace(c)){
                    temp[j++] = c;
                    c = *message++;
                }
            }
            create_message("User-Agent", temp, 10, j, 0);
            j = 0;
            free(temp);
            
        }
        if(strncasecmp(message, "Accept:", 7) == 0){
            message = message + 7;
            i = i + 7;
            c = *message++;
            while(c != '\n'){
                c = *message++;
                i++;
            }
        }
        if(strncasecmp(message, "Content-Length:", 15) == 0) {
            message = message + 15;
            i = i + 15;
            c = *message++;
            while(c != '\n'){
                c = *message++;
                i++;
            }
        }
        if(strncasecmp(message, "Content-Type:", 13) == 0) {
            message = message + 13;
            i = i + 13;
            c = *message++;
            while(c != '\n'){
                c = *message++;
                i++;
            }
        }
        if(isalnum(c)){
            char *nntp = (char *)malloc(sizeof(char) * 100);
            j = 0;
            i++;
            nntp[j++] = c;
            int d = 0;
            int check_chge = 0;
            message++;
            while(!isspace(*message) && *message != '\0'){
                c = *message;
                if(c == '&') {
                    create_message(nntp, temp, d, j, 1);
                    check_chge = 0;
                    j = 0;
                    free(nntp);
                    free(temp);
                    nntp = (char *)malloc(sizeof(char) * 100);
                } else if(c == '='){
                    d = j;
                    j = 0;
                    check_chge = 1;
                    temp = (char *)malloc(sizeof(char) * 100);
                } else if(c != '=' && !check_chge){
                    nntp[j++] = c;
                } else if(check_chge) {
                    
                    
                    if(strncasecmp(message, "%20", 3) == 0){
                        temp[j++] = ' ';
                        message = message + 3;
                        continue;
                    } else {
                        temp[j++] = c;
                    }
                }
                message++;
                i++;
            }
            if(j != 0){
                create_message(nntp, temp, d, j, 1);
                free(temp);
                free(nntp);
                j = 0;
                d = 0;
            }
        }               
            
        if(isspace(c)){
            while(isspace(c)){ c = *message++;}
            message--;
        } else {
            message++;
        }
        i++;
                    
    }
    printf("\nFirst second\n");
    ndata->meta_data = header;

    return ndata;
    
}

Link to my codebase

Answer 1

Minor nits

• if I type 'stoop' I get no error and a new server is started
• printf("Server %sed\n", START); makes the reader go 'huh?' I had to read a few times to see if it was doing something fancy
• you didnt mention platform, some calls are not c standard, like strncasecmp
• server_conn is never set to anything but 1, again making a reader look harder than they need - just say while(1) or for(;;;)
• in process_server you have 1000 all over the place, use MAX_MESSAGE_SIZE.
• I would call the return from socket listenSocket or something like that, listenid read like its a number you are using to keep track of something. Help the reader

Serious issue

Your socket receive is wrong. You assume you receive a whole message in one chunk.

    while((read_size = recv(fd, client_message, 1000, 0)) > 0) {
        printf("%s", client_message);
        re_data = router(client_message, read_size);
        printf("Method %s --url::%s=+++\n", re_data->method, re_data->url);
        switch_ch = 1;
        break;
            
    }

what happens if recv only returns you the first byte on the first call, which it is perfectly entitled to do. I am sure this works when client and server are on the same machine, but will fail if they are on separate machines.

Answer 2

Include Statements for C Library Header Files

While the code compiles as written, the standard for including C Library header files such as stdio.h, stdlib.h and ctype.h is to put the name of the file between angle brackets (i.e. < and >). The only time you might not want to follow this guideline is if you are modifying the C library and the header files associated with the routines being modified.

Including Executable Code

The code is currently including three C source files - this is not a generally accepted practice. The accepted practice is to compile all C source files separately with function prototypes for the functions to be shared in header files that can be included by the files that will be referencing the shared functions. Including C source files as is done in this code may result in multiple definitions of functions that will prevent the code from linking into an executable file.

Test for Possible Memory Allocation Errors

In modern high-level languages such as C++, memory allocation errors throw an exception that the programmer can catch. This is not the case in the C programming language. While it is rare in modern computers because there is so much memory, memory allocation can fail, especially if the code is working in a limited memory application such as embedded control systems. In the C programming language when memory allocation fails, the functions malloc(), calloc() and realloc() return NULL. Referencing any memory address through a NULL pointer results in undefined behavior (UB).

Possible unknown behavior in this case can be a memory page error (in Unix this would be referred to as Segmentation Violation), corrupted data in the program and in very old computers it could even cause the computer to reboot (corruption of the stack pointer).

To prevent this undefined behavior a best practice is to always follow the memory allocation statement with a test that the pointer that was returned is not NULL.

Example of Current Code:

    char* client_message = (char*)malloc(sizeof(char) * 1000);

Example of Current Code with Test:

    char* client_message = malloc(sizeof(*client_message) * 1000);
    if (client_message == NULL)
    {
        fprintf(stderr, "Malloc of client message failed in process_server\n");
        exit(EXIT_FAILURE);
    }

Convention When Using Memory Allocation in C

As shown in the example with the test above, when using malloc(), calloc() or realloc() in C a common convention is to call sizeof(*PTR) rather sizeof(PTR_TYPE); this makes the code easier to maintain and less error prone, since less editing is required if the type of the pointer changes. The cast to the proper type of variable is not necessary since the allocation functions return a pointer to void.

Complexity

The 4 functions in the code, main(), process_server(), tokenize() and router() are too complex (i.e. they do too much). Each of these functions should be broken into multiple functions that accomplish a smaller piece of what the large function accomplishes. There are multiple reasons for this - functions that are larger than a single screen or IDE are very hard to understand. This makes reading, writing, debugging and maintaining these functions very difficult. The art or science of programming is to break problems into smaller and smaller pieces so each piece is very easy to solve.

As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program. There is an example below of how the main() function can be broken up.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

Example of Main

static void show_headline()
{
    printf("\n\tDE is a basic server built to carry out simple things\n"
        "\t-----    -----\n"
        "\t|    |   |\n"
        "\t|    |   -----\n"
        "\t|    |   |\n"
        "\t-----    -----\n"
        "\tMethods implemented [Get & Put]\n"
        "\tTo start type start\n"
        "\tTo stop type stop\nDE]");
}

static void process_command_line(int argc, char** argv, int* port, char serAdd[])
{
    if (argc == 1) {
        port = PORT;
        strcpy(serAdd, ADDRESS);
    }
    else if (argc == 2) {
        port = atoi(argv[1]);
        strcpy(serAdd, ADDRESS);
    }
    else if (argc == 3) {
        port = atoi(argv[1]);
        int n = strlen(argv[2]);
        strncpy(serAdd, argv[2], n);
        if (n >= internet_address_len) {
            error_message("Internet address is incorrect");
        }
        serAdd[n] = '\0';
    }

}

int main(int argc, char** argv) {
    int port, pid, server_conn = 1;
    int internet_address_len = 15;
    char serAdd[internet_address_len];
    char strAction[7];

    process_command_line(argc, argv, &port, serAdd);
    show_headline();

    scanf("%s", strAction);
    if (strcmp(strAction, START) == 0) {
        printf("Server %sed\n", START);
        int listenid = start_server(port);
        printf("listen on port %d & listening is %d\nDE]", port, listenid);
        signal(SIGPIPE, SIG_IGN);
        while (server_conn) {
            pid = fork();
            if (pid == 0) {
                process_server(listenid);
                break;
            }
            scanf("%s", strAction);
            if (strcmp(strAction, STOP) == 0) {
                close(listenid);
                return 0;
            }

        }

    }

    return 0;

}