5

I have this parser, I want to use the size of the pointer (that is 8 bytes) in the function strncpy, I was able do it with the "method" part, why does it crash the "path" part ?

It tells me Segmentation fault (core dumped) in the path part, so it does not have the authorization?

Am I missing something ?

int http_parser(char *request) {

    char tmp_request[BUFFER_SIZE];
   
    char *method;
    char *path;
       
    strncpy(tmp_request, request, sizeof(tmp_request));

    char *token = strtok(tmp_request, " ");

    if (token == NULL) {
        printf("\nInvalid Request Format!");
        return 0;
    }

    //METODO
    strncpy(method, token, sizeof(method));
  
    token = strtok(NULL, " ");
    //printf("METODO!\n");
    if (token == NULL) {
        printf("Invalid Request Method!\n"); 
        return 0;
    } else {
        printf("Metodo : %s\n",method);
    }

    //path
    strncpy(path, token, sizeof(path));
    token = strtok(NULL, " ");

    printf("PATH\n");
    if (token == NULL) {
        printf("Invalid Request Path\n");
        return 0;
    } else {
        printf("Path : %s\n",path);
    }
}

I tried using sizeof(8) and it works, but I want to know why it doesn't work when I do it this way.

3
  • 5
    method is uninitialized (and does not point to allocated memory) when it is used in strncpy(method, token, sizeof(method));. Also sizeof(method) is the size of the pointer, not the data it's supposed to be pointing to. Commented Aug 2 at 14:19
  • you can replace your wrong calls of strncpy by strdup for instance (but warning, that dynamically allocate memory, so you will have to free them later). But if the goal is just to print the tokens, just print token each time Commented Aug 2 at 14:26
  • Note that the first strncpy() doesn't null-terminate the copied string if it is bigger than the array. GCC with enough warnings enabled will point that out. Commented Aug 2 at 14:30

2 Answers 2

6

strncpy(method, token, sizeof(method)) is incorrect in multiple respects:

  • the destination pointer method is uninitialized: the code has undefined behavior, and does indeed produce a segmentation fault, ie: it attempts to write to an invalid address (a segment of memory that is not mapped to the program space, causing a fault, a software exception handled by the OS, which is rejected and this causes an abrupt program termination).
  • sizeof(method) is the size of the destination pointer, 8 bytes on your 64-bit system, but strncpy expects the length of the destination array, which you can only get with sizeof if method is defined as an array, not a pointer.
  • strncpy may produce a string in the destination array that is not null terminated (no '\0' byte at the end) if the source string is too long: this will cause undefined behavior when you later pass this array to strtok or another string function.

Here are some recommendations:

  • Instead of making a copy of the string and using strtok to parse it, use strcspn and strspn to parse the original string. More generally, never use strncpy() because it is misunderstood and error prone and avoid strtok() because it is non-reentrant and possibly not thread safe and modifies the first string argument.

  • Learn the difference between pointers and arrays. Only use sizeof on arrays to get their length.

  • Always compile with extra warnings and make them fatal: this will save hours of debugging time. gcc -Wall -Wextra -Werror, /W4 with microsoft compilers.

  • There is no standard string function to copy with truncation. BSD systems have strlcpy for this purpose. You can define and use this alternative:

    size_t pstrcpy(char *dest, size_t size, const char *src) {
         if (!size) return 0;
         for (size_t i = 0; i < size; i++) {
             if ((dest[i] = src[i]) == '\0';
                 return i;
         }
         dest[size - 1] = '\0';  // perform truncation
         return size;
    }
    

    Truncation can be detected easily: the return value is the number of characters if the string fits and the size of the array if the string was truncated.

Here is a modified version of your code:

#include <string.h>

// this is a replacement for strtok()
// return a pointer to the next word and store the length to len
// advance *pp past the word
// return NULL if no word, otherwise return a pointer to the word
const char *next_token(const char *s, const char *seps,
                       size_t *len, const char **pp)
{
    // if NULL string, use state pointer
    if (!s) {
        s = *pp;
    }
    // skip separators
    s += strspn(s, seps);
    // get token length
    *len = strcspn(s, seps);
    // update state
    *pp = s + *len;
    // return pointer to token or NULL if none left
    return *s ? s : NULL;
}

int http_parser(const char *request) {
    const char *method;
    const char *path;
    size_t method_len, path_len;
    const char *p;

    method = next_token(request, " ", &method_len, &p);
    if (method == NULL) {
        printf("Invalid request method!\n");
        return 0;
    } else {
        printf("method : %.*s\n", (int)method_len, method);
    }

    path = next_token(NULL, " ", &path_len, &p);
    if (path == NULL) {
        printf("Invalid request path!\n");
        return 0;
    } else {
        printf("path : %.*s\n", (int)path_len, path);
    }
    return 1;
}
Sign up to request clarification or add additional context in comments.

Comments

4

There are several errors here.

First, method and path are uninitialized and therefore don't point to a valid buffer. Using strncpy to write into them triggers undefined behavior in your code. In the first case it appears to work properly while in the second case it causes a crash.

Second, sizeof(method) and sizeof(path) give you the size of a pointer (most likely 8), while you probably want the size of the buffer.

If you were to fix this by making them arrays:

char method[BUFFER_SIZE];
char path[BUFFER_SIZE];

This would give you space to write to, and the above sizeof expressions would give you a proper value.

This still isn't entirely proper however, as strncpy doesn't null-terminate the destination string in all cases, meaning you'd have to manually add one in the array's last element. In this particular case though it isn't required because the source string is known to be smaller than the destination so the null termination will in fact happen.

All that being said, you don't actually have to copy anything at all. Since strtok inserts null bytes into the source string to tokenize it, you can simply use pointers instead of arrays and just point them to the proper places in the source.

int http_parser(char *request) {

    char tmp_request[BUFFER_SIZE];

    char *method;
    char *path;

    strncpy(tmp_request, request, sizeof(tmp_request));

    char *token = strtok(tmp_request, " ");

    if (token == NULL) {
        printf("\nInvalid Request Format!");
        return 0;
    }

    method = token;
    printf("Method : %s\n",method);

    token = strtok(NULL, " ");
    if (token == NULL) {
        printf("Invalid Request Path\n");
        return 0;
    }

    path = token;
    printf("Path : %s\n",path);
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.