improve JID parsing according to RFC7622

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>

src/jid.c

@@ -37,8 +37,10 @@ char *xmpp_jid_new(xmpp_ctx_t *ctx,
     size_t len, nlen, dlen, rlen;
 
     /* jid must at least have a domain */
-    if (domain == NULL)
+    if (domain == NULL) {
+        strophe_error(ctx, "jid", "domainpart missing.");
         return NULL;
+    }
 
     /* accumulate lengths */
     dlen = strlen(domain);
@@ -46,6 +48,26 @@ char *xmpp_jid_new(xmpp_ctx_t *ctx,
     rlen = (resource) ? strlen(resource) + 1 : 0;
     len = nlen + dlen + rlen;
 
+    if (dlen > 1023) {
+        strophe_error(ctx, "jid", "domainpart too long.");
+        return NULL;
+    }
+    if (nlen > 1024) {
+        strophe_error(ctx, "jid", "localpart too long.");
+        return NULL;
+    }
+    if (rlen > 1024) {
+        strophe_error(ctx, "jid", "resourcepart too long.");
+        return NULL;
+    }
+
+    if (node) {
+        if (strcspn(node, "\"&'/:<>@") != nlen - 1) {
+            strophe_error(ctx, "jid", "localpart contained invalid character.");
+            return NULL;
+        }
+    }
+
     /* concat components */
     result = strophe_alloc(ctx, len + 1);
     if (result != NULL) {
@@ -96,17 +118,29 @@ char *xmpp_jid_bare(xmpp_ctx_t *ctx, const char *jid)
  */
 char *xmpp_jid_node(xmpp_ctx_t *ctx, const char *jid)
 {
+    char *dup_jid = strophe_strdup(ctx, jid);
     char *result = NULL;
     const char *c;
 
-    c = strchr(jid, '@');
+    /* Apply the same parsing rules from rfc7622 Section 3.2
+     * 1. Strip resource
+     * 2. take part before the '@'
+     */
+
+    char *resource = strchr(dup_jid, '/');
+    if (resource != NULL) {
+        *resource = '\0';
+    }
+
+    c = strchr(dup_jid, '@');
     if (c != NULL) {
-        result = strophe_alloc(ctx, (c - jid) + 1);
+        result = strophe_alloc(ctx, (c - dup_jid) + 1);
         if (result != NULL) {
-            memcpy(result, jid, (c - jid));
+            memcpy(result, dup_jid, (c - dup_jid));
-            result[c - jid] = '\0';
+            result[c - dup_jid] = '\0';
         }
     }
+    strophe_free(ctx, dup_jid);
 
     return result;
 }
@@ -120,24 +154,29 @@ char *xmpp_jid_node(xmpp_ctx_t *ctx, const char *jid)
  */
 char *xmpp_jid_domain(xmpp_ctx_t *ctx, const char *jid)
 {
-    char *result = NULL;
+    char *dup_jid = strophe_strdup(ctx, jid);
-    const char *c;
-    size_t dlen;
 
-    c = strchr(jid, '@');
+    /* rfc7622 Section 3.2
-    if (c == NULL) {
+     * 1.  Remove any portion from the first '/' character to the end of the
-        /* no node, assume domain */
+     *     string (if there is a '/' character present).
-        c = jid;
+     */
+
+    char *resource = strchr(dup_jid, '/');
+    if (resource != NULL) {
+        *resource = '\0';
+    }
+
+    /* 2.  Remove any portion from the beginning of the string to the first
+     *     '@' character (if there is an '@' character present).
+     */
+    char *at_sign = strchr(dup_jid, '@');
+    char *result = NULL;
+    if (at_sign != NULL) {
+        result = strophe_strdup(ctx, (at_sign + 1));
     } else {
-        /* advance past the separator */
+        result = strophe_strdup(ctx, dup_jid);
-        c++;
-    }
-    dlen = strcspn(c, "/"); /* do not include resource */
-    result = strophe_alloc(ctx, dlen + 1);
-    if (result != NULL) {
-        memcpy(result, c, dlen);
-        result[dlen] = '\0';
     }
+    strophe_free(ctx, dup_jid);
 
     return result;
 }

tests/test.h

@@ -34,17 +34,19 @@
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 #endif
 
-#define COMPARE(v1, v2)                                  \
+#define COMPARE(v1, v2)                                         \
-    do {                                                 \
+    do {                                                        \
-        const char *__v1 = v1;                           \
+        const char *__v1 = v1;                                  \
-        const char *__v2 = v2;                           \
+        const char *__v2 = v2;                                  \
-        if (!__v1 || !__v2 || strcmp(__v1, __v2) != 0) { \
+        if ((__v1 == NULL) && (__v2 == NULL)) {                 \
-            printf("Error:    %s\n"                      \
+            /* noop */                                          \
-                   "Expected: %s\n"                      \
+        } else if (!__v1 || !__v2 || strcmp(__v1, __v2) != 0) { \
-                   "Got:      %s\n",                     \
+            printf("Error:    %s\n"                             \
-                   #v1 " != " #v2, __v1, __v2);          \
+                   "Expected: %s\n"                             \
-            exit(1);                                     \
+                   "Got:      %s\n",                            \
-        }                                                \
+                   #v1 " != " #v2, __v1, __v2);                 \
+            exit(1);                                            \
+        }                                                       \
     } while (0)
 
 #define COMPARE_BUF(v1, len1, v2, len2)                                \

tests/test_jid.c

@@ -15,114 +15,80 @@
 #include "strophe.h"
 #include "common.h"
 
-static const char jid1[] = "foo@bar.com";
+#include "test.h"
-static const char jid2[] = "anyone@example.com/hullo";
-static const char jid3[] = "manic.porter@xyz.net/frob";
-static const char jid4[] = "domain.tld";
 
 static const char *_s(const char *s)
 {
     return s == NULL ? "<NULL>" : s;
 }
 
-int test_jid(xmpp_ctx_t *ctx)
+static int test_jid(xmpp_ctx_t *ctx)
 {
     char *bare;
     char *node;
     char *domain;
     char *resource;
+    size_t n;
+    struct {
+        const char *jid;
+        const char *bare;
+        const char *node;
+        const char *domain;
+        const char *resource;
+    } testcases[] = {
+        {"foo@bar.com", "foo@bar.com", "foo", "bar.com", NULL},
+        {
+            "anyone@example.com/hullo",
+            "anyone@example.com",
+            "anyone",
+            "example.com",
+            "hullo",
+        },
+        {
+            "a.example.com/b@example.net",
+            "a.example.com",
+            NULL,
+            "a.example.com",
+            "b@example.net",
+        },
+        {
+            "manic.porter@xyz.net/frob",
+            "manic.porter@xyz.net",
+            "manic.porter",
+            "xyz.net",
+            "frob",
+        },
+        {
+            "domain.tld",
+            "domain.tld",
+            NULL,
+            "domain.tld",
+            NULL,
+        },
+    };
 
-    bare = xmpp_jid_bare(ctx, jid1);
+    for (n = 0; n < sizeof(testcases) / sizeof(testcases[0]); ++n) {
-    node = xmpp_jid_node(ctx, jid1);
+        bare = xmpp_jid_bare(ctx, testcases[n].jid);
-    domain = xmpp_jid_domain(ctx, jid1);
+        node = xmpp_jid_node(ctx, testcases[n].jid);
-    resource = xmpp_jid_resource(ctx, jid1);
+        domain = xmpp_jid_domain(ctx, testcases[n].jid);
-    printf("jid '%s' parsed to %s, %s, %s\n", jid1, _s(node), _s(domain),
+        resource = xmpp_jid_resource(ctx, testcases[n].jid);
-           _s(resource));
+        printf("jid '%s' parsed to %s, %s, %s\n", testcases[n].jid, _s(node),
-    if (bare == NULL || strcmp(bare, "foo@bar.com"))
+               _s(domain), _s(resource));
-        return 1;
+        COMPARE(testcases[n].bare, bare);
-    if (node == NULL || strcmp(node, "foo"))
+        COMPARE(testcases[n].node, node);
-        return 1;
+        COMPARE(testcases[n].domain, domain);
-    if (domain == NULL || strcmp(domain, "bar.com"))
+        COMPARE(testcases[n].resource, resource);
-        return 1;
+        if (bare)
-    if (resource != NULL)
+            strophe_free(ctx, bare);
-        return 1;
+        if (node)
-    if (bare)
+            strophe_free(ctx, node);
-        strophe_free(ctx, bare);
+        if (domain)
-    if (node)
+            strophe_free(ctx, domain);
-        strophe_free(ctx, node);
+        if (resource)
-    if (domain)
+            strophe_free(ctx, resource);
-        strophe_free(ctx, domain);
+    }
-    if (resource)
-        strophe_free(ctx, resource);
 
-    bare = xmpp_jid_bare(ctx, jid2);
+    printf("test_jid() finished successfully\n");
-    node = xmpp_jid_node(ctx, jid2);
-    domain = xmpp_jid_domain(ctx, jid2);
-    resource = xmpp_jid_resource(ctx, jid2);
-    printf("jid '%s' parsed to %s, %s, %s\n", jid2, _s(node), _s(domain),
-           _s(resource));
-    if (bare == NULL || strcmp(bare, "anyone@example.com"))
-        return 1;
-    if (node == NULL || strcmp(node, "anyone"))
-        return 1;
-    if (domain == NULL || strcmp(domain, "example.com"))
-        return 1;
-    if (resource == NULL || strcmp(resource, "hullo"))
-        return 1;
-    if (bare)
-        strophe_free(ctx, bare);
-    if (node)
-        strophe_free(ctx, node);
-    if (domain)
-        strophe_free(ctx, domain);
-    if (resource)
-        strophe_free(ctx, resource);
-
-    bare = xmpp_jid_bare(ctx, jid3);
-    node = xmpp_jid_node(ctx, jid3);
-    domain = xmpp_jid_domain(ctx, jid3);
-    resource = xmpp_jid_resource(ctx, jid3);
-    printf("jid '%s' parsed to %s, %s, %s\n", jid3, _s(node), _s(domain),
-           _s(resource));
-    if (bare == NULL || strcmp(bare, "manic.porter@xyz.net"))
-        return 1;
-    if (node == NULL || strcmp(node, "manic.porter"))
-        return 1;
-    if (domain == NULL || strcmp(domain, "xyz.net"))
-        return 1;
-    if (resource == NULL || strcmp(resource, "frob"))
-        return 1;
-    if (bare)
-        strophe_free(ctx, bare);
-    if (node)
-        strophe_free(ctx, node);
-    if (domain)
-        strophe_free(ctx, domain);
-    if (resource)
-        strophe_free(ctx, resource);
-
-    bare = xmpp_jid_bare(ctx, jid4);
-    node = xmpp_jid_node(ctx, jid4);
-    domain = xmpp_jid_domain(ctx, jid4);
-    resource = xmpp_jid_resource(ctx, jid4);
-    printf("jid '%s' parsed to %s, %s, %s\n", jid4, _s(node), _s(domain),
-           _s(resource));
-    if (bare == NULL || strcmp(bare, "domain.tld"))
-        return 1;
-    if (node != NULL)
-        return 1;
-    if (domain == NULL || strcmp(domain, "domain.tld"))
-        return 1;
-    if (resource != NULL)
-        return 1;
-    if (bare)
-        strophe_free(ctx, bare);
-    if (node)
-        strophe_free(ctx, node);
-    if (domain)
-        strophe_free(ctx, domain);
-    if (resource)
-        strophe_free(ctx, resource);
 
     return 0;
 }
@@ -143,6 +109,19 @@ int test_jid_new(xmpp_ctx_t *ctx)
         return 1;
     strophe_free(ctx, jid);
 
+    const char *invalid_chars = "\"&'/:<>@";
+    char localpart[] = "localpart";
+    do {
+        localpart[1] = *invalid_chars;
+        jid = xmpp_jid_new(ctx, localpart, "bar.com", NULL);
+        if (jid != NULL) {
+            printf("Shouldn't have created JID with localpart=\"%s\"\n",
+                   localpart);
+            return 1;
+        }
+        invalid_chars++;
+    } while (*invalid_chars != '\0');
+
     return 0;
 }