Extracting information from a URI

Question

I'm completely new to C and my first project is to create a simple cURL-like HTTP request client... but first, I need to create a function that can parse a URL. I've created this function, and it seems to work quite well, but since I'm so new to the complexities of C, can someone check for any errors or to see if I can improve clock efficiency?

Example usage can be found here.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void filterURLstring(char *weburl, char **protocol, char **username, char **password, char **host, char **port, char **path);

int main() {
    char *text = "http://mhdfffffjddj:abc123@192.168.0.2:8888/servlet/rece/";
    char *protocol, *username, *password, *host, *port, *path;
    filterURLstring(text, &protocol, &username, &password, &host, &port, &path);
    printf("Protocol:\"%s\"\n", protocol);
    printf("Username:\"%s\"\n", username);
    printf("Password:\"%s\"\n", password);
    printf("Host:\"%s\"\n", host);
    printf("Port:\"%s\"\n", port);
    printf("Path:\"%s\"\n", path);
    free(protocol);
    free(username);
    free(password);
    free(host);
    free(port);
    free(path);
    return 0;
}

void filterURLstring(char *weburl, char **protocol, char **username, char **password, char **host, char **port, char **path) {
    char *postprotocol, *postuserinfo, *postusername, *login, *posthost, *posthostname, *hostloc, *postport;
    int isPath;
    if ((postprotocol = strstr(weburl, "://")) == NULL) {
        printf("Input URL is invalid. Exiting...\n");
        exit(1);
    }
    *protocol = malloc(sizeof(char) * (postprotocol - weburl + 1));
    strncpy(*protocol, weburl, postprotocol - weburl);
    postprotocol = (postprotocol + (*(postprotocol+3)=='/'?4:3));
    if ((postuserinfo = strstr(postprotocol, "@")) != NULL) {
        if ((postusername = strchr(postprotocol, ':')) != NULL) {
            *password = malloc(sizeof(char) * (postuserinfo - postusername + 1));
            strncpy(*password, postusername + 1, (postuserinfo - postusername - 1));
        } else {
            *password = NULL;
        }
        int userlen = (postusername?postusername:postuserinfo) - postprotocol;
        *username = malloc(sizeof(char) *  userlen + 1);
        strncpy(*username, postprotocol, userlen);
    }
    hostloc = (postuserinfo?postuserinfo + 1:postprotocol);
    if ((posthost = strchr(hostloc, '/')) == NULL) {
        posthost = hostloc + strlen(hostloc);
        isPath = 1;
    }
    if ((posthostname = strchr(hostloc, ':')) == NULL) {
        posthostname = posthost;
    }
    *host = malloc(sizeof(char) * (posthostname - hostloc + 1));
    strncpy(*host, hostloc, (posthostname - hostloc));
    if (posthostname != posthost) {
        posthostname++;
        *port = malloc(sizeof(char) * (posthost - posthostname + 1));
        strncpy(*port, posthostname, (posthost - posthostname));
    } else {
        *port = malloc(sizeof(char) * 2 + 1);
        strncpy(*port, "80", 2);
    }
    if (isPath) {
        *path = malloc(sizeof(char) * strlen(posthost));
        strncpy(*path, posthost, strlen(posthost));
    } else {
        *path = calloc(1, 1);
    }
}

---

I changed my code according to the answer. It can be seen here.

Answer 1

Think in terms of an Abstract Data Type (ADT)

@x79 is right about using a struct for this. In fact, it makes sense to think of this in terms of an abstract data type, as defined in Code Complete:

An abstract data type is a collection of data and operations that work on that data.

The collection of data is the URL info, wrapped in a struct, and the operations can be:

UrlInfo * createUrlInfo(const char *);
deleteUrlInfo(UrlInfo *);
isValidUrlInfo(UrlInfo *);
printUrlInfo(UrlInfo *);

This collection of methods together form an abstract data type. They encapsulate the operations you need, while hiding the implementation details. Users of a UrlInfo don't need to know how the operations work, they just need to know what the operations are in the ADT (create / delete / isValid / print). These methods closely collaborate with each other, emulating class-like behavior in object oriented languages.

Armed with this ADT, your main function can be rewritten in a more functional, natural way:

int main() {
    const char * text = "http://mhdfffffjddj:abc123@192.168.0.2:8888/servlet/rece/";
    UrlInfo * urlInfo = createUrlInfo(text);
    if (isValidUrlInfo(urlInfo)) {
        printUrlInfo(urlInfo);
    }
    deleteUrlInfo(urlInfo);
}

The purpose of the isValidUrlInfo is to move the exit statement out of the current implementation. It's not good when a function exits in the middle of the program. Controlling the program flow is not the job of a function parsing a URL. The parser should just parse, and let the caller decide what to do with an invalid input.

Other improvements

port make would make sense to store in an int, not a char*

Instead of hardcoding the URL used by the program, it would be much more useful to take it as a command line argument, for example:

int main(int argc, char ** argv) {
    const char * text = argv[1];
    UrlInfo * urlInfo = createUrlInfo(text);
    if (isValidUrlInfo(urlInfo)) {
        printUrlInfo(urlInfo);
    }
    deleteUrlInfo(urlInfo);
}

No need to do return 0 at the end of main, the compiler automatically adds that.

This line is too tightly packed:

    int userlen = (postusername?postusername:postuserinfo) - postprotocol;

Use spaces around operators to make it more readable, like this:

    int userlen = (postusername ? postusername : postuserinfo) - postprotocol;

Suggested implementation

Putting some of the above points together, a Url Info abstract data type and a main method using it might look like this:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct UrlInfo {
    char * protocol;
    char * username;
    char * password;
    char * host;
    char * port;
    char * path;
} UrlInfo;

UrlInfo * createUrlInfo(const char *);
void deleteUrlInfo(UrlInfo *);
int isValidUrlInfo(UrlInfo *);
void printUrlInfo(UrlInfo *);

int main(int argc, char ** argv) {
    int i;
    UrlInfo * urlInfo;
    for (i = 1; i < argc; ++i) {
        urlInfo = createUrlInfo(argv[i]);
        if (isValidUrlInfo(urlInfo)) {
            printUrlInfo(urlInfo);
        } else {
            printf("Input URL is invalid: %s\n", argv[i]);
        }
        deleteUrlInfo(urlInfo);
        puts("");
    }
}

UrlInfo * createUrlInfo(const char * weburl) {
    char *postprotocol, *postuserinfo, *postusername, *login, *posthost, *posthostname, *hostloc, *postport;
    UrlInfo * urlInfo = malloc(sizeof(UrlInfo));

    urlInfo->protocol = NULL;
    urlInfo->username = NULL;
    urlInfo->password = NULL;
    urlInfo->host = NULL;
    urlInfo->port = NULL;
    urlInfo->path = NULL;

    int isPath;
    if ((postprotocol = strstr(weburl, "://")) == NULL) {
        return urlInfo;
    }
    urlInfo->protocol = malloc(sizeof(char) * (postprotocol - weburl + 1));
    strncpy(urlInfo->protocol, weburl, postprotocol - weburl);
    postprotocol = (postprotocol + (*(postprotocol + 3) == '/' ? 4 : 3));
    if ((postuserinfo = strstr(postprotocol, "@")) != NULL) {
        if ((postusername = strchr(postprotocol, ':')) != NULL) {
            urlInfo->password = malloc(sizeof(char) * (postuserinfo - postusername + 1));
            strncpy(urlInfo->password, postusername + 1, (postuserinfo - postusername - 1));
        } else {
            urlInfo->password = NULL;
        }
        int userlen = (postusername?postusername:postuserinfo) - postprotocol;
        urlInfo->username = malloc(sizeof(char) *  userlen + 1);
        strncpy(urlInfo->username, postprotocol, userlen);
    }
    hostloc = (postuserinfo?postuserinfo + 1:postprotocol);
    if ((posthost = strchr(hostloc, '/')) == NULL) {
        posthost = hostloc + strlen(hostloc);
        isPath = 1;
    }
    if ((posthostname = strchr(hostloc, ':')) == NULL) {
        posthostname = posthost;
    }
    urlInfo->host = malloc(sizeof(char) * (posthostname - hostloc + 1));
    strncpy(urlInfo->host, hostloc, (posthostname - hostloc));
    if (posthostname != posthost) {
        posthostname++;
        urlInfo->port = malloc(sizeof(char) * (posthost - posthostname + 1));
        strncpy(urlInfo->port, posthostname, (posthost - posthostname));
    } else {
        urlInfo->port = malloc(sizeof(char) * 2 + 1);
        strncpy(urlInfo->port, "80", 2);
    }
    if (isPath) {
        urlInfo->path = malloc(sizeof(char) * strlen(posthost));
        strncpy(urlInfo->path, posthost, strlen(posthost));
    } else {
        urlInfo->path = calloc(1, 1);
    }
    return urlInfo;
}

void deleteUrlInfo(UrlInfo * urlInfo) {
    free(urlInfo->protocol);
    free(urlInfo->username);
    free(urlInfo->password);
    free(urlInfo->host);
    free(urlInfo->port);
    free(urlInfo->path);
    free(urlInfo);
}

int isValidUrlInfo(UrlInfo * urlInfo) {
    return urlInfo != NULL && urlInfo->protocol != NULL;
}

void printUrlInfo(UrlInfo * urlInfo) {
    printf("Protocol:\"%s\"\n", urlInfo->protocol);
    printf("Username:\"%s\"\n", urlInfo->username);
    printf("Password:\"%s\"\n", urlInfo->password);
    printf("Host:\"%s\"\n", urlInfo->host);
    printf("Port:\"%s\"\n", urlInfo->port);
    printf("Path:\"%s\"\n", urlInfo->path);
}

Answer 2

Typically you create a function to reduce redundancy, meaning you are going to call it repeatedly. Here you only call the function once so you should put everything into the main function. This way you could avoid using malloc() and free() and the 7 extra pointer variables.

If you need to keep the function (maybe this module is going to be expanded upon?) then you should create a struct for the URI parts and pass the struct to the function instead of 7 separate variables.

Also, it looks like may need to change this line

*path = malloc(sizeof(char) * strlen(posthost));

to

*path = malloc(sizeof(char) * strlen(posthost) + 1);