Normalize both URLs and local paths

("Normalization" = collapsing `.` and `..`)

Before, only local paths were being normalized. This was because, by
some twisted logic, I was under the impression that `https://a.b.c/d`
and `https://a.b.c/d/.` are technically supposed to be different
identifiers. My recent visit to RFC3986 disproved this.

But regardless of what the standards say, it doesn't make sense to treat
those URLs differently in the context of RP validation. They will be
cached into the same directory.

Not that this aspect of the code used to be incorrect, to the best of
my knowledge. Fort needs normalization for proper pre-download
identification (which in turn is needed to prevent a file from being
downloaded twice during the same iteration). Old code used to identify
by local path, and the upcoming session desync commit will need to index
by remote path. Hence the need for this change.

diff --git a/src/alloc.c b/src/alloc.c
index 00a1b09cb2ffbb23192b54f818ce12708e145f85..8bc33ff5dca19c751f8736474c2dec3abed825ad 100644 (file)
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -60,3 +60,15 @@ pstrdup(const char *s)
 
        return result;
 }
+
+char *
+pstrndup(char const *s, size_t size)
+{
+       char *result;
+
+       result = strndup(s, size);
+       if (result == NULL)
+               enomem_panic();
+
+       return result;
+}

diff --git a/src/alloc.h b/src/alloc.h
index b9e34e5a55c6b94c64e9640e986c9f7b1fe571d0..5bd04975b9dac16777d4858bd845e7786093026f 100644 (file)
--- a/src/alloc.h
+++ b/src/alloc.h
@@ -20,5 +20,6 @@ void *prealloc(void *ptr, size_t size);
 
 /* strdup(), but panic on allocation failure. */
 char *pstrdup(char const *s);
+char *pstrndup(char const *s, size_t size);
 
 #endif /* SRC_ALLOC_H_ */

