0

I am getting 501 Not implemented on every file i open. Via valgrind i get the error many times:

==4890== Conditional jump or move depends on uninitialised value(s)
==4890==    at 0x4C2F1D0: strcmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4890==    by 0x401EBC: lookup (server.c:708)
==4890==    by 0x401C15: main (server.c:249)
==4890== 
==4890== Conditional jump or move depends on uninitialised value(s)
==4890==    at 0x401EC2: lookup (server.c:708)
==4890==    by 0x401C15: main (server.c:249)
==4890== 

Here are my parse and lookup functions:

    const char* lookup(const char* path)
{
    // TODO
    char* c = strrchr(path, '.');

    int n = strlen(c);

    char a[n]; 

    for (int i = 0; c[i]; i++) {
        a[i] = tolower(c[i]);
    }

    if (strcmp(a, ".css") == 0) {
        return "text/css";
    }
    if (strcmp(a, ".html") == 0) {
        return "text/html";
    }
    if (strcmp(a, ".gif") == 0) {
        return "image/gif";
    }
    if (strcmp(a, ".ico") == 0) {
        return "image/x-icon";
    }
    if (strcmp(a, ".jpg") == 0) {
        return "image/jpeg";
    }
    if (strcmp(a, ".js") == 0) {
        return "text/javascript";
    }
    if (strcmp(a, ".php") == 0) {
        return "text/x-php";
    }
    if (strcmp(a, ".png") == 0) {
        return "image/png";
    }
    return NULL;
}

/**
 * Parses a request-line, storing its absolute-path at abs_path 
 * and its query string at query, both of which are assumed
 * to be at least of length LimitRequestLine + 1.
 */
 bool parse(const char* line, char* abs_path, char* query)
{
    // check if method is of type GET
    if (strncmp(line, "GET ", 4) != 0)
    {
        error(405);
        return false;
    } 

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

    // get request target
    char* haystack = strchr(line, '/');
    char* needle = strchr(haystack, ' ');

    // will be used to check what comes after the request-target
    char* http_version = needle;

    // check for extra targets
    char* errchk = strchr(http_version + 1, ' ');
    if (errchk != NULL)
    {
        error(505);
        return false;   
    }

    char target[needle - haystack];
    strncpy(target, haystack, needle - haystack);
    target[needle - haystack] = '\0'; 

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

    // parse absolute path
    haystack = target;
    needle = strchr(haystack, '?');

    // check if a query is present
    if (needle == NULL)
    {
        strcpy(abs_path, haystack);
        query[0] = '\0';
    }
    else
    {
        // if so, remove query
        char path[needle - haystack + 1];
        strncpy(path, haystack, needle - haystack);
        path[needle - haystack + 1] = '\0'; 

        strcpy(abs_path, path);

        // parse query
        strcpy(query, needle + 1);
    }

    // check for HTTP version
    needle = strstr(http_version, "HTTP/1.1");

    if (needle == NULL)
    {
        error(505);
        return false;
    }

    return true;
}

Here are my load and indexes functions:

bool load(FILE* file, BYTE** content, size_t* length)
{
    // allocate memory on the heap

    *content = NULL;
    *length = 0;

    char* file_content = malloc(sizeof(char));
    if (file_content == NULL) 
    {
        return false; 
    }

    int i = 0;
    for (int c = fgetc(file); c != EOF; c = fgetc(file))
    {
        file_content[i] = (char) c;
        i++;
        file_content = (char*) realloc(file_content, sizeof(char)*(i+1));
    }
    // add terminating string and increase the size by one

    // stores the address of the first byte of file contant in heap
    *content = file_content;
    *length = i;
    return true;
}

char* indexes(const char* path)
{
    int path_length = strlen(path) + 1;


char* ind_php = "/index.php";
char* ind_html = "/index.html";

char* inx1 = malloc(path_length + 12);

if (inx1 == NULL) return NULL;

strcpy(inx1, path);
strcat(inx1, ind_php);

if (access(inx1, F_OK) != -1) {
    return inx1;
} else {
    free(inx1);
}

char* inx = malloc(path_length + 12);

if (inx == NULL) return NULL;

strcpy(inx, path);
strcat(inx, ind_html);

if (access(inx, F_OK) != -1) {
    return inx;
} else {
    free(inx);
}

return NULL;   
}

Any help will be really appreciated. PS : I checked my parse and lookup functions with check50 (server1.c) and got all green ticks.

Thanks in advance

2
  • The 501 error would be a result of lookup function (look at the call in main around line 244). Valgrind explicitly mentions lookup ==4890== by 0x401EBC: lookup (server.c:708). Review your lookup function for errors in your use of strcmp (==4890== at 0x4C2F1D0: strcmp ......). – DinoCoderSaurus May 13 '16 at 12:45
  • @DinoCoderSaurus Thanks for your reply, i checked my lookup function and according to me it's right.... I have attached it's code... please see if you can help – Saeed Jassani May 14 '16 at 4:21
0

valgrind is complaining because you are treating a like a string e.g. strcmp(a,".css"), but you haven't created a string. The thing that makes a string a string is the terminating null byte. Your a does not have such a terminator (nor is it declared large enough to accommodate it). From man strlen (emphasis added):

The strlen() function calculates the length of the string s, excluding the terminating null byte ('\0').

It should be quite straightforward to fix this problem. You might also consider another approach to make the problem just go away. It appears the only reason you are declaring a is to convert c to lower case. Why go to all that trouble when there is a string function that will do that for you? Here is a hint from the instructions:

Odds are you’ll find functions like strcasecmp, strcpy, and/or strrchr of help!

10
  • thanks for your reply.. can you help me with this? sandbox.cs50.net/checks/178969514db14f8e9bffe2430fb864ec – Saeed Jassani May 14 '16 at 13:44
  • Problem with target, though it is not presenting in check50, it is a memory leak. It is allocated for N bytes, which are named 0 to N - 1. This line target[needle - haystack] = '\0'; is an "invalid write of 1 byte" because you are putting '\0' in target[N]. That's not target's memory. The check50 errors come from path. Similar problem to target plus some. It is allocated for N bytes, strncpy N-1 bytes (named 0 through N-2), then set path[N] (which doesn't belong to path!) to '\0'. path[N - 1] could be anything, it's definitely wrong, and it's a memory leak. – DinoCoderSaurus May 14 '16 at 16:42
  • thanks again, can you help plz me solve this issue? i understand that that memory should not be accessed but i need to terminate the strings.. if i remove that line, my server doesn't work – Saeed Jassani May 15 '16 at 8:19
  • To much info to stuff in that comment. I think the bottom line is remove the + 1 from here: path[needle - haystack + 1] = '\0'; Add a +1 here char target[needle - haystack]; All things being equal, that should fix server problems. Please think about this critically when you are done celebrating your server victory :) 0-index is a fundamental concept in most (all?) languages that is important to master. Remember, if you allocate an array for N elements, there is no "element named N" (or element[N]) because counting starts at 0. – DinoCoderSaurus May 15 '16 at 10:29
  • 1
    it works now :) thanks a lot... i will surely remember your advice – Saeed Jassani May 15 '16 at 11:13

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .