pset6 server fails check50

Question

I can't find why this doesn't work, can someon help me? Thanks.

My code fails the following in check50 server1:

:( Requesting cat.exe returns error code 501
   \ expected output, but not "HTTP/1.1 404 Not Found\r\nContent-Type:..."

https://sandbox.cs50.net/checks/9d5b4521abb5459bbc4c59f3905e5c5b

And the following in check50 server2:

:( Requesting cat.jpg returns 200, image/jpeg, and correct image
:( Requesting cat.html returns 200, text/html, and correct file
:( Requesting cat2.HTML returns 200, text/html, and correct file
:( Requesting cat3.HtMl returns 200, text/html, and correct file
:( Requesting cat.gif returns 200, image/gif, and correct file
:( Requesting favicon.ico returns 200, image/x-icon, and correct file
:( Requesting test.css returns 200, text/css, and correct file
:( Requesting test.js returns 200, text/javascript, and correct file
:( Requesting hello.php returns 200, text/html, and correct output
   \ expected output, not an exit code of 0
:( Requesting hello.php?name=Alice returns 200, text/html, and correct output
   \ expected output, not an exit code of 0
:( Requesting /test redirects to /test/
:( Requesting /test/ outputs /test/index.html
:( Requesting directory containing index.php outputs index.php
   \ expected output, but not "HTTP/1.1 301 Moved Permanently\r\nLocat..."
:( Requesting two files in a row (cat.html then cat.jpg) succeeds

https://sandbox.cs50.net/checks/ebfe53568033401a8385c1927e979dbc

My code is as follows:

INDEX

char* indexes(const char* path)
{
    char *php_file_dir = malloc(sizeof(path) + sizeof("index.php"));
    char *html_file_dir = malloc(sizeof(path) + sizeof("index.html"));

    strcat(php_file_dir, path);
    strcat(php_file_dir, "index.php\0");

    strcat(html_file_dir, path);
    strcat(html_file_dir, "index.html\0");

    FILE *php_file = fopen(php_file_dir, "r");;
    FILE *html_file = fopen(html_file_dir, "r");

    if (php_file != NULL)
    {
        free(html_file_dir);
        fclose(php_file);
        fclose(html_file);
        return php_file_dir;
    }

    if (html_file != NULL)
    {
        free(php_file_dir);
        fclose(php_file);
        fclose(html_file);
        return html_file_dir;
    }

    return NULL;
}

LOAD

bool load(FILE* file, BYTE** content, size_t* length)
{
    if (file == NULL)
    {
        return false;
    }

    fseek(file, 0l, SEEK_END);
    size_t size = ftell(file);
    fseek(file, 0l, SEEK_SET);

    *length = size;

    *content = malloc(size);

    if (*content == NULL)
    {
        return false;
    }

    for (int i = 0; i < size; i++)
    {
        fread(content, 1, size, file);
    }

    if (*length != size)
    {
        return false;
    }

    return true;
}

LOOKUP

const char* lookup(const char* path)
{
    char *f_end = strrchr(path, '.');

    if (strcasecmp(f_end, ".css") == 0)
    {
        return "text/css";
    }

    if (strcasecmp(f_end, ".html") == 0)
    {
        return "text/html";
    }

    if (strcasecmp(f_end, ".gif") == 0)
    {
        return "image/gif";
    }

    if (strcasecmp(f_end, ".ico") == 0)
    {
        return "image/x-icon";
    }

    if (strcasecmp(f_end, ".jpg") == 0)
    {
        return "image/jpeg";
    }

    if (strcasecmp(f_end, ".js") == 0)
    {
        return "text/javascript";
    }

    if (strcasecmp(f_end, ".php") == 0)
    {
        return "text/x-php";
    }

    if (strcasecmp(f_end, ".png") == 0)
    {
        return "image/png";
    }

    return NULL;
}

PARSE

bool parse(const char* line, char* abs_path, char* query)
{
    // Checks if method is GET
    if (strncmp(line, "GET ", 4) != 0)
    {
        error(405);
        return false;
    }

    // To point at the start of the request instead of the first space
    char *req_pos = strchr(line, ' ');
    req_pos += 1;

    // To point at the start of the version instead of the second space
    char *v_pos = strrchr(line, ' ');
    v_pos += 1;

    char *request = malloc(v_pos - req_pos);
    strncpy(request, req_pos, (v_pos - req_pos - 1));

    char *version = malloc( &(line[strlen(line) - 1]) - v_pos);
    strncpy(version, v_pos, &(line[strlen(line) - 1]) - v_pos);

    request[strlen(request) - 1] = '\0';

    if ( (strstr(line, "  ")) || (strchr(request, ' ')) || (strchr(version, ' ')) )
    {
        error(400);
        free(request);
        free(version);
        return false;
    }

    if (request[0] != '/')
    {
        error(501);
        free(request);
        free(version);
        return false;
    }

    if (strncmp(version, "HTTP/1.1", 8) != 0)
    {
        error(505);
        free(request);
        free(version);
        return false;
    }

    if (strchr(request, '"') != NULL)
    {
        error(400);
        free(request);
        free(version);
        return false;
    }

    if (strchr(request, '?') == NULL)
    {
        strcpy(query, "");
        strcpy(abs_path, request); 
        free(request);
        free(version);
        return true;
    }

    if (strchr(request, '?') != NULL)
    {
        if (request[strlen(request) - 2] == '?')
        {
            request[strlen(request) - 2] = '\0';

            strcpy(query, "");
            strcpy(abs_path, request);

            free(request);
            free(version);
            return true;
        }

        // To start query after the "?"
        char *q_pos = strchr(request, '?');
        q_pos += 1;

        strcpy(query, q_pos);

        free(request);
        free(version);
        return true;
    }


    free(request);
    free(version);
    error(501);
    return false;
}

Answer 1

There are some problems that are quick to see. They are most likely not all the problems in play here.

parse

The server1 failure probably stems from here strncpy(request, req_pos, (v_pos - req_pos - 1)); From man strncpy:

The strncpy() function is similar, except that at most n bytes of src are copied. Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated.

Suggest you read up on calloc. When you calloc, instead of malloc, "The memory is set to zero.". If you calloc'd request, all the bytes would be initialized to zero. Assuming you don't strncpy more bytes than it is allocated for, it will be null-terminated (0 byte). And yes, I see this request[strlen(request) - 1] = '\0';. I think there's two possible problems there. First, if request is not properly null terminated, than how can you rely on the result of strlen(request)? Second, look at the math. If request is /cat.exe, then strlen(request) is 8. 8 - 1 is 7. strlen[7] is 'e', so now request is "/cat.ex". 404.

load

This result \ expected output, not an exit code of 0 from check50/server2 indicates a seg fault (from load in this case). From the spec:

Odds are you’re unfamiliar with popen. That function opens a "pipe" to a process (php-cgi in our case), which provides us with a FILE pointer via which we can read that process’s standard output (as though it were an actual file).

php files are opened as pipes. You cannot use fseek or ftell or any number of file functions on a pipe. This post explains it far better than I can.

indexes

The strcats (eg strcat(php_file_dir, "index.php\0");) in indexes are a problem. It appears you are trying to null terminate the string literal. You don't have to (nor should you), the computer does it for you. Any string literal like "index.php" is automagically null terminated.

This is just the low-hanging fruit. After you make these corrections, you will likely still have some problems. Make sure you test your server by making requests with a browser or curl. You really need to know that server works before you submit it to check50. And use the tools. You will find some intro info to debug50 here, and some tips on using gdb here. This pset really challenges one to exhibit a solid understanding of pointers and memory allocation and learning to use new functions; coalescing everything learned thus far. It can never be said that it's easy.

Answer 2

First of all, WOW: your code is awesome.

I took LOOKUP on the side and I could not see any obvious errors.

I did find some behaviors on PARSE you might find useful.

This is how I debugged your function:

I have this program called parse.c where I copy and pasted your function:

#include <stdio.h>
#include <string.h>
#include <strings.h>
#include <cs50.h>
#define LimitRequestLine 8190

bool parse(const char* line, char* abs_path, char* query);
void error(int e); 

int main(void)
{
    const char* line = "GET /abs_path?query HTTP/1.1\r\n";

    char abs_path[LimitRequestLine + 1]; 
    char    query[LimitRequestLine + 1]; 

    printf("%s\n\n\n", parse(line, abs_path, query)? "\n\n\ttrue" : "false");

    printf("\n\n\tabs_path: %s\n\n", abs_path);
    printf("\n\n\tquery:    %s\n\n", query);

}

// PARSE

bool parse(const char* line, char* abs_path, char* query)
{
    // copy and pasted your parse function's contents here.
}

void error(int e)
{
    printf("\n\t%i\t", e);
}

Your code compiled alright. Notice that I have a variable line. Having line be

"GET /abs_path?query HTTP/1.1\r\n"

The output should have been:

~/workspace/pset6 $ ./parse


        true




        abs_path: /abs_path



        query:    query

However, the output was:

~/workspace/pset6 $ ./parse


        true




        abs_path: 



        query:    quer

So here we see two things we are missing: the absolute path, and the last character of the query.

When I talk about line numbers, the numbers are based on the code you provided (copying and pasting your LOOKUP block in a text editor). For example:

bool parse(const char* line, char* abs_path, char* query) is line #1

strncpy(version, v_pos, &(line[strlen(line) - 1]) - v_pos); is line #22

and so on.

I proceeded to do some gdb on parse.c with your function and this is what I found:

1.- Your code is awesome. I could see how almost all variables got populated correctly with your pointer math.

2.- You lose the last character of the query on line 24. This is because you already had a \0 at the end of request. Why? If according to the strncpy man page:

... Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated. ...

I think it was because you malloced the size on line 18, but I am not certain.

So you can delete line 24:

request[strlen(request) - 1] = '\0';

3.- The if condition that gets evaluated to true is the one on line 67, but not the one on line 69 inside of it. You don't have a piece of code that sets abs_path if the condition on line 69 is false. That is why abs_path is empty when you return.

I suggest you add something like line 74:

strcpy(abs_path, request);

Just ignore everything after the ?

in about line 86 after you set the query.