diff --git a/src/cache/local_cache.c b/src/cache/local_cache.c
index 3f431d676e5d890ccdf6c74a69600ea54b76a26d..9d98bd4bf1d4daeb1903b49864302ce297b73c24 100644 (file)
--- a/src/cache/local_cache.c
+++ b/src/cache/local_cache.c
@@ -482,15 +482,12 @@ static int
 get_url(struct rpki_uri *uri, const char *tal, struct rpki_uri **url)
 {
        char const *guri, *c;
-       char *guri2;
+       char *gcopy;
        unsigned int slashes;
        int error;
 
-       if (uri_get_type(uri) != UT_RPP) {
-               uri_refget(uri);
-               *url = uri;
-               return 0;
-       }
+       if (uri_get_type(uri) != UT_RPP)
+               goto reuse_uri;
 
        /*
         * Careful with this code. rsync(1):
@@ -534,6 +531,9 @@ get_url(struct rpki_uri *uri, const char *tal, struct rpki_uri **url)
         * But note: This only works if we're synchronizing a directory.
         * But this is fine, because this hack stacks with the minimum common
         * path performance hack.
+        *
+        * Minimum common path performance hack: rsync the rsync module root,
+        * not every RPP separately. The former is much faster.
         */
 
        guri = uri_get_global(uri);
@@ -541,27 +541,35 @@ get_url(struct rpki_uri *uri, const char *tal, struct rpki_uri **url)
        for (c = guri; *c != '\0'; c++) {
                if (*c == '/') {
                        slashes++;
-                       if (slashes == 4)
-                               return __uri_create(url, tal, UT_RPP,
-                                   NULL, guri, c - guri + 1);
+                       if (slashes == 4) {
+                               if (c[1] == '\0')
+                                       goto reuse_uri;
+                               gcopy = pstrndup(guri, c - guri + 1);
+                               goto gcopy2url;
+                       }
                }
        }
 
-       if (slashes == 3 && *(c - 1) != '/') {
-               guri2 = pstrdup(guri); /* Remove const */
-               guri2[c - guri] = '/';
-               error = __uri_create(url, tal, UT_RPP, NULL, guri2,
-                   c - guri + 1);
-               free(guri2);
-               return error;
+       if (slashes == 3 && c[-1] != '/') {
+               gcopy = pmalloc(c - guri + 2);
+               memcpy(gcopy, guri, c - guri);
+               gcopy[c - guri] = '/';
+               gcopy[c - guri + 1] = '\0';
+               goto gcopy2url;
        }
 
-       /*
-        * Minimum common path performance hack: rsync the rsync module root,
-        * not every RPP separately. The former is much faster.
-        */
        return pr_val_err("Can't rsync URL '%s': The URL seems to be missing a domain or rsync module.",
            guri);
+
+reuse_uri:
+       uri_refget(uri);
+       *url = uri;
+       return 0;
+
+gcopy2url:
+       error = uri_create(url, tal, UT_RPP, NULL, gcopy);
+       free(gcopy);
+       return error;
 }
 
 static bool

diff --git a/src/data_structure/path_builder.c b/src/data_structure/path_builder.c
index a9ab658c0a2be436a53ad32767544b6274c75efb..5dbee72c2a74fa46a466fcc04528b2035f84b21d 100644 (file)
--- a/src/data_structure/path_builder.c
+++ b/src/data_structure/path_builder.c
@@ -15,12 +15,13 @@
 #endif
 #define MAX_CAPACITY 4096u
 
+/* @reserve needs to be < INITIAL_CAPACITY. */
 void
-pb_init(struct path_builder *pb)
+__pb_init(struct path_builder *pb, size_t reserve)
 {
        pb->string = pmalloc(INITIAL_CAPACITY);
-       pb->string[0] = 0;
-       pb->len = 0;
+       pb->string[reserve] = 0;
+       pb->len = reserve;
        pb->capacity = INITIAL_CAPACITY;
 }
 

diff --git a/src/data_structure/path_builder.h b/src/data_structure/path_builder.h
index 89989d8380e3bf3af8bf8a0ff877751db160c409..45713c8ccaab5bfee2f0f5df2631a02d29038cfa 100644 (file)
--- a/src/data_structure/path_builder.h
+++ b/src/data_structure/path_builder.h
@@ -9,7 +9,8 @@ struct path_builder {
        size_t capacity;
 };
 
-void pb_init(struct path_builder *);
+void __pb_init(struct path_builder *, size_t);
+#define pb_init(pb) __pb_init(pb, 0)
 int pb_init_cache(struct path_builder *, char const *, char const *);
 
 /*

diff --git a/src/object/certificate.c b/src/object/certificate.c
index bd4dc248f26d7107de8d7f1cd3258d9c2c62b553..920a561ccd02d592512b82a84d31105b91943456 100644 (file)
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -1479,11 +1479,12 @@ end:
  * Create @uri from the @ad
  */
 static int
-uri_create_ad(struct rpki_uri **uri, ACCESS_DESCRIPTION *ad, enum uri_type type,
-    bool is_notif)
+uri_create_ad(struct rpki_uri **uri, ACCESS_DESCRIPTION *ad, enum uri_type type)
 {
        ASN1_STRING *asn1str;
+       char *str;
        int ptype;
+       int error;
 
        asn1str = GENERAL_NAME_get0_value(ad->location, &ptype);
 
@@ -1513,7 +1514,7 @@ uri_create_ad(struct rpki_uri **uri, ACCESS_DESCRIPTION *ad, enum uri_type type,
        /*
         * GEN_URI signals an IA5String.
         * IA5String is a subset of ASCII, so this cast is safe.
-        * No guarantees of a NULL chara, though.
+        * No guarantees of a NULL chara though, which is why we need a dup.
         *
         * TODO (testers) According to RFC 5280, accessLocation can be an IRI
         * somehow converted into URI form. I don't think that's an issue
@@ -1522,9 +1523,15 @@ uri_create_ad(struct rpki_uri **uri, ACCESS_DESCRIPTION *ad, enum uri_type type,
         * directory our g2l version of @asn1_string should contain.
         * But ask the testers to keep an eye on it anyway.
         */
-       return __uri_create(uri,
-           tal_get_file_name(validation_tal(state_retrieve())), type, NULL,
-           ASN1_STRING_get0_data(asn1str), ASN1_STRING_length(asn1str));
+       str = pstrndup((char const *)ASN1_STRING_get0_data(asn1str),
+           ASN1_STRING_length(asn1str));
+
+       error = uri_create(uri,
+           tal_get_file_name(validation_tal(state_retrieve())),
+           type, NULL, str);
+
+       free(str);
+       return error;
 }
 
 /**
@@ -1558,8 +1565,7 @@ handle_ad(int nid, struct ad_metadata const *meta, SIGNATURE_INFO_ACCESS *ia,
        for (i = 0; i < sk_ACCESS_DESCRIPTION_num(ia); i++) {
                ad = sk_ACCESS_DESCRIPTION_value(ia, i);
                if (OBJ_obj2nid(ad->method) == nid) {
-                       error = uri_create_ad(&uri, ad, meta->type,
-                           meta == &RPKI_NOTIFY);
+                       error = uri_create_ad(&uri, ad, meta->type);
                        switch (error) {
                        case 0:
                                break;

diff --git a/src/object/manifest.c b/src/object/manifest.c
index f527db86bbb559d7ca9ca43e472b81230f5dabe1..02b12c7c20b507f463a9ef81bb042e27fc12fe45 100644 (file)
--- a/src/object/manifest.c
+++ b/src/object/manifest.c
@@ -25,7 +25,7 @@ cage(struct rpki_uri **uri, struct rpki_uri *notif)
 
        return uri_create_caged(uri,
            tal_get_file_name(validation_tal(state_retrieve())), notif,
-           uri_get_global(*uri), uri_get_global_len(*uri));
+           uri_get_global(*uri));
 }
 
 static int

diff --git a/src/types/uri.c b/src/types/uri.c
index e381bf1823d81315d245b32489840f997cd6088d..753c3b5c2e6fcb418bcb7e3a587af15d39cda4c4 100644 (file)
--- a/src/types/uri.c
+++ b/src/types/uri.c
@@ -35,8 +35,6 @@ struct rpki_uri {
         * non-ASCII characters.
         */
        char *global;
-       /** Length of @global. */
-       size_t global_len;
 
        /**
         * "Local URI".
@@ -107,27 +105,65 @@ validate_url_character(int character)
            : pr_val_err("URL has non-printable character code '%d'.", character);
 }
 
+static int append_guri(struct path_builder *, char const *, char const *,
+    int, bool);
+
 /**
- * Initializes @uri->global* by cloning @str.
- * This function does not assume that @str is null-terminated.
+ * Initializes @uri->global by building a normalized version of @str.
  */
 static int
-str2global(char const *str, size_t str_len, struct rpki_uri *uri)
+str2global(char const *str, struct rpki_uri *uri)
 {
+#define SCHEMA_LEN 8 /* strlen("rsync://"), strlen("https://") */
+
+       char const *s;
+       char const *pfx;
        int error;
-       size_t i;
+       struct path_builder pb;
 
-       for (i = 0; i < str_len; i++) {
-               error = validate_url_character(str[i]);
+       if (str == NULL){
+               uri->global = NULL;
+               return 0;
+       }
+
+       for (s = str; s[0] != '\0'; s++) {
+               error = validate_url_character(s[0]);
                if (error)
                        return error;
        }
 
-       uri->global = pmalloc(str_len + 1);
-       strncpy(uri->global, str, str_len);
-       uri->global[str_len] = '\0';
-       uri->global_len = str_len;
+       pfx = NULL;
+       error = 0;
+
+       switch (uri->type) {
+       case UT_TA_RSYNC:
+       case UT_RPP:
+       case UT_CAGED:
+       case UT_AIA:
+       case UT_SO:
+       case UT_MFT:
+               pfx = "rsync://";
+               error = ENOTRSYNC;
+               break;
+       case UT_TA_HTTP:
+       case UT_NOTIF:
+       case UT_TMP:
+               pfx = "https://";
+               error = ENOTHTTPS;
+               break;
+       }
+
+       if (pfx == NULL)
+               pr_crit("Unknown URI type: %u", uri->type);
+
+       __pb_init(&pb, SCHEMA_LEN - 1);
+       error = append_guri(&pb, str, pfx, error, true);
+       if (error) {
+               pb_cleanup(&pb);
+               return error;
+       }
 
+       uri->global = strncpy(pb.string, str, SCHEMA_LEN);
        return 0;
 }
 
@@ -207,7 +243,6 @@ ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5)
        joined[dir_len + ia5->size] = '\0';
 
        uri->global = joined;
-       uri->global_len = dir_len + ia5->size;
        return 0;
 }
 
@@ -315,9 +350,10 @@ get_rrdp_workspace(struct path_builder *pb, char const *tal,
        if (error)
                return error;
 
-       error = append_guri(pb, notif->global, "https://", ENOTHTTPS, true);
+       error = pb_append(pb, &notif->global[SCHEMA_LEN]);
        if (error)
                pb_cleanup(pb);
+
        return error;
 }
 
@@ -325,16 +361,16 @@ get_rrdp_workspace(struct path_builder *pb, char const *tal,
  * Maps "rsync://a.b.c/d/e.cer" into "<local-repository>/rsync/a.b.c/d/e.cer".
  */
 static int
-map_simple(struct rpki_uri *uri, char const *tal, char const *gprefix, int err)
+map_simple(struct rpki_uri *uri, char const *tal, char const *subdir)
 {
        struct path_builder pb;
        int error;
 
-       error = pb_init_cache(&pb, tal, NULL);
+       error = pb_init_cache(&pb, tal, subdir);
        if (error)
                return error;
 
-       error = append_guri(&pb, uri->global, gprefix, err, false);
+       error = pb_append(&pb, &uri->global[SCHEMA_LEN]);
        if (error) {
                pb_cleanup(&pb);
                return error;
@@ -358,16 +394,17 @@ map_caged(struct rpki_uri *uri, char const *tal, struct rpki_uri *notif)
        if (error)
                return error;
 
-       if (uri->global[0] == '\0')
-               goto end; /* Caller is only interested in the cage. */
+       if (uri->global == NULL)
+               goto success; /* Caller is only interested in the cage. */
 
-       error = append_guri(&pb, uri->global, "rsync://", ENOTRSYNC, true);
+       error = pb_append(&pb, &uri->global[SCHEMA_LEN]);
        if (error) {
                pb_cleanup(&pb);
                return error;
        }
 
-end:   uri->local = pb.string;
+success:
+       uri->local = pb.string;
        return 0;
 }
 
@@ -378,11 +415,11 @@ autocomplete_local(struct rpki_uri *uri, char const *tal,
        switch (uri->type) {
        case UT_TA_RSYNC:
        case UT_RPP:
-               return map_simple(uri, tal, "rsync://", ENOTRSYNC);
+               return map_simple(uri, tal, "rsync");
 
        case UT_TA_HTTP:
        case UT_NOTIF:
-               return map_simple(uri, tal, "https://", ENOTHTTPS);
+               return map_simple(uri, tal, "https");
 
        case UT_TMP:
                return cache_tmpfile(&uri->local);
@@ -403,24 +440,26 @@ autocomplete_local(struct rpki_uri *uri, char const *tal,
 /*
  * I think the reason why @guri is not a char * is to convey that it doesn't
  * need to be NULL terminated, but I'm not sure.
+ *
+ * FIXME callers now need to ensure @guri is NULL-terminated.
  */
 int
-__uri_create(struct rpki_uri **result, char const *tal, enum uri_type type,
-    struct rpki_uri *notif, void const *guri, size_t guri_len)
+uri_create(struct rpki_uri **result, char const *tal, enum uri_type type,
+    struct rpki_uri *notif, char const *guri)
 {
        struct rpki_uri *uri;
        int error;
 
        uri = pmalloc(sizeof(struct rpki_uri));
+       uri->type = type;
+       uri->references = 1;
 
-       error = str2global(guri, guri_len, uri);
+       error = str2global(guri, uri);
        if (error) {
                free(uri);
                return error;
        }
 
-       uri->type = type;
-
        error = autocomplete_local(uri, tal, notif);
        if (error) {
                free(uri->global);
@@ -428,8 +467,6 @@ __uri_create(struct rpki_uri **result, char const *tal, enum uri_type type,
                return error;
        }
 
-       uri->references = 1;
-
        *result = uri;
        return 0;
 }
@@ -446,6 +483,8 @@ uri_create_mft(struct rpki_uri **result, char const *tal,
        int error;
 
        uri = pmalloc(sizeof(struct rpki_uri));
+       uri->type = (notif == NULL) ? UT_RPP : UT_CAGED;
+       uri->references = 1;
 
        error = ia5str2global(uri, mft->global, ia5);
        if (error) {
@@ -453,8 +492,6 @@ uri_create_mft(struct rpki_uri **result, char const *tal,
                return error;
        }
 
-       uri->type = (notif == NULL) ? UT_RPP : UT_CAGED;
-
        error = autocomplete_local(uri, tal, notif);
        if (error) {
                free(uri->global);
@@ -462,8 +499,6 @@ uri_create_mft(struct rpki_uri **result, char const *tal,
                return error;
        }
 
-       uri->references = 1;
-
        *result = uri;
        return 0;
 }
@@ -514,12 +549,6 @@ uri_get_local(struct rpki_uri *uri)
        return uri->local;
 }
 
-size_t
-uri_get_global_len(struct rpki_uri *uri)
-{
-       return uri->global_len;
-}
-
 bool
 uri_equals(struct rpki_uri *u1, struct rpki_uri *u2)
 {
@@ -530,14 +559,17 @@ uri_equals(struct rpki_uri *u1, struct rpki_uri *u2)
 bool
 uri_has_extension(struct rpki_uri *uri, char const *ext)
 {
+       size_t uri_len;
        size_t ext_len;
        int cmp;
 
+       uri_len = strlen(uri->global);
        ext_len = strlen(ext);
-       if (uri->global_len < ext_len)
+
+       if (uri_len < ext_len)
                return false;
 
-       cmp = strncmp(uri->global + uri->global_len - ext_len, ext, ext_len);
+       cmp = strncmp(uri->global + uri_len - ext_len, ext, ext_len);
        return cmp == 0;
 }
 

diff --git a/src/types/uri.h b/src/types/uri.h
index dbfe9c323890ef9873a5779736dec6213434dc8a..3ba86704e1b3ec1c019306195b3a39b8d31f3017 100644 (file)
--- a/src/types/uri.h
+++ b/src/types/uri.h
@@ -19,18 +19,16 @@ enum uri_type {
 
 struct rpki_uri;
 
-int __uri_create(struct rpki_uri **, char const *, enum uri_type,
-    struct rpki_uri *, void const *, size_t);
+int uri_create(struct rpki_uri **, char const *, enum uri_type,
+    struct rpki_uri *, char const *);
 int uri_create_mft(struct rpki_uri **, char const *, struct rpki_uri *,
     struct rpki_uri *, IA5String_t *);
 struct rpki_uri *uri_create_cache(char const *);
 
-#define uri_create(uri, tal, type, notif, guri) \
-       __uri_create(uri, tal, type, notif, guri, strlen(guri))
-#define uri_create_caged(uri, tal, notif, guri, guri_len) \
-       __uri_create(uri, tal, UT_CAGED, notif, guri, guri_len)
+#define uri_create_caged(uri, tal, notif, guri) \
+       uri_create(uri, tal, UT_CAGED, notif, guri)
 #define uri_create_cage(uri, tal, notif) \
-       uri_create_caged(uri, tal, notif, "", 0)
+       uri_create_caged(uri, tal, notif, NULL)
 
 struct rpki_uri *uri_refget(struct rpki_uri *);
 void uri_refput(struct rpki_uri *);
@@ -41,7 +39,6 @@ void uri_refput(struct rpki_uri *);
  */
 char const *uri_get_global(struct rpki_uri *);
 char const *uri_get_local(struct rpki_uri *);
-size_t uri_get_global_len(struct rpki_uri *);
 
 bool uri_equals(struct rpki_uri *, struct rpki_uri *);
 bool uri_has_extension(struct rpki_uri *, char const *);

diff --git a/test/crypto/hash_test.c b/test/crypto/hash_test.c
index 5673aa5891cedf85197d2eaf7dca823bba2ba11e..c870c9afc17f691e5cf107cf9c1d85bb91602073 100644 (file)
--- a/test/crypto/hash_test.c
+++ b/test/crypto/hash_test.c
@@ -45,7 +45,6 @@ START_TEST(test_hash)
        hash_setup();
 
        uri.global = "https://example.com/resources/lorem-ipsum.txt";
-       uri.global_len = strlen(uri.global);
        uri.local = "resources/lorem-ipsum.txt";
        uri.type = UT_TA_HTTP;
        uri.references = 1;

diff --git a/test/rrdp_test.c b/test/rrdp_test.c
index e5ac7acbea01ad8379218e1bd953ce299468e205..cf67f788890aa3d470352a62a244fd9ca02f99ed 100644 (file)
--- a/test/rrdp_test.c
+++ b/test/rrdp_test.c
@@ -413,7 +413,6 @@ START_TEST(test_parse_snapshot_bad_publish)
        ck_assert_int_eq(0, relax_ng_init());
 
        notif_uri.global = "https://example.com/notification.xml";
-       notif_uri.global_len = strlen(notif_uri.global);
        notif_uri.local = "cache/example.com/notification.xml";
        notif_uri.type = UT_NOTIF;
        notif_uri.references = 1;

diff --git a/test/types/uri_test.c b/test/types/uri_test.c
index 8b611e8dfdcbe497226cf81888a15cd99dbe4610..e80f308aeedab5b303d25455ab219c5afe97b5d4 100644 (file)
--- a/test/types/uri_test.c
+++ b/test/types/uri_test.c
@@ -56,7 +56,7 @@ START_TEST(test_constructor)
        uri_refput(uri);
 
        ck_assert_int_eq(0, URI_CREATE_HTTP(uri, "https://a.b.c/"));
-       ck_assert_str_eq("https://a.b.c/", uri_get_global(uri));
+       ck_assert_str_eq("https://a.b.c", uri_get_global(uri));
        ck_assert_str_eq("tmp/test.tal/https/a.b.c", uri_get_local(uri));
        uri_refput(uri);
 
@@ -71,22 +71,22 @@ START_TEST(test_constructor)
        uri_refput(uri);
 
        ck_assert_int_eq(0, URI_CREATE_HTTP(uri, "https://a.b.c/d/.."));
-       ck_assert_str_eq("https://a.b.c/d/..", uri_get_global(uri));
+       ck_assert_str_eq("https://a.b.c", uri_get_global(uri));
        ck_assert_str_eq("tmp/test.tal/https/a.b.c", uri_get_local(uri));
        uri_refput(uri);
 
        ck_assert_int_eq(0, URI_CREATE_HTTP(uri, "https://a.b.c/."));
-       ck_assert_str_eq("https://a.b.c/.", uri_get_global(uri));
+       ck_assert_str_eq("https://a.b.c", uri_get_global(uri));
        ck_assert_str_eq("tmp/test.tal/https/a.b.c", uri_get_local(uri));
        uri_refput(uri);
 
        ck_assert_int_eq(0, URI_CREATE_HTTP(uri, "https://a.b.c/././d/././e/./."));
-       ck_assert_str_eq("https://a.b.c/././d/././e/./.", uri_get_global(uri));
+       ck_assert_str_eq("https://a.b.c/d/e", uri_get_global(uri));
        ck_assert_str_eq("tmp/test.tal/https/a.b.c/d/e", uri_get_local(uri));
        uri_refput(uri);
 
        ck_assert_int_eq(0, URI_CREATE_HTTP(uri, "https://a.b.c/a/b/.././.."));
-       ck_assert_str_eq("https://a.b.c/a/b/.././..", uri_get_global(uri));
+       ck_assert_str_eq("https://a.b.c", uri_get_global(uri));
        ck_assert_str_eq("tmp/test.tal/https/a.b.c", uri_get_local(uri));
        uri_refput(uri);