Pset6 check50 server1 parse error

Question

I am having trouble addressing the below two errors when I run the first check50 of pset6.

check50 2015.fall.pset6.server1 server.c
:( Requesting cat.exe returns error code 501
   \ expected output, but not "HTTP/1.1 505 HTTP Version Not Supported..."
:( Requesting non-existant file returns error code 404
   \ expected output, but not "HTTP/1.1 505 HTTP Version Not Supported..."

I have been stuck on this for quite a while. Any thoughts or suggestions would be immensely appreciated!

bool parse(const char* line, char* abs_path, char* query)
{
    int length = strlen(line);
    int spaceLocation = 0;
    int quesLocation = 0;
    int endLocation = 0;

    // check method GET
    if (line[0] != 'G' || line[1] != 'E' || line[2] != 'T' || line[3] != ' ')
    {
        error(405);
        return false;
    }

    // find second space and \r\n
    for (int i = 4; i < length; i++)
    {
        if (line[i] == ' ')
        {
            spaceLocation = i;
        }
        if (line[i] == '\'')
        {
            endLocation = i;
        }
    }

    // check request-target
    if (line[4] != '/')
    {
        error(501);
        return false;
    }

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

    // find query (if any)
    int count = 0;
    for (int i = 4; i < spaceLocation; i++)
    {
        if (line[i] == '?')
        {
            quesLocation = i;
            count++;
        }
    }

    if (count > 1)
    {
        error(400);
        return false;
    }
    else if (count == 1)
    {
        for (int i = quesLocation + 1; i < spaceLocation; i++)
        {
            if (line[i] == '"')
            {
                error(400);
                return false;
            }
        }

        int queryLen = spaceLocation - quesLocation - 1;
        char queryTemp[queryLen];
        for (int j = 0; j < queryLen; j++)
        {
            queryTemp[j] = line[quesLocation + 1 + j];
        }
        query = queryTemp;
    }

    int pathLen = quesLocation - 3 - 1;
    char pathTemp[pathLen];
    for (int j = 0; j < pathLen; j++)
    {
        pathTemp[j] = line[4 + j];
    }
    abs_path = pathTemp;

    // check HTTP-version    
    char httpVersion [8];
    httpVersion [0] = 'H';
    httpVersion [1] = 'T';
    httpVersion [2] = 'T';
    httpVersion [3] = 'P';
    httpVersion [4] = '/';
    httpVersion [5] = '1';
    httpVersion [6] = '.';
    httpVersion [7] = '1';

    if ((endLocation - (spaceLocation + 1)) != 8)
    {
        error(505);
        return false;
    }
    else
    {
        for (int i = 0; i < 8; i++)
        {
            if (line[spaceLocation + 1 + i] != httpVersion[i])
            {
                error(505);
                return false;
            }
        }
    }

    error(501);
    return false;
}

Answer 1

Since there are only two places in server.c that return a 505, and they are both in this parse function, that's a good place to start looking.

debug50 or gdb (tips here) would be perfect tools to help you find the problem. It takes some practice to use the effectively, but either will server you well as you work your way through this problem set.

One problem is the way endLocation is calculated. From the spec

and CRLF represents \r\n.

The symbols \r and \n each represent one byte, one char. There is no physical \ to match on. (Think about using \n in printf formats). This line if (line[i] == '\'') will be true if line[i] is a single-quote. From the comments and context, that is not the intention.

You might find this article helpful in clarifying the point. There are many other articles on web as well.

Once you sort that, you will still face problems (debug50, gdb!). There is no return true; in this parse, so if it doesn't encounter any other error codes in the function, it will always give a 501 error and return false.

Answer 2

I inserted your program in this parse.c to debug it:

#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

// copy and pasted your function here


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

Your program should have this output in case of success:

    true




    abs_path: /abs_path



    query:    query

Instead, I got this with your function:

    505     false




    abs_path: 



    query:    

I am going to speak about line numbers based on the code you provided. For example in your code block:

int length = strlen(line); // is line #3

for (int i = 4; i < spaceLocation; i++) // is line #43

I debugged it with gdb and I found the following: To define endLocation, you are looking for the character '\''. That means looking for a single quote. I ignore why would this mean an end location of the line.

Actually, because there is no single quotes in the whole line, endLocation remains untouched after your loop on lines 16-26.

Since endLocation is 0, by the time you reach line 96, the condition evaluates to false and you error out 505:

if ((endLocation - (spaceLocation + 1)) != 8)
{
    error(505);
    return false;
}

The format of the request line is the following:

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

So you have several options as characters that denote the end of the line. I would just take the strlen that you saved in the variable length and take the http version to be "HTTP/1.1\r\n". Notice that I include \r\n in the string so I can compare the last 10 characters to that.

Another option could be to take the \r as the end character, but with this implementation you use yet another variable. Why not just take the strlen as the end of the line?

I suggest you treat length as end location. You can change line 96 to be:

if ((length - (spaceLocation + 1)) != 10)

And add two more characters to httpVersion on line 95, for example:

httpVersion [8] = '\r';
httpVersion [9] = '\n';

And of course redefine it as an array of 10 characters instead:

char httpVersion [10];

That will solve one problem. But I noticed that you have another behaviours that could cause problems.

When you do either of these lines:

query = queryTemp;
abs_path = pathTemp;

You do not change the original abs_path or original query that they gave you. Why? Because they gave you the address of each one. And instead of working with those addresses, the program is making abs_path and query point to other addresses (the ones of the arrays that you create in this function).

So, to change the addresses they gave you, you could do these lines instead of the previous ones:

for query:

    queryTemp[queryLen] = '\0';
    strcpy(query, queryTemp);

for abs_path:

pathTemp[pathLen] = '\0';
strcpy(abs_path, pathTemp);

The null terminator is important at the end of a string and there is no assignment before in the program, so I think that will do.