0

Server is functional and gives the correct output using the curl requests individually. However, using gdb, if I use curl to request hello.html after I've requested cat.html (specifically), I get a seg fault on this line(in load):

buffer = malloc(sizeof(BYTE) * size);

and using gdb, if I use curl to request cat.html and then request cat.html again (specifically), I get a seg fault on this line(in main):

path = malloc(strlen(root) + strlen(p) + 1);

which is what follows the parse function, so I assume it's seg faulting because of how I implemented parse.

Can someone please help me out?

Here's my load:

#define CHUNK 1024

bool load(FILE* file, BYTE** content, size_t* length)
{
BYTE *buffer, *buffer2;
size_t size, temp;

size = CHUNK;
temp = size;

buffer = malloc(sizeof(BYTE) * size);

size_t cur_len = 0;

while ((cur_len = fread(buffer, sizeof(BYTE), CHUNK, file)) > 0) 
{
    size += cur_len;

    if (temp - size < CHUNK) 
    {
        // buffer is too small, the next read could overflow!
        temp *= 2;                      
        buffer2 = malloc(sizeof(BYTE) * temp);
        memcpy(buffer2, buffer, size * sizeof(BYTE));
        free(buffer);
        buffer = buffer2;
    }
}
strcat(buffer, "\0");
*content = buffer;
*length = size;

return true;
}

and here's my parse:

