Server.c, errors in Check50, probably a problem with load function

Question

I'm almost done with server.c but can't seem to find the last error which doesn't allow me to pass check50.

I tried to analyse the existing responses to similar questions but couldn't use them to find my bug. Any help would be very much appreciated!

The errors I receive are:

:( Requesting cat.jpg returns 200, image/jpeg, and correct image
:( Requesting /test/ outputs /test/index.html
:( Requesting directory containing index.php outputs index.php
:( Requesting two files in a row (cat.html then cat.jpg) succeeds

Given these errors I suppose there is some existing problem with jpg's and php's. The second one is strange though because I checked with GDB that the indexes function returns the correct path as a result when asked for index.html.

load()

bool load(FILE* file, BYTE** content, size_t* length)
{
    *content = NULL;
    *length = 0;
    *content = malloc(512 * sizeof(BYTE));
    if (*content == NULL)
        {
            return false;
        }
    size_t size = fread(*content, sizeof(BYTE), 512, file);
    while (size > 0)
    {   
        *length += size;
        *content = realloc(*content, *length * sizeof(BYTE));
        if (*content == NULL)
        {
            return false;
        }
        size = fread(*content + *length, sizeof(BYTE), 512, file);
    }
    return true;
}

EDIT #1 My jpg convertion in the lookup function:

else if (strcasecmp(final, ".jpg") == 0)
{
    mime = "image/jpeg";
}

The error in Check50 is:

expected an exit code of 0, not standard error of "======= Backtrace: =========\n======= M..."

except for index.php where the error is:

expected output, not standard error of "======= Backtrace: =========\n======= M..."

My indexes function seems to be ok, here it is:

// ensure path is readable and executable
if (access(path, R_OK | X_OK) == -1)
{
    error(403);
    return NULL;
}
char *new_path = malloc(strlen(path) + 2);
strcpy (new_path, path);
char* path1 = malloc(51);
if (path1 == NULL)
{
    return NULL;
}
strcpy(path1, new_path);
char* path2 = malloc(52);
if (path2 == NULL)
{
    return NULL;
}
strcpy(path2, new_path);
strcat(path1, "/index.php");
strcat(path2, "/index.html");
path1[strlen(new_path) + strlen("/index.php")] = '\0';
path2[strlen(new_path) + strlen("/index.html")] = '\0';
// check if index.php or index.html exists and returns. Else return NULL
if (access(path1, R_OK) != -1)
{
    free(path2);
    return path1;
}
else if (access(path2, R_OK) != -1)
{
    free(path1);
    return path2;
}
else
{
    free(path1);
    free(path2);
    return NULL;   
}    

Answer 1

The backtrace result from check50 indicates a memory violation/ seg fault. What happens if you make a browser request tocat.jpg? Odds are server crashes.

load

(assuming cat.jpg) After the first fread, what have we got?

1. *content is allocated for 512 bytes.
2. size is 512
3. *length is 0

Enter the while loop:

1. *length becomes 512
2. reallocate *content for *length bytes. which is 512
3. Read another 512 bytes into *content starting at the *length byte. Oops, that's not *content's memory.

indexes take 2

Your allocations are still hinky. Let's go through path1, I leave path2 to you.

path1 will end up being path + index.php. So it needs to be allocated for strlen(path) + strlen("index.php") + 1. You do not need new_path at all. It's adding unnecessary complexity. Once path1 is allocated properly, let strcpy and strcat manage the null terminator (which they automatically do). No need for this path1[strlen(new_path) + strlen("/index.php")] = '\0'; (which may be the cause of the Backtrace error from check50, I wouldn't be surprised if the path name in check50 is longer than 52 chars; path on the IDE would likely be home/ubuntu/workspace/pset6/public which is around 35). Your indexes function should boil down to malloc > strcpy > strcat > access. (And appropriate frees).

Once you wrap your mind around that, and pass check50, think about this. Do you really need path1 and path2? Couldn't you simply allocate one variable (for the longer of the two, of course)? Build it (strcpy,strcat), test it (access). If it "passes", return it. If not, build the second one, test it. If it passes, return it. Otherwise free it, return NULL.

indexes (1st iteration)

This allocation char* path1 = malloc(60); is rando (and path2 too). You know exactly how big to allocate those vars. How long is path? Plus how long is index.html|index.php? Plus one for the null-byte. This may not, in fact, cause check50 to fail (after load is fixed). It is a potential memory leak, however.

Keep at it with gdb. And don't forget to test your server with a browser and/or curl before you submit it to check50.