Seg fault in server.c (and sometimes 404)

Question

so I'm doing pset6. My implementation passed every check50 test except the last one, which is:

:( Requesting two files in a row (cat.html then cat.jpg) succeeds

When I test server.c with the public folder, the server starts, but when I click on something like 'cat.html' or 'cat.jpg', the server gives me seg fault and sometimes even 404 or 405. Like the screenshot below:

I don't know which part of the implementation I did wrong. Here is my implementation of parse:

bool parse(const char* line, char* abs_path, char* query)
{
    int len = strlen(line), 
        space_count = 0,
        first_space_pos,
        second_space_pos;

    // Check if the form of the request target is valid.
    // Also checks the position of the first space.
    for (int i = 0; i < len; i++)
    {
        if (line[i] == ' ')
        {
            if (space_count == 0)
            {
                first_space_pos = i;
            }
            if (space_count == 1)
            {
                second_space_pos = i;
            }
            space_count++;
        }
        else if (line[i] == '"')
        {
            error(400);
            return false;
        }
    }

    // If there are more than 2 spaces.
    if (space_count != 2)
    {
        error(400);
        return false;
    }

    // Get the method!
    char* method = malloc(sizeof(char) * (first_space_pos+1));
    for (int i = 0; i < first_space_pos; i++)
    {
        method[i] = toupper(line[i]);
    }

    // If method is not GET, return false.
    if (strcmp(method, "GET") != 0)
    {
        error(405);
        return false;
    }


    // Get the HTTP version!
    char* http_w_space = strrchr(line, ' ');
    int http_len = strlen(http_w_space) + 1;

    char* http_version = malloc(sizeof(char) * http_len);

    for (int i = 0; i < http_len; i++)
    {
        http_version[i] = http_w_space[i+1];
    }

    // If HTTP version is not 1.1, return false.
    if (strcmp(http_version, "HTTP/1.1\r\n") != 0)
    {
        error(505);
        return false;
    }

    // Get the request target!
    char* req_target_extended = strchr(line, ' ');
    char* req_target = malloc(sizeof(char) * (second_space_pos - first_space_pos - 1));
    int extended_space_count = 0;

    for (int i = 0, n = strlen(req_target_extended); i < n; i++)
    {
        if (req_target_extended[i+1] == ' ')
        {
            extended_space_count++;
        }
        if (extended_space_count == 1)
        {
            break;
        }
        req_target[i] = req_target_extended[i+1];

    }

    // If request-target does not begin with /, respond to the browser with 501 Not Implemented and return false
    if (req_target[0] != '/')
    {
        error(501);
        return false;
    }

    // Query
    char* query_flag = NULL;
    int question_mark_pos = 0, req_target_len = strlen(req_target);

    // Go through request_target.
    // Search for the '?' mark.
    // Get the query.
    for (int i = 0, n = strlen(req_target); i < n; i++)
    {
        if (req_target[i] == '?')
        {
            question_mark_pos = i;
            break;
        }
        abs_path[i] = req_target[i];
    }

    query_flag = strchr(req_target, '?');
    if (query_flag == NULL || strlen(query_flag) == 1)
    {
        query[0] = '\0';
    }
    else
    {
        int index = 0;
        for (int i = question_mark_pos + 1; i < req_target_len; i++)
        {
            query[index] = req_target[i];
            index++;
        }
    }

    if (abs_path != NULL && query != NULL)
    {
        return true;
    }

    error(501);
    return false;
}

If anyone had had this problem before, or if anyone knows what went wrong in my code, please do comment and explain. Thanks everyone in advance!

Answer 1

I have many takes on your implementation. in the sunny day scenario, your parse function should work if you fixed the problems hinted at below:

• are you sure you're terminating method correctly?
• are you sure you're not accessing elements that are beyond line?
• are you sure you're allocating memory for req_target that is big enough to hold the null terminator?
• are you sure strlen(req_target_extended) is the length you expect?
• are you sure you're terminating req_target correctly?
• are you sure strlen(query_flag) would return 1 in case there's only a ? in the request target?
• are you sure you're terminating query correctly?

I got the problems above just by looking at the code. there may be more problems if I pull out a debugger.

I also think the best way wouldn't be to fix these problems directly. instead, I recommend you rethink your logic, google around see how you can do stuff. there are functions in the C standard library that may help you extract and copy the different elements.

here is some pseudocode that may give you some hints on a more elegant implementation:

copy line into a new string, lineCpy // to be able to manipulate its contents
get the first token of lineCpy, where a token is delimited by a space
if such token doesn't exist or is not "GET" // don't perform any operations (e.g., capitalizing) on it
    produce 404 and return false

get the next token of lineCpy, where a token is delimited by a space
if such token doesn't exist or doesn't start with a '/'
    produce 501 and return false
else if it has a '"'
    produce 400 and return false

if the next token is not "HTTP/1.1\r\n"
    produce 505 and return false


find the first occurrence of '?' in the request target
if no '?' found
    terminate query
    copy request target into abs_path
    return true

copy the characters after the '?' into query
calculate the length of the absolute path and store it in abs_length // maybe using pointer arithmetic
copy abs_length characters from the request target into abs_path // make sure you terminate abs_path

return true