bool parse(const char* line, char* abs_path, char* query)
{
// Extract 'method' string from 'line' string
int line_len = strlen(line);
char* temp = malloc(line_len);
char* method = malloc(line_len);
strcpy(temp, line);
strcpy(method, temp);
method = strtok(temp, " ");

// method spec
if((strlen(method)) != 3)
{
    error(405);
    return false;
}

if((strcasecmp(method, "get")) != 0)
{
    error(405);
    return false;
}

int j = 0;
while(j < line_len)
{
    // only one space separating strings
    if((line[j] == ' ') && (line[j + 1] == ' '))
    {
        error(400);
        return false;
    }

    // filename must not have " 
    if (line[j] == '"')
    {
        error(400);
        return false;
    }

    j++;
}

// HTTP Version spec
char* version = strcasestr(line, "HTTP/1.1");
if(version == NULL)
{
    error(505);
    return false;
}

// .exe is not valid
char* exe_file = strcasestr(line, ".exe");
if(exe_file != NULL)
{
    error(501);
    return false;
}

// Extract 'request-target' string from 'line' string
char* temp1 = malloc(line_len);
char* req_target = malloc(line_len);
strcpy(temp1, line);
strcpy(req_target, temp1);

req_target = strtok(temp1, " ");

if(strstr(line, "?") != NULL)
{
    req_target = strtok(NULL, "?");
}
else
{
    req_target = strtok(NULL, " ");
}

int tar_len = (strlen(req_target)) + 1;
char* temp2 = malloc(tar_len);
strcpy(temp2, req_target);
strcat(temp2, "\0");

// req_target spec
if(temp2[0] != '/')
{
    error(501);
    return false;
}

// CRCF spec
char* CRCF = strstr(line, "\r\n");
if(CRCF == NULL)
{
    error(400);
    return false;
}

 // set new abs_path
strcpy(abs_path, temp2);

// Extract 'query' string from 'line' string
char* temp3 = malloc(line_len);
char* the_query = malloc(line_len);
strcpy(temp3, line);
strcpy(the_query, temp3);

char* quest_m = strchr(the_query, '?');
if(quest_m != NULL)
{
    the_query = strtok(temp3, " ");
    the_query = strtok(NULL, "?");
    the_query = strtok(NULL, "  ");
}

int qry_len = strlen(the_query) + 2;

// check if query exists
if(the_query != NULL)
{
    char* char_eq = strchr(the_query, '=');

    if(char_eq != NULL)
    {
        char* qry = malloc(qry_len);
        strcpy(qry, the_query);
        strcat(qry, "\0");
        strcpy(query, qry);
    }
    else
    {
        char* qry = malloc(1);
        strcpy(qry, "\0");
    }
}
else
{
    char* qry = malloc(1);
    strcpy(qry, "\0");
}

return true;
}
5
  • Suggest you look at valgrind result after one request of /cat.html. FWIW this is how I call valgrind valgrind -v --leak-check=full --show-leak-kinds=all --track-origins=yes ./server ./public. Your parse is big on malloc. free? not so much. When you find the solution, write it up as an answer so others might benefit from your work. You will be able to "accept" the answer after 2 days. Sep 1 '16 at 12:33
  • I appreciate your help @DinoCoderSaurus again. I've managed to fix up my parse stuff (as far as I know) so valgrind doesn't show me issues in parse anymore. All I really changed was the size of my mallocs (according to the spec they need to be of size (LimitRequestLine + 1). The free'ing takes place in main (I think) because if I try free stuff in parse I get errors because it's trying to free memory that's already been freed. Sep 2 '16 at 9:47
  • However, using valgrind I still get an issue with my load function on this line I believe: buffer = malloc(sizeof(BYTE) * size); Sep 2 '16 at 9:49
  • I'm really unsure of how to go about fixing it. Valgrind tells me: Address 0x550fb20 is 0 bytes after a block of size 1,024 alloc'd. Does that mean, I'm allocating unnecessarily? Sep 2 '16 at 9:55
  • When I saw the mallocs without corresponding frees in the parse function, I stopped reading and assumed (!) that was the problem. Shame on me. It's definitely part of the problem. I also chose to ignore the fact that l'il old cat.html at 156 bytes can slip under the memory leak radar, but it calls big bad cat.jpg at 26860 bytes, which will not. Working on a more complete analysis now. A leading indicator of problems with load: there is no realloc. Sep 2 '16 at 14:25
0

Let's get this out of the way first. Don't do this strcat(buffer, "\0");. You need to return the exact contents of file to main. This changes the contents; it adds something that's not there. A browser will not complain about this little addition. valgrind and/or check50/server2 probably will. Assuming buffer is allocated for the exact length of the file, you've just added '\0' to memory that doesn't belong to buffer. Speaking of which.....

Now let's take /cat.jpg for a spin in load.

size = CHUNK; size=1024
temp = size; temp=1024
buffer = malloc(sizeof(BYTE) * size); buffer is [1024] (i.e. allocated for 1024 bytes)
(cur_len = fread(buffer, sizeof(BYTE), CHUNK, file)) first read, cur_len =1024
size += cur_len; size = 2048
if (temp - size < CHUNK) 1024 - 2048 < 1024, TRUE
temp *= 2; temp = 2048
buffer2 = malloc(sizeof(BYTE) * temp); buffer2 is [2048]
memcpy(buffer2, buffer, size * sizeof(BYTE)); copy 2048 bytes of buffer into buffer2.

WAT???? buffer only owns 1024 bytes. Who owns the other 1024?? (maybe temp that was allocated in parse?). Any file bigger than CHUNK bytes will provoke this, including favicon.ico at around 1406. If you are inclined to debug with gdb, that might be an easier file to work with than cat.jpg.

[WARNING: It is very important that you check my math. gdb does better arithmetic than I do (everybody does better arithmetic than I do!)]

This free(buffer); provokes the Address 0x550fb20 is 0 bytes after a block of size 1,024 alloc'd. message from valgrind. It likely explains the "double free" messages that pop up when you (correctly) add the frees to parse.

I have no "change this to that and all will be well". You kinda have to rethink your function. The CHUNK, cur_len approach is great. Your code indicates you know what needs to be done. The devil is in the details. The function will need realloc. And while you are rethinking, consider this: memcpy(buffer2, buffer, size * sizeof(BYTE)); is always copying the contents of buffer into the contents of buffer2, starting at the first byte.

You must log in to answer this question.

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