PSET6 parse function writing random character to strings

Question

UPDATE: The issue was resolved thanks to the comment below.

I'm working on the parse() function and seem to have random results with it. So far I wasn't able to launch the Web Server from my IDE as I get 505 or 405 errors. I've used printf() to see what my function actually writes and copies into the strings and some results are confusing - random characters come up here and there and I don't understand where they are coming from. Would be really glad if someone can help me understand what the problem is. Here is the code:

bool parse(const char* line, char* abs_path, char* query)
{          
    /* Ensuring that the line structure is consistent with the rule "method SP request-target SP HTTP-version CRLF" */
    //Find the first occurrence of a space character. Question: do we need to allocate memory here for the char*?
    const char* first_SP = strchr(line, ' ');        
    if (first_SP == NULL)
    {
        error(400);
        return false;
    }

    //parse method type from the line. Adding + 1 for \0 terminating character
    char method[first_SP - line + 1];        
    strncpy(method, line, first_SP - line);
    method[first_SP - line + 1] = '\0';

    //temporary code check
    printf("Method: '%s'\n", method);

    //remove the first space in line - uses pointer to go to the next pointer location (after the space)
    first_SP++;              
    //check if the rest of the line exists
    if (strlen(first_SP) < 2)    
    {
        error(400);
        return false;
    }

    //Find the second occurrence of a space character
    const char* second_SP = strchr(first_SP, ' ');        
    if (second_SP == NULL)
    {
        error(400);
        return false;
    }

    //parse request-target from the line. adding + 1 for \0 terminating character
    char rqst_trg[second_SP - first_SP + 1];        
    strncpy(rqst_trg, first_SP, second_SP - first_SP);
    rqst_trg[second_SP - first_SP + 1] = '\0';

    //temporary code check
    printf("Request: '%s'\n", rqst_trg);

    //remove the first space in line
    second_SP++;              
    //check if the rest of the line exists
    if (strlen(second_SP) < 2)    
    {
        error(400);
        return false;
    }
    //there should be no other spaces until end of line
    for (int i = 0; i < strlen(second_SP); i++)  
    {
        if ( *(second_SP + i) == ' ') 
        {
            error(400);
            return false; 
        }
    }

    //Find the end of the line that should be CRLF 
    const char* end = strstr(second_SP, "\r\n");
    if (end == NULL)
    {
        error(500);
        return false;
    }

    //parse HTTP version from the line. adding + 1 for \0 terminating character
    char httpv[end - second_SP + 1];     
    strncpy(httpv, second_SP, end - second_SP);
    httpv[end - second_SP + 1] = '\0';

    //temporary code check
    printf("Version: '%s'\n", httpv);

    //Make sure the method is GET, the only supported method
    if (strcmp(method, "GET") != 0)         
    {
        error(405);
        return false;        
    }

    /* Make sure the request target has a correct syntax  */
    // request target has to start with '/' (47 in ASCII) 
    if (rqst_trg[0] != '/')                      
    {
        error(501);
        return false;        
    }
    //request target cannot contain " character (34 in ASCII)
    for (int i = 0; i < strlen(rqst_trg); i++)  
    {
        if (rqst_trg[i] == '"') 
        {
            error(400);
            return false; 
        }
    }

    //Make sure the HTTP version is 1.1, the only supported version
    if (strcmp(httpv, "HTTP/1.1") != 0)         
    {
        error(505);
        return false;        
    }

    //Find the ? sign if there is a query
    const char* question = strchr(rqst_trg, '?');
    //pointer to string to copy
    //char *cptr;

    // if there's no query
    if (question == NULL)                               
    {
        //parse absolute path from the request line
        strcpy(abs_path, rqst_trg);         
        //the substring for query is absent, so query should be ""
        query[0] = '\0';                    
        abs_path[strlen(abs_path)] = '\0';
    }
    //if ? sign exists, but no query is present after it
    else if (strlen(question) == 1)         
    {
        //parse absolute path from the request line without the last ? character
        strcpy(abs_path, rqst_trg - 1);         
        //the substring for query is absent, so query should be ""
        query[0] = '\0';                    
        abs_path[strlen(abs_path)] = '\0';
    }
    //if the query is present
    else                                    
    {
        //parse the part of the request line that relates to the absomute path only, not the query
        strncpy(abs_path, rqst_trg, question - rqst_trg);   
        // parse the query to the query variable without the first ? character
        strcpy(query, question + 1);                        
        abs_path[strlen(abs_path)] = '\0';
        query[strlen(query)] = '\0';
    }

    return true;
}

The results of my printf() vary and can be as follows:

Method: 'GET'
Request: '/'
Version: 'HTTP/1.16'

Method: 'GET'
Request: '/p'
Version: 'HTTP/1.16'

Method: 'GETr'
Request: '/'
Version: 'HTTP/1.16'

I do not reallocate any memory and use the memory locations passed to the function for abs_path and query and add null terminators to my strings, however, the output doesn't work as expected.

Answer 1

Off-by-one. Let's see how this Method: 'GETr' could happen when line is "GET / HTTP/1.1\r\n". (We'll assume all addresses/pointers are relative to 0).

char method[first_SP - line + 1]; will create a char array with 4 elements (3 - 0 + 1), which are indexes 0 through 3.

strncpy(method, line, first_SP - line); will copy 3 characters from line into method, so method is "GET?" (the ? indicates we do not know what is in that 4th allocated byte)

method[first_SP - line + 1] = '\0'; Where is the null terminator going? 3 - 0 - 1 is 4. But method has only been allocated for indexes 0 through 3. So method is still "GET?". And the null terminator is going into memory that method doesn't own (read memory leak).

The random characters you see are whatever the contents of memory are the time. At a quick glance it looks like all your null terminators may be "off-by-one". There may be other problems in the function, but this should get you moving in the right direction. It is worth the extra time required to learn how to use debug50 or gdb with server.