Server.c failing with open ended query

Question

I've heard you guys rock here, so here's my first question:

My server.c code passes all tests but for one,

:( Requesting hello.php? returns 200, text/html, and correct output
   \ expected output, not an exit code of 0

The server segfaults when I request /hello.php?. I've narrowed it down to a peculiar finding, when I ran valgrind on it and made that request it showed me this:

GET /hello.php? HTTP/1.1
==8442== Invalid read of size 1
==8442==    at 0x4C2E0E2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8442==    by 0x402765: parse (server.c:822)
==8442==    by 0x40191A: main (server.c:189)
==8442==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==8442== 

It seems that line is not initialized at this point. When I connect and have the server print the contents of line as it is passed into the parse function, nothing shows up whatever request I make...but the server gives me the cat picture, says Hello Kathleen, etc.

Here's my parse code, let me know what you make of this mess! Much obliged :)

bool parse(const char* line, char* abs_path, char* query){
    printf("^%s^ line\n", line);

    char* linestr = malloc(sizeof(BYTES) * (strlen(line) + 1));
    strcpy (linestr, line);
    //printf("***%s*&\n", linestr);
    char* first_space = strchr(linestr, ' ') + 1;
    char* next_space = strchr(first_space, ' ');
    //printf("*%s* first_space\n", first_space);
   // printf("$%s$ next_space\n", next_space);
    if ((first_space[0] == ' ') || (next_space[1] == ' ')) {
        error(400);
        //printf("error400\n");
        free(linestr);
        return false;
    }
    else {

        //method is 1st token
        char* method = strtok(linestr, " ");

        //make sure method is GET
        if(strcmp(method, "GET") != 0) {
            free(linestr);
            error(405);
            return false;
        }

        else {    
            //request-line is next token
            char* request_line = strtok (NULL, " ");
            //printf("$%s$ requestline\n", request_line);
            //http version is next token
            char* http_version = strtok (NULL, "\r\n");
            //if request_line is null, strtok found two spaces OR if there's a " in request line, error 400
            if (strchr(request_line, '"') != NULL) {
                free(linestr);
                error(400);
                return false;
            }
            //requestline must start with /
            else if(request_line[0] != '/') {
                free(linestr);
                error(501);
                return false;
            }
            //now check http version
            else if (strcmp(http_version, "HTTP/1.1") != 0) {
                free(linestr);
                error(505);
                return false;
            }
            else {
                //if there's a query present
                if(strchr(request_line, '?') != NULL) {
                    //tokenize query on ?
                    //set absolute path
                    char* absolute_path = strtok(request_line, "?");
                    //printf("(%s) absolutepath with query\n", absolute_path);
                    char* possible_query = strtok(NULL, "?");
                    //printf("$%s$ possiblequery\n", possible_query);

                    //if query has a word after the ?
                    if(strlen(possible_query) > 3) {
                        //abs_path stays the same
                        strcpy(query, possible_query);
                        strcpy(abs_path, absolute_path);
                        free(linestr);
                        return true;
                    }
                    //it's not a valid query
                    else {
                        strcpy(abs_path, absolute_path);
                        //abs_path stays the same
                        query[0] = '\0';
                        free(linestr);
                        return true;
                    }
                }
                else {
                    //if there's no query
                    strcpy(abs_path, request_line);
                    //printf("$%sabspath with no query\n", abs_path);
                    query[0] = '\0';
                    free(linestr);
                    return true;
                //query scope
                }
            //request line scope
            }
        //method scope
        }
    //multiple space detector    
    }
//fxn def
}

Answer 1

The problem starts here char* possible_query = strtok(NULL, "?");. In the case where request_line is "/hello.php?", strtok will return NULL, because the first call to strtok consumed all of request_line. This line if(strlen(possible_query) > 3) { gives the seg fault 'cause you can't strlen NULL.

I think this printf("^%s^ line\n", line); is showing "confusing" results because of the CRLF terminator of line. Change up your format, like remove " line", since the message is for you and you know you're printing line; you should see different (better) results.

Answer 2

It's OK for strtok() to return NULL if you send it GET /hello.php? HTTP/1.1 -- as long as you immediately check to see whether it returned NULL, and do something sensible if it did. (In this case, there would be a question mark but no query string, so you'd just return the abs_path and discard the ?.)

In pseudocode:

if strtok() does not return NULL
    return query and abs_path
else
    return abs_path

At no point do you actually need to check if(strlen(possible_query) > 3) and run the risk of possibly dereferencing a NULL pointer.

By the way, if you get this sorted, please click a green check mark. I don't care if you accept my answer or @DinoCoderSaurus's answer -- just don't let your question turn into a ZOMBIE QUESTION.