From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Fri, 15 Sep 2023 21:56:49 +0000 (-0600)
Subject: Refactor path_builder
X-Git-Tag: 1.6.0~67
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2bdbfd882c041e3c3abe2bdb06aaaa929444ae35;p=thirdparty%2FFORT-validator.git

Refactor path_builder

Mainly so a local cache tree traversal recovers from a malformed URI.

1. Atomize append operations

Failure used to leave the path builder in an essentially undefined
state, which meant the path had to be thrown away, precluding further
tree traversal.

A failing `append()` no longer modifies the path builder, allowing the
tree traversal to simply skip the ailing branch.

2. Remove lazy failure

The old path_builder was postponing error reporting to path compilation
(`pb_peek()` and `pb_compile()`).

This was an old optimization (meant to simplify path building code),
and was getting in the way of the atomizing.

Errors are now fail-fast, thrown during path construction
(`pb_append*()`).

3. Move path normalization to `uri`

Path normalization (collapsing `.`, `..` and `/`) was getting in the
way of the atomizing, in addition to only really being useful for the
URI-to-cache conversion code.

3. Restore support for absolute paths

This was just a small TODO spawned a few commits ago.
---

diff --git a/src/cache/local_cache.c b/src/cache/local_cache.c
index e9098a94..bdf08f74 100644
--- a/src/cache/local_cache.c
+++ b/src/cache/local_cache.c
@@ -162,15 +162,18 @@ get_metadata_json_filename(char **filename)
 	int error;
 
 	pb_init(&pb);
-	pb_append(&pb, config_get_local_repository());
-	pb_append(&pb, "metadata.json");
+	error = pb_append(&pb, config_get_local_repository());
+	if (error)
+		goto cancel;
+	error = pb_append(&pb, "metadata.json");
+	if (error)
+		goto cancel;
 
-	error = pb_compile(&pb, filename);
-	if (error) {
-		pr_op_err("Unable to build metadata.json's path: %s",
-		    strerror(error));
-	}
+	*filename = pb.string;
+	return 0;
 
+cancel:
+	pb_cleanup(&pb);
 	return error;
 }
 
@@ -336,34 +339,36 @@ delete_node_file(struct cache_node *node, bool is_file)
 {
 	struct path_builder pb;
 	struct cache_node *cursor;
-	char *path;
 	int error;
 
 	pb_init(&pb);
-	for (cursor = node; cursor != NULL; cursor = cursor->parent)
-		pb_append(&pb, cursor->basename);
-	pb_append(&pb, config_get_local_repository());
-	pb_reverse(&pb);
-	error = pb_compile(&pb, &path);
-	if (error) {
-		pr_val_err("Cannot override '%s'; path is bogus: %s",
-		    node->basename, strerror(error));
-		return error;
+	for (cursor = node; cursor != NULL; cursor = cursor->parent) {
+		error = pb_append(&pb, cursor->basename);
+		if (error)
+			goto cancel;
 	}
+	error = pb_append(&pb, config_get_local_repository());
+	if (error)
+		goto cancel;
+	pb_reverse(&pb);
 
 	if (is_file) {
-		if (remove(path) != 0) {
+		if (remove(pb.string) != 0) {
 			error = errno;
 			pr_val_err("Cannot override file '%s': %s",
-			    path, strerror(error));
+			    pb.string, strerror(error));
 		}
 	} else {
-		error = file_rm_rf(path);
+		error = file_rm_rf(pb.string);
 		pr_val_err("Cannot override directory '%s': %s",
-		    path, strerror(error));
+		    pb.string, strerror(error));
 	}
 
-	free(path);
+	pb_cleanup(&pb);
+	return error;
+
+cancel:
+	pb_cleanup(&pb);
 	return error;
 }
 
@@ -529,19 +534,11 @@ static void cleanup_nodes(struct cache_node *node)
 static void
 pb_rm_r(struct path_builder *pb, char const *filename, bool force)
 {
-	char const *path;
 	int error;
 
-	error = pb_peek(pb, &path);
-	if (error) {
-		pr_op_err("Path builder error code %d; cannot delete directory. (Basename is '%s')",
-		    error, filename);
-		return;
-	}
-
-	error = file_rm_rf(path);
+	error = file_rm_rf(pb->string);
 	if (error && !force)
-		pr_op_err("Cannot delete %s: %s", path, strerror(error));
+		pr_op_err("Cannot delete %s: %s", pb->string, strerror(error));
 }
 
 /* Safe removal, Postorder */
@@ -560,12 +557,18 @@ ctt_init(struct cache_tree_traverser *ctt, struct cache_node *node,
 		return;
 	}
 
+	/* FIXME test these error paths */
 	while (node->children != NULL) {
-		/* FIXME We need to recover from path too long... */
-		pb_append(pb, node->basename);
+		if (pb_append(pb, node->basename) != 0) {
+			node = node->parent;
+			goto end;
+		}
 		node = node->children;
 	}
-	pb_append(pb, "a");
+	if (pb_append(pb, "a") != 0)
+		node = node->parent;
+
+end:
 	ctt->pb = pb;
 	ctt->next = node;
 	ctt->next_sibling = true;
@@ -581,7 +584,7 @@ ctt_next(struct cache_tree_traverser *ctt)
 
 	pb_pop(ctt->pb, true);
 	if (ctt->next_sibling)
-		pb_append(ctt->pb, next->basename);
+		pb_append(ctt->pb, next->basename); /* FIXME */
 
 	if (next->hh.next != NULL) {
 		ctt->next = next->hh.next;
@@ -598,7 +601,6 @@ static void cleanup_files(struct cache_node *node, char const *name)
 {
 	struct cache_tree_traverser ctt;
 	struct path_builder pb;
-	char const *path;
 	struct stat meta;
 	DIR *dir;
 	struct dirent *file;
@@ -606,27 +608,22 @@ static void cleanup_files(struct cache_node *node, char const *name)
 	int error;
 
 	pb_init(&pb);
-	pb_append(&pb, config_get_local_repository());
+	if (pb_append(&pb, config_get_local_repository()) != 0)
+		goto end;
 
 	if (node == NULL) {
 		/* File might exist but node doesn't: Delete file */
-		pb_append(&pb, name);
+		if (pb_append(&pb, name) != 0)
+			goto end;
 		pb_rm_r(&pb, name, true);
-		pb_cancel(&pb);
+		pb_cleanup(&pb);
 		return;
 	}
 
 	ctt_init(&ctt, node, &pb);
 
 	while ((node = ctt_next(&ctt)) != NULL) {
-		error = pb_peek(&pb, &path);
-		if (error) {
-			pr_op_err("Cannot clean up directory (basename is '%s'): %s",
-			    node->basename, strerror(error));
-			break;
-		}
-
-		if (stat(path, &meta) != 0) {
+		if (stat(pb.string, &meta) != 0) {
 			error = errno;
 			if (error == ENOENT) {
 				/* Node exists but file doesn't: Delete node */
@@ -635,7 +632,7 @@ static void cleanup_files(struct cache_node *node, char const *name)
 			}
 
 			pr_op_err("Cannot clean up '%s'; stat() returned errno %d: %s",
-			    path, error, strerror(error));
+			    pb.string, error, strerror(error));
 			break;
 		}
 
@@ -645,15 +642,15 @@ static void cleanup_files(struct cache_node *node, char const *name)
 
 		if (!S_ISDIR(meta.st_mode)) {
 			/* File is not a directory; welp. */
-			remove(path);
+			remove(pb.string);
 			delete_node(node);
 		}
 
-		dir = opendir(path);
+		dir = opendir(pb.string);
 		if (dir == NULL) {
 			error = errno;
 			pr_op_err("Cannot clean up '%s'; S_ISDIR() but !opendir(): %s",
-			    path, strerror(error));
+			    pb.string, strerror(error));
 			continue; /* AAAAAAAAAAAAAAAAAH */
 		}
 
@@ -666,9 +663,10 @@ static void cleanup_files(struct cache_node *node, char const *name)
 				child->flags |= CNF_FOUND;
 			} else {
 				/* File child's node does not exist: Delete. */
-				pb_append(&pb, file->d_name);
-				pb_rm_r(&pb, file->d_name, false);
-				pb_pop(&pb, true);
+				if (pb_append(&pb, file->d_name) == 0) {
+					pb_rm_r(&pb, file->d_name, false);
+					pb_pop(&pb, true);
+				}
 			}
 		}
 
@@ -703,7 +701,8 @@ static void cleanup_files(struct cache_node *node, char const *name)
 		}
 	}
 
-	pb_cancel(&pb);
+end:
+	pb_cleanup(&pb);
 }
 
 static int
diff --git a/src/cache/tmp.c b/src/cache/tmp.c
index 1c737817..23f93ad3 100644
--- a/src/cache/tmp.c
+++ b/src/cache/tmp.c
@@ -20,12 +20,20 @@ int
 cache_tmpfile(char **filename)
 {
 	struct path_builder pb;
+	int error;
 
 	pb_init(&pb);
 
-	pb_append(&pb, config_get_local_repository());
-	pb_append(&pb, "tmp");
-	pb_append_uint(&pb, atomic_fetch_add(&file_counter, 1u));
+	error = pb_append(&pb, config_get_local_repository());
+	if (error)
+		return error;
+	error = pb_append(&pb, "tmp");
+	if (error)
+		return error;
+	error = pb_append_u32(&pb, atomic_fetch_add(&file_counter, 1u));
+	if (error)
+		return error;
 
-	return pb_compile(&pb, filename);
+	*filename = pb.string;
+	return 0;
 }
diff --git a/src/data_structure/path_builder.c b/src/data_structure/path_builder.c
index 12ba0060..78611c24 100644
--- a/src/data_structure/path_builder.c
+++ b/src/data_structure/path_builder.c
@@ -10,31 +10,26 @@
 
 /* These are arbitrary; feel free to change them. */
 #ifndef INITIAL_CAPACITY /* Unit tests want to override this */
-#define INITIAL_CAPACITY 128
+#define INITIAL_CAPACITY 128u
 #endif
-#define MAX_CAPACITY 4096
+#define MAX_CAPACITY 4096u
 
 void
 pb_init(struct path_builder *pb)
 {
 	pb->string = pmalloc(INITIAL_CAPACITY);
+	pb->string[0] = 0;
 	pb->len = 0;
 	pb->capacity = INITIAL_CAPACITY;
-	pb->error = 0;
 }
 
-/*
- * Returns true on success, false on failure.
- */
-static bool
-pb_grow(struct path_builder *pb, size_t total_len)
+static int
+pb_grow(struct path_builder *pb, size_t total_len, char const *addend)
 {
 	if (total_len > MAX_CAPACITY) {
-		free(pb->string);
-		pr_val_err("Path too long: %zu > %u characters.", total_len,
-		    MAX_CAPACITY);
-		pb->error = ENOSPC;
-		return false;
+		pr_val_err("Unable to concatenate '%.32s' (might be truncated) to path '%s': Path too long (%zu > %u)",
+		    addend, pb->string, total_len, MAX_CAPACITY);
+		return ENOSPC;
 	}
 
 	do {
@@ -42,170 +37,96 @@ pb_grow(struct path_builder *pb, size_t total_len)
 	} while (total_len > pb->capacity);
 
 	pb->string = prealloc(pb->string, pb->capacity);
-	return true;
-}
-
-static char const *
-find_slash(char const *str, size_t len)
-{
-	char const *wall;
-
-	for (wall = str + len; str < wall; str++)
-		if (str[0] == '/')
-			return str;
-
-	return str;
+	return 0;
 }
 
-/*
- * Do NOT include the null character in @addlen.
- * Assumes @addend needs no slashes.
- */
-static void
-add(struct path_builder *pb, char const *addend, size_t addlen)
+int
+pb_appendn(struct path_builder *pb, char const *addend, size_t addlen)
 {
 	size_t total_len;
+	bool add_slash;
+	int error;
 
-	total_len = pb->len + addlen;
-	if (total_len > pb->capacity && !pb_grow(pb, total_len))
-		return;
+	if (addlen == 0)
+		return 0;
 
-	memcpy(pb->string + pb->len, addend, addlen);
-	pb->len += addlen;
-}
+	add_slash = (pb->len != 0);
+	if (add_slash)
+		addlen++;
 
-static void
-add_slashed(struct path_builder *pb, char const *addend, size_t addlen)
-{
-	/* Normalize first */
-	switch (addlen) {
-	case 1:
-		if (addend[0] == '.')
-			return;
-		break;
-	case 2:
-		if (addend[0] == '.' && addend[1] == '.') {
-			pb_pop(pb, false);
-			return;
-		}
-		break;
+	total_len = pb->len + addlen + 1;
+	if (total_len > pb->capacity) {
+		error = pb_grow(pb, total_len, addend);
+		if (error)
+			return error;
 	}
 
-	/* Ok, do */
-	if (pb->len > 0)
-		add(pb, "/", 1);
-	add(pb, addend, addlen);
-}
-
-/* Do NOT include the null character in @addlen. */
-static void
-pb_append_limited(struct path_builder *pb, char const *addend, size_t addlen)
-{
-	char const *wall;
-	char const *next_slash;
+	if (add_slash) {
+		pb->string[pb->len] = '/';
+		memcpy(pb->string + pb->len + 1, addend, addlen);
+	} else {
+		memcpy(pb->string + pb->len, addend, addlen);
+	}
 
-	if (pb->error)
-		return;
+	pb->len += addlen;
+	pb->string[pb->len] = 0;
 
-	do {
-		for (wall = addend + addlen; addend < wall; addend++, addlen--)
-			if (addend[0] != '/')
-				break;
-		next_slash = find_slash(addend, addlen);
-		if (addend == next_slash)
-			return;
-		add_slashed(pb, addend, next_slash - addend);
-		addlen -= next_slash - addend;
-		addend = next_slash;
-	} while (addlen > 0);
+	return 0;
 }
 
-void
+int
 pb_append(struct path_builder *pb, char const *addend)
 {
-	pb_append_limited(pb, addend, strlen(addend));
+	return pb_appendn(pb, addend, strlen(addend));
 }
 
-void
-pb_append_guri(struct path_builder *pb, struct rpki_uri *uri)
-{
-	char const *guri;
-	char const *colon;
-	size_t schema_len;
-
-	if (pb->error)
-		return;
-
-	guri = uri_get_global(uri);
-
-	colon = strstr(guri, ":");
-	schema_len = colon - guri;
-	pb_append_limited(pb, guri, schema_len);
-
-	pb_append_limited(pb, colon + 3,
-	    uri_get_global_len(uri) - schema_len - 3);
-}
-
-void
-pb_append_uint(struct path_builder *pb, unsigned int num)
+int
+pb_append_u32(struct path_builder *pb, uint32_t num)
 {
-	size_t room;
+#define MAX_STRLEN 9 /* 8 hexadecimal digits plus null chara */
+	char buffer[MAX_STRLEN];
 	int num_len;
 
-	if (pb->error)
-		return;
-
-	if (pb->len != 0 && pb->string[pb->len - 1] != '/')
-		add(pb, "/", 1);
-
-	room = pb->capacity - pb->len;
-	num_len = snprintf(pb->string + pb->len, room, "%X", num);
-	if (num_len < 0)
-		goto bad_print;
-	if (num_len >= room) {
-		if (!pb_grow(pb, pb->len + num_len + 1))
-			return;
-
-		room = pb->capacity - pb->len;
-		num_len = snprintf(pb->string + pb->len, room, "%X", num);
-		if (num_len < 0)
-			goto bad_print;
-		if (num_len >= room)
-			pr_crit("pb: %d %zu", num_len, room);
+	num_len = snprintf(buffer, MAX_STRLEN, "%X", num);
+	if (num_len < 0) {
+		pr_val_err("Cannot stringify number '%u': Unknown cause. Error code might be %d.",
+		    num, num_len);
+		return EIO; /* num_len is not necessarily an error code */
 	}
+	if (num_len >= MAX_STRLEN)
+		pr_crit("pb: Number %u requires %d digits", num, num_len);
 
-	pb->len += num_len;
-	return;
-
-bad_print:
-	free(pb->string);
-	pb->error = EIO; /* num_len is not necessarily an error code */
+	return pb_appendn(pb, buffer, num_len);
 }
 
 /* Removes the last component added. */
-void
+int
 pb_pop(struct path_builder *pb, bool fatal)
 {
 	size_t i;
 
-	if (pb->error)
-		return;
-	if (pb->len == 0) {
+	if (pb->len == 0 || (pb->len == 1 && pb->string[0] == '/')) {
 		if (fatal)
 			pr_crit("Programming error: Attempting to pop empty path builder");
-		free(pb->string);
-		pb->error = -pr_val_err("Path cannot '..' over the root.");
-		return;
+		return -pr_val_err("Path cannot '..' over the root.");
 	}
 
 	for (i = pb->len - 1; i >= 1; i--) {
 		if (pb->string[i] == '/') {
+			pb->string[i] = 0;
 			pb->len = i;
-			return;
+			return 0;
 		}
 	}
 
-	pb->len = (pb->string[0] == '/') && (pb->len > 1);
+	if (pb->string[0] == '/') {
+		pb->string[1] = 0;
+		pb->len = 1;
+	} else {
+		pb->string[0] = 0;
+		pb->len = 0;
+	}
+	return 0;
 }
 
 static void
@@ -228,9 +149,6 @@ pb_reverse(struct path_builder *pb)
 	size_t min;
 	size_t max;
 
-	if (pb->error)
-		return;
-
 	reverse_string(pb->string, pb->len);
 
 	min = 0;
@@ -244,37 +162,8 @@ pb_reverse(struct path_builder *pb)
 	reverse_string(&pb->string[min], pb->len - min);
 }
 
-/*
- * Returns @pb's current accumulated path. Do not free it.
- * Result is a temporary pointer; it becomes junk if you call any other pb
- * functions on @pb afterwards.
- */
-int
-pb_peek(struct path_builder *pb, char const **result)
-{
-	add(pb, "\0", 1);
-	if (pb->error)
-		return pb->error;
-
-	*result = pb->string;
-	pb->len--;
-	return 0;
-}
-
-/* Should not be called more than once. */
-int
-pb_compile(struct path_builder *pb, char **result)
-{
-	add(pb, "\0", 1);
-	if (pb->error)
-		return pb->error;
-
-	*result = pb->string;
-	return 0;
-}
-
 void
-pb_cancel(struct path_builder *pb)
+pb_cleanup(struct path_builder *pb)
 {
 	free(pb->string);
 }
diff --git a/src/data_structure/path_builder.h b/src/data_structure/path_builder.h
index adc5fae4..a8c91317 100644
--- a/src/data_structure/path_builder.h
+++ b/src/data_structure/path_builder.h
@@ -1,43 +1,31 @@
 #ifndef SRC_DATA_STRUCTURE_PATH_BUILDER_H_
 #define SRC_DATA_STRUCTURE_PATH_BUILDER_H_
 
-/* FIXME add support for absolute paths */
-
 #include <stdbool.h>
 #include <stddef.h>
 #include "types/uri.h"
 
 struct path_builder {
 	char *string;
-	size_t len;
+	size_t len; /* Includes the null chara */
 	size_t capacity;
-	int error;
 };
 
 void pb_init(struct path_builder *);
 
 /*
- * Note, the append()s merge slashes:
- *
- * 	a + b = a/b
- * 	a/ + b = a/b
- * 	a + /b = a/b
- * 	a/ + /b = a/b
- * 	a// + ///b = a/b
- * 	a///b + c//d = a/b/c/d
+ * The appends are atomic.
+ * They are also naive; they don't collapse `.`, `..` nor slashes.
  */
 
-void pb_append(struct path_builder *, char const *);
-void pb_append_guri(struct path_builder *, struct rpki_uri *);
-void pb_append_uint(struct path_builder *, unsigned int);
+int pb_appendn(struct path_builder *, char const *, size_t);
+int pb_append(struct path_builder *, char const *);
+int pb_append_u32(struct path_builder *, uint32_t);
 
-void pb_pop(struct path_builder *, bool);
+int pb_pop(struct path_builder *, bool);
 
 void pb_reverse(struct path_builder *);
 
-int pb_peek(struct path_builder *, char const **);
-int pb_compile(struct path_builder *, char **);
-
-void pb_cancel(struct path_builder *);
+void pb_cleanup(struct path_builder *);
 
 #endif /* SRC_DATA_STRUCTURE_PATH_BUILDER_H_ */
diff --git a/src/types/uri.c b/src/types/uri.c
index 5fad50bf..4b40a6e1 100644
--- a/src/types/uri.c
+++ b/src/types/uri.c
@@ -209,38 +209,120 @@ ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5)
 	return 0;
 }
 
-/*
- * Maps "rsync://a.b.c/d/e.cer" into "<local-repository>/rsync/a.b.c/d/e.cer".
- */
+struct path_parser {
+	char const *token;
+	char const *slash;
+	size_t len;
+};
+
+/* Return true if there's a new token, false if we're done. */
+static bool
+path_next(struct path_parser *parser)
+{
+	if (parser->slash == NULL)
+		return false;
+
+	parser->token = parser->slash + 1;
+	parser->slash = strchr(parser->token, '/');
+	parser->len = (parser->slash != NULL)
+	    ? (parser->slash - parser->token)
+	    : strlen(parser->token);
+
+	return parser->token[0] != 0;
+}
+
+static bool
+path_is_dot(struct path_parser *parser)
+{
+	return parser->len == 1 && parser->token[0] == '.';
+}
+
+static bool
+path_is_dotdots(struct path_parser *parser)
+{
+	return parser->len == 2
+	    && parser->token[0] == '.'
+	    && parser->token[1] == '.' ;
+}
+
 static int
-map_simple(struct rpki_uri *uri, char const *gprefix, char const *lprefix,
-    int err)
+append_guri(struct path_builder *pb, char const *guri, char const *gprefix,
+    int err, bool skip_schema)
 {
-	struct path_builder pb;
+	struct path_parser parser;
+	size_t dot_dot_limit;
 	int error;
 
-	if (!str_starts_with(uri->global, gprefix)) {
-		pr_val_err("URI '%s' does not begin with '%s'.",
-		    uri->global, lprefix);
+	/* Schema */
+	if (!str_starts_with(guri, gprefix)) {
+		pr_val_err("URI '%s' does not begin with '%s'.", guri, gprefix);
 		return err;
 	}
 
-	pb_init(&pb);
-	pb_append_guri(&pb, uri);
-	error = pb_compile(&pb, &uri->local);
+	if (!skip_schema) {
+		error = pb_appendn(pb, guri, 5);
+		if (error)
+			return error;
+	}
+
+	/* Domain */
+	parser.slash = guri + 7;
+	if (!path_next(&parser))
+		return pr_val_err("URI '%s' seems to lack a domain.", guri);
+	if (path_is_dot(&parser)) {
+		/* Dumping files to the cache root is unsafe. */
+		return pr_val_err("URI '%s' employs the root domain. This is not really cacheable, so I'm going to distrust it.",
+		    guri);
+	}
+	if (path_is_dotdots(&parser)) {
+		return pr_val_err("URI '%s' seems to be dot-dotting past its own schema.",
+		    guri);
+	}
+	error = pb_appendn(pb, parser.token, parser.len);
 	if (error)
 		return error;
 
-	if (!str_starts_with(uri->local, lprefix)) {
-		pr_val_err("URI '%s' seems to be dot-dotting to its scheme.",
-		    uri->global);
-		free(uri->local);
-		return -EINVAL;
+	/* Other components */
+	dot_dot_limit = pb->len;
+	while (path_next(&parser)) {
+		if (path_is_dotdots(&parser)) {
+			error = pb_pop(pb, false);
+			if (error)
+				return error;
+			if (pb->len < dot_dot_limit) {
+				return pr_val_err("URI '%s' seems to be dot-dotting past its own domain.",
+				    guri);
+			}
+		} else if (!path_is_dot(&parser)) {
+			error = pb_appendn(pb, parser.token, parser.len);
+			if (error)
+				return error;
+		}
 	}
 
 	return 0;
 }
 
+/*
+ * 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 *gprefix, int err)
+{
+	struct path_builder pb;
+	int error;
+
+	pb_init(&pb);
+	error = append_guri(&pb, uri->global, gprefix, err, false);
+	if (error) {
+		pb_cleanup(&pb);
+		return error;
+	}
+
+	uri->local = pb.string;
+	return 0;
+}
+
 /*
  * Maps "rsync://a.b.c/d/e.cer" into
  * "<local-repository>/rrdp/<notification-path>/a.b.c/d/e.cer".
@@ -250,6 +332,7 @@ map_caged(struct rpki_uri *uri)
 {
 	struct path_builder pb;
 	struct rpki_uri *notification;
+	int error;
 
 	notification = validation_get_notification_uri(state_retrieve());
 	if (notification == NULL)
@@ -257,11 +340,18 @@ map_caged(struct rpki_uri *uri)
 
 	pb_init(&pb);
 
-	pb_append(&pb, "rrdp");
-	pb_append_guri(&pb, notification);
-	pb_append_guri(&pb, uri);
+	error = pb_append(&pb, "rrdp");
+	if (error)
+		return error;
+	error = append_guri(&pb, notification->global, "https://", ENOTHTTPS, true);
+	if (error)
+		return error;
+	error = append_guri(&pb, uri->global, "rsync://", ENOTRSYNC, true);
+	if (error)
+		return error;
 
-	return pb_compile(&pb, &uri->local);
+	uri->local = pb.string;
+	return 0;
 }
 
 static int
@@ -269,9 +359,9 @@ autocomplete_local(struct rpki_uri *uri)
 {
 	switch (uri->type) {
 	case UT_RSYNC:
-		return map_simple(uri, "rsync://", "rsync/", ENOTRSYNC);
+		return map_simple(uri, "rsync://", ENOTRSYNC);
 	case UT_HTTPS:
-		return map_simple(uri, "https://", "https/", ENOTHTTPS);
+		return map_simple(uri, "https://", ENOTHTTPS);
 	case UT_CAGED:
 		return map_caged(uri);
 	}
diff --git a/test/cache/local_cache_test.c b/test/cache/local_cache_test.c
index 01a82fff..9a6c67d0 100644
--- a/test/cache/local_cache_test.c
+++ b/test/cache/local_cache_test.c
@@ -191,7 +191,7 @@ validate_node(struct cache_node *expected, struct cache_node *expected_parent,
 	}
 	ck_assert_ptr_eq(expected_parent, actual->parent);
 
-	pb_append(pb, expected->basename);
+	ck_assert_int_eq(0, pb_append(pb, expected->basename));
 
 	HASH_ITER(hh, expected->children, expected_child, tmp) {
 		HASH_FIND_STR(actual->children, expected_child->basename,
@@ -235,7 +235,6 @@ search_dir(DIR *parent, char const *path, char const *name)
 static void
 validate_file(struct cache_node *expected, struct path_builder *pb)
 {
-	char const *path;
 	struct stat meta;
 	DIR *dir;
 	struct dirent *file;
@@ -245,8 +244,7 @@ validate_file(struct cache_node *expected, struct path_builder *pb)
 	if (expected == NULL)
 		return;
 
-	pb_append(pb, expected->basename);
-	ck_assert_int_eq(0, pb_peek(pb, &path));
+	ck_assert_int_eq(0, pb_append(pb, expected->basename));
 
 	if (is_rsync(expected)) {
 		/* Currently, the unit tests do not fake rsync files */
@@ -266,13 +264,13 @@ validate_file(struct cache_node *expected, struct path_builder *pb)
 	}
 
 must_be_file:
-	ck_assert_int_eq(0, stat(path, &meta));
+	ck_assert_int_eq(0, stat(pb->string, &meta));
 	ck_assert_int_eq(1, S_ISREG(meta.st_mode));
 	goto end;
 
 must_be_dir:
 	errno = 0;
-	dir = opendir(path);
+	dir = opendir(pb->string);
 	error = errno;
 	ck_assert_int_eq(0, error);
 	ck_assert_ptr_nonnull(dir);
@@ -284,7 +282,7 @@ must_be_dir:
 		HASH_FIND_STR(expected->children, file->d_name, child);
 		if (child == NULL) {
 			ck_abort_msg("file %s/%s is not supposed to exist.",
-			    path, file->d_name);
+			    pb->string, file->d_name);
 		}
 
 		validate_file(child, pb);
@@ -293,7 +291,7 @@ must_be_dir:
 	ck_assert_int_eq(0, error);
 
 	HASH_ITER(hh, expected->children, child, tmp)
-		search_dir(dir, path, child->basename);
+		search_dir(dir, pb->string, child->basename);
 
 	closedir(dir);
 end:
@@ -321,12 +319,12 @@ validate_trees(struct cache_node *actual, struct cache_node *nodes,
 	}
 
 	pb_init(&pb);
-	pb_append(&pb, "tmp");
+	ck_assert_int_eq(0, pb_append(&pb, "tmp"));
 
 	validate_node(nodes, NULL, actual, &pb);
 	validate_file(files, &pb);
 
-	pb_cancel(&pb);
+	pb_cleanup(&pb);
 
 	delete_node(nodes);
 	if (nodes != files)
diff --git a/test/data_structure/path_builder_test.c b/test/data_structure/path_builder_test.c
index 4021a8a0..a8017d6b 100644
--- a/test/data_structure/path_builder_test.c
+++ b/test/data_structure/path_builder_test.c
@@ -7,101 +7,85 @@
 #include "mock.c"
 #include "data_structure/path_builder.c"
 
-/* Mocks */
-
-__MOCK_ABORT(uri_get_global, char const *, NULL, struct rpki_uri *uri)
-__MOCK_ABORT(uri_get_global_len, size_t, 0, struct rpki_uri *uri)
-
-/* Tests */
-
-#define CHECK_PB(_len, _capacity, _error)				\
-	ck_assert_uint_eq(_len, pb.len);				\
-	ck_assert_uint_eq(_capacity, pb.capacity);			\
-	ck_assert_int_eq(_error, pb.error)
-
-#define CHECK_RESULTS(expected)						\
-	ck_assert_uint_eq(0, pb_peek(&pb, &peek_result));		\
-	ck_assert_str_eq(expected, peek_result);			\
-	ck_assert_uint_eq(0, pb_compile(&pb, &compile_result));	\
-	ck_assert_str_eq(expected, compile_result);			\
-	free(compile_result);
-
-#define CHECK_ERROR							\
-	ck_assert_uint_eq(EINVAL, pb_peek(&pb, &peek_result));	\
-	ck_assert_uint_eq(EINVAL, pb_compile(&pb, &compile_result));
+#define CHECK_PB(_string, _capacity)					\
+	ck_assert_str_eq(_string, pb.string);				\
+	ck_assert_uint_eq(strlen(_string), pb.len);			\
+	ck_assert_uint_eq(_capacity, pb.capacity);
 
 START_TEST(test_append)
 {
 	struct path_builder pb;
-	char const *peek_result;
-	char *compile_result;
 
 	pb_init(&pb);
-	pb_append(&pb, "");
-	CHECK_PB(0, 8, 0);
-	CHECK_RESULTS("");
+	ck_assert_int_eq(0, pb_append(&pb, ""));
+	CHECK_PB("", 8);
+	pb_cleanup(&pb);
+
+	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	CHECK_PB("a", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	CHECK_RESULTS("a");
+	ck_assert_int_eq(0, pb_append(&pb, "/a"));
+	CHECK_PB("/a", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	CHECK_PB("a", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "b"));
+	CHECK_PB("a/b", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a/b");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
+	ck_assert_int_eq(0, pb_append(&pb, "a/b"));
+	CHECK_PB("a/b", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a/");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b/");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
+	ck_assert_int_eq(0, pb_append(&pb, "a/"));
+	CHECK_PB("a/", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "b/"));
+	CHECK_PB("a//b/", 8);
+	pb_cleanup(&pb);
 
-	/* notes from .h */
 	pb_init(&pb);
-	pb_append(&pb, "a/");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
+	ck_assert_int_eq(0, pb_append(&pb, "a/"));
+	CHECK_PB("a/", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "b"));
+	CHECK_PB("a//b", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "/b");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	CHECK_PB("a", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "/b"));
+	CHECK_PB("a//b", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a/");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "/b");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
+	ck_assert_int_eq(0, pb_append(&pb, "a/"));
+	CHECK_PB("a/", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "/b"));
+	CHECK_PB("a///b", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "//a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "///");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b////");
-	CHECK_PB(3, 8, 0);
-	pb_append(&pb, "/////c//////");
-	CHECK_PB(5, 8, 0);
-	CHECK_RESULTS("a/b/c");
+	ck_assert_int_eq(0, pb_append(&pb, "//a"));
+	CHECK_PB("//a", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "///"));
+	CHECK_PB("//a////", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "b////"));
+	CHECK_PB("//a/////b////", 16);
+	ck_assert_int_eq(0, pb_append(&pb, "/////c//////"));
+	CHECK_PB("//a/////b//////////c//////", 32);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "//a///b//c//");
-	CHECK_PB(5, 8, 0);
-	CHECK_RESULTS("a/b/c");
+	ck_assert_int_eq(0, pb_append(&pb, "//a///b//c//"));
+	CHECK_PB("//a///b//c//", 16);
+	pb_cleanup(&pb);
 }
 END_TEST
 
@@ -109,335 +93,209 @@ END_TEST
 START_TEST(test_uint)
 {
 	struct path_builder pb;
-	char const *peek_result;
-	char *compile_result;
 
 	pb_init(&pb);
-	pb_append_uint(&pb, 291);
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("123"); /* hex */
+	pb_append_u32(&pb, 0x123);
+	CHECK_PB("123", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append_uint(&pb, 19088743);
-	CHECK_PB(7, 8, 0);
-	CHECK_RESULTS("1234567");
+	pb_append_u32(&pb, 0x1234567);
+	CHECK_PB("1234567", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append_uint(&pb, 305419896);
-	CHECK_PB(8, 16, 0);
-	CHECK_RESULTS("12345678");
+	pb_append_u32(&pb, 0x12345678);
+	CHECK_PB("12345678", 16);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append_uint(&pb, 74565);
-	CHECK_PB(5, 8, 0);
-	pb_append_uint(&pb, 7);
-	CHECK_PB(7, 8, 0);
-	CHECK_RESULTS("12345/7");
+	pb_append_u32(&pb, 0x12345);
+	CHECK_PB("12345", 8);
+	pb_append_u32(&pb, 0x7);
+	CHECK_PB("12345/7", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append_uint(&pb, 74565);
-	CHECK_PB(5, 8, 0);
-	pb_append_uint(&pb, 120);
-	CHECK_PB(8, 16, 0);
-	CHECK_RESULTS("12345/78");
+	pb_append_u32(&pb, 0x12345);
+	CHECK_PB("12345", 8);
+	pb_append_u32(&pb, 0x78);
+	CHECK_PB("12345/78", 16);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append_uint(&pb, 74565);
-	CHECK_PB(5, 8, 0);
-	pb_append_uint(&pb, 1929);
-	CHECK_PB(9, 16, 0);
-	CHECK_RESULTS("12345/789");
+	pb_append_u32(&pb, 0x12345);
+	CHECK_PB("12345", 8);
+	pb_append_u32(&pb, 0x789);
+	CHECK_PB("12345/789", 16);
+	pb_cleanup(&pb);
 }
 END_TEST
 
 START_TEST(test_pop)
 {
 	struct path_builder pb;
-	char const *peek_result;
-	char *compile_result;
-
-	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	pb_pop(&pb, false);
-	CHECK_PB(1, 8, 0);
-	CHECK_RESULTS("a");
-
-	pb_init(&pb);
-	pb_append(&pb, "abc");
-	CHECK_PB(3, 8, 0);
-	pb_append(&pb, "def");
-	CHECK_PB(7, 8, 0);
-	pb_pop(&pb, false);
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("abc");
-
-	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_pop(&pb, false);
-	CHECK_PB(0, 8, 0);
-	CHECK_RESULTS("");
-
-	pb_init(&pb);
-	pb_append(&pb, "/a");
-	CHECK_PB(1, 8, 0);
-	pb_pop(&pb, false);
-	CHECK_PB(0, 8, 0);
-	CHECK_RESULTS("");
-
-	pb_init(&pb);
-	pb_pop(&pb, false);
-	CHECK_PB(0, 8, EINVAL);
-	CHECK_ERROR;
-
-	pb_init(&pb);
-	pb_append(&pb, "a");
-	pb_pop(&pb, false);
-	CHECK_PB(0, 8, 0);
-	pb_pop(&pb, false);
-	CHECK_PB(0, 8, EINVAL);
-	CHECK_ERROR;
-
-//	pb_init(&pb);
-//	pb_append(&pb, "/");
-//	CHECK_PB(1, 8, 0);
-//	pb_pop(&pb);
-//	CHECK_PB(0, 8, 0);
-//	CHECK_RESULTS("");
-//
-//	pb_init(&pb);
-//	pb_append(&pb, "///");
-//	CHECK_PB(3, 8, 0);
-//	pb_pop(&pb);
-//	CHECK_PB(2, 8, 0);
-//	pb_pop(&pb);
-//	CHECK_PB(1, 8, 0);
-//	pb_pop(&pb);
-//	CHECK_PB(0, 8, 0);
-//	CHECK_RESULTS("");
-}
-END_TEST
-
-START_TEST(test_peek)
-{
-	struct path_builder pb;
-	char const *peek_result;
-
-	/*
-	 * Most of pb_peek() has already been tested above,
-	 * just check it leaves the pb in a stable state.
-	 */
 
 	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	CHECK_PB("a", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "b"));
+	CHECK_PB("a/b", 8);
+	ck_assert_int_eq(0, pb_pop(&pb, false));
+	CHECK_PB("a", 8);
+	pb_cleanup(&pb);
 
-	pb_peek(&pb, &peek_result);
-	ck_assert_str_eq("", peek_result);
+	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "abc"));
+	CHECK_PB("abc", 8);
+	ck_assert_int_eq(0, pb_append(&pb, "def"));
+	CHECK_PB("abc/def", 8);
+	ck_assert_int_eq(0, pb_pop(&pb, false));
+	CHECK_PB("abc", 8);
+	pb_cleanup(&pb);
 
-	pb_append(&pb, "a");
-	pb_peek(&pb, &peek_result);
-	ck_assert_str_eq("a", peek_result);
+	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	CHECK_PB("a", 8);
+	ck_assert_int_eq(0, pb_pop(&pb, false));
+	CHECK_PB("", 8);
+	pb_cleanup(&pb);
 
-	pb_append(&pb, "b");
-	pb_peek(&pb, &peek_result);
-	ck_assert_str_eq("a/b", peek_result);
+	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "/a"));
+	CHECK_PB("/a", 8);
+	ck_assert_int_eq(0, pb_pop(&pb, false));
+	CHECK_PB("/", 8);
+	pb_cleanup(&pb);
 
-	pb_pop(&pb, true);
-	pb_peek(&pb, &peek_result);
-	ck_assert_str_eq("a", peek_result);
+	pb_init(&pb);
+	ck_assert_int_eq(EINVAL, pb_pop(&pb, false));
+	CHECK_PB("", 8);
+	pb_cleanup(&pb);
 
-	pb_pop(&pb, true);
-	pb_peek(&pb, &peek_result);
-	ck_assert_str_eq("", peek_result);
+	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	ck_assert_int_eq(0, pb_pop(&pb, false));
+	CHECK_PB("", 8);
+	ck_assert_int_eq(EINVAL, pb_pop(&pb, false));
+	CHECK_PB("", 8);
+	pb_cleanup(&pb);
 
-	free(pb.string);
+	pb_init(&pb);
+	ck_assert_int_eq(0, pb_append(&pb, "/"));
+	CHECK_PB("/", 8);
+	ck_assert_int_eq(EINVAL, pb_pop(&pb, false));
+	CHECK_PB("/", 8);
+	pb_cleanup(&pb);
 }
 END_TEST
 
 START_TEST(test_reverse)
 {
 	struct path_builder pb;
-	char const *peek_result;
-	char *compile_result;
 
 	/* 0 components */
 	pb_init(&pb);
 	pb_reverse(&pb);
-	CHECK_PB(0, 8, 0);
-	CHECK_RESULTS("");
+	CHECK_PB("", 8);
+	pb_cleanup(&pb);
 
 	/* 1 component */
 	pb_init(&pb);
-	pb_append(&pb, "a");
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
 	pb_reverse(&pb);
-	CHECK_PB(1, 8, 0);
-	CHECK_RESULTS("a");
+	CHECK_PB("a", 8);
+	pb_cleanup(&pb);
 
 	/* 2 components */
 	pb_init(&pb);
-	pb_append(&pb, "a");
-	pb_append(&pb, "b");
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	ck_assert_int_eq(0, pb_append(&pb, "b"));
 	pb_reverse(&pb);
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("b/a");
+	CHECK_PB("b/a", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "abc");
-	pb_append(&pb, "def");
+	ck_assert_int_eq(0, pb_append(&pb, "abc"));
+	ck_assert_int_eq(0, pb_append(&pb, "def"));
 	pb_reverse(&pb);
-	CHECK_PB(7, 8, 0);
-	CHECK_RESULTS("def/abc");
+	CHECK_PB("def/abc", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "abcd");
-	pb_append(&pb, "efgh");
+	ck_assert_int_eq(0, pb_append(&pb, "abcd"));
+	ck_assert_int_eq(0, pb_append(&pb, "efgh"));
 	pb_reverse(&pb);
-	CHECK_PB(9, 16, 0);
-	CHECK_RESULTS("efgh/abcd");
+	CHECK_PB("efgh/abcd", 16);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "abc");
-	pb_append(&pb, "efgh");
+	ck_assert_int_eq(0, pb_append(&pb, "abc"));
+	ck_assert_int_eq(0, pb_append(&pb, "efgh"));
 	pb_reverse(&pb);
-	CHECK_PB(8, 8, 0);
-	CHECK_RESULTS("efgh/abc");
+	CHECK_PB("efgh/abc", 16);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "abcd");
-	pb_append(&pb, "fgh");
+	ck_assert_int_eq(0, pb_append(&pb, "abcd"));
+	ck_assert_int_eq(0, pb_append(&pb, "fgh"));
 	pb_reverse(&pb);
-	CHECK_PB(8, 8, 0);
-	CHECK_RESULTS("fgh/abcd");
+	CHECK_PB("fgh/abcd", 16);
+	pb_cleanup(&pb);
 
 	/* 3 components */
 	pb_init(&pb);
-	pb_append(&pb, "abc");
-	pb_append(&pb, "def");
-	pb_append(&pb, "ghi");
+	ck_assert_int_eq(0, pb_append(&pb, "abc"));
+	ck_assert_int_eq(0, pb_append(&pb, "def"));
+	ck_assert_int_eq(0, pb_append(&pb, "ghi"));
 	pb_reverse(&pb);
-	CHECK_PB(11, 16, 0);
-	CHECK_RESULTS("ghi/def/abc");
+	CHECK_PB("ghi/def/abc", 16);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "ab");
-	pb_append(&pb, "cde");
-	pb_append(&pb, "fghi");
+	ck_assert_int_eq(0, pb_append(&pb, "ab"));
+	ck_assert_int_eq(0, pb_append(&pb, "cde"));
+	ck_assert_int_eq(0, pb_append(&pb, "fghi"));
 	pb_reverse(&pb);
-	CHECK_PB(11, 16, 0);
-	CHECK_RESULTS("fghi/cde/ab");
+	CHECK_PB("fghi/cde/ab", 16);
+	pb_cleanup(&pb);
 
 	/* 4 components */
 	pb_init(&pb);
-	pb_append(&pb, "a");
-	pb_append(&pb, "b");
-	pb_append(&pb, "c");
-	pb_append(&pb, "d");
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	ck_assert_int_eq(0, pb_append(&pb, "b"));
+	ck_assert_int_eq(0, pb_append(&pb, "c"));
+	ck_assert_int_eq(0, pb_append(&pb, "d"));
 	pb_reverse(&pb);
-	CHECK_PB(7, 8, 0);
-	CHECK_RESULTS("d/c/b/a");
+	CHECK_PB("d/c/b/a", 8);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "ab");
-	pb_append(&pb, "cd");
-	pb_append(&pb, "ef");
-	pb_append(&pb, "gh");
+	ck_assert_int_eq(0, pb_append(&pb, "ab"));
+	ck_assert_int_eq(0, pb_append(&pb, "cd"));
+	ck_assert_int_eq(0, pb_append(&pb, "ef"));
+	ck_assert_int_eq(0, pb_append(&pb, "gh"));
 	pb_reverse(&pb);
-	CHECK_PB(11, 16, 0);
-	CHECK_RESULTS("gh/ef/cd/ab");
+	CHECK_PB("gh/ef/cd/ab", 16);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "a");
-	pb_append(&pb, "bcd");
-	pb_append(&pb, "efgh");
-	pb_append(&pb, "ijklm");
+	ck_assert_int_eq(0, pb_append(&pb, "a"));
+	ck_assert_int_eq(0, pb_append(&pb, "bcd"));
+	ck_assert_int_eq(0, pb_append(&pb, "efgh"));
+	ck_assert_int_eq(0, pb_append(&pb, "ijklm"));
 	pb_reverse(&pb);
-	CHECK_PB(16, 16, 0);
-	CHECK_RESULTS("ijklm/efgh/bcd/a");
+	CHECK_PB("ijklm/efgh/bcd/a", 32);
+	pb_cleanup(&pb);
 
 	pb_init(&pb);
-	pb_append(&pb, "abcdefghijklmnopq");
-	pb_append(&pb, "r");
-	pb_append(&pb, "stu");
-	pb_append(&pb, "vx");
+	ck_assert_int_eq(0, pb_append(&pb, "abcdefghijklmnopq"));
+	ck_assert_int_eq(0, pb_append(&pb, "r"));
+	ck_assert_int_eq(0, pb_append(&pb, "stu"));
+	ck_assert_int_eq(0, pb_append(&pb, "vx"));
 	pb_reverse(&pb);
-	CHECK_PB(26, 32, 0);
-	CHECK_RESULTS("vx/stu/r/abcdefghijklmnopq");
-}
-END_TEST
-
-START_TEST(test_normalization)
-{
-	struct path_builder pb;
-	char const *peek_result;
-	char *compile_result;
-
-	pb_init(&pb);
-	pb_append(&pb, ".");
-	CHECK_PB(0, 8, 0);
-	CHECK_RESULTS("");
-
-	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
-
-	pb_init(&pb);
-	pb_append(&pb, ".");
-	CHECK_PB(0, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(0, 8, 0);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(3, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(3, 8, 0);
-	CHECK_RESULTS("a/b");
-
-	pb_init(&pb);
-	pb_append(&pb, "..");
-	CHECK_PB(0, 8, EINVAL);
-	CHECK_ERROR;
-
-	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	pb_append(&pb, "..");
-	CHECK_PB(1, 8, 0);
-	CHECK_RESULTS("a");
-
-	pb_init(&pb);
-	pb_append(&pb, "a");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "b");
-	CHECK_PB(3, 8, 0);
-	pb_append(&pb, "..");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, ".");
-	CHECK_PB(1, 8, 0);
-	pb_append(&pb, "..");
-	CHECK_PB(0, 8, 0);
-	CHECK_RESULTS("");
-
-	/* dot dot injection */
-	pb_init(&pb);
-	pb_append(&pb, "a/../b");
-	CHECK_PB(1, 8, 0);
-	CHECK_RESULTS("b");
+	CHECK_PB("vx/stu/r/abcdefghijklmnopq", 32);
+	pb_cleanup(&pb);
 }
 END_TEST
 
@@ -451,9 +309,7 @@ pdu_suite(void)
 	tcase_add_test(core, test_append);
 	tcase_add_test(core, test_uint);
 	tcase_add_test(core, test_pop);
-	tcase_add_test(core, test_peek);
 	tcase_add_test(core, test_reverse);
-	tcase_add_test(core, test_normalization);
 
 	suite = suite_create("path_builder");
 	suite_add_tcase(suite, core);
diff --git a/test/mock.h b/test/mock.h
index d699cb64..cdba5e92 100644
--- a/test/mock.h
+++ b/test/mock.h
@@ -22,9 +22,6 @@
     type name(__VA_ARGS__) { __MOCK_ABORT_MSG; return result; }
 #define MOCK_ABORT_INT(name, ...) \
     __MOCK_ABORT(name, int, 0, __VA_ARGS__)
-/* FIXME delete? */
-#define MOCK_ABORT_BOOL(name, ...) \
-    __MOCK_ABORT(name, bool, false, __VA_ARGS__)
 #define MOCK_ABORT_ENUM(name, type, ...) \
     __MOCK_ABORT(name, enum type, 0, __VA_ARGS__)
 #define MOCK_ABORT_PTR(name, type, ...) \
diff --git a/test/types/uri_test.c b/test/types/uri_test.c
index 87d538b3..2c2e3992 100644
--- a/test/types/uri_test.c
+++ b/test/types/uri_test.c
@@ -22,35 +22,63 @@ START_TEST(test_constructor)
 {
 	struct rpki_uri *uri;
 
-	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
-	uri_refput(uri);
+	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, ""));
+	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "h"));
+	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "http"));
+	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "https"));
+	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "https:"));
+	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "https:/"));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://"));
 
 	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
 	uri_refput(uri);
 
-	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://"));
+	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
+	uri_refput(uri);
+
+	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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/d", uri_get_local(uri));
+	uri_refput(uri);
+
+	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
+	uri_refput(uri);
 
 	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
 	uri_refput(uri);
 
-	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://a.b.c/.."));
-	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://a.b.c/d/../.."));
-
 	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
 	uri_refput(uri);
 
-	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "rsync://a.b.c/d"));
-	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, ""));
-	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://.."));
+	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
+	uri_refput(uri);
+
+	ck_assert_int_eq(0, uri_create(&uri, UT_HTTPS, "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_local(uri));
+	uri_refput(uri);
+
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://a.b.c/.."));
 	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://a.b.c/../.."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://a.b.c/d/../.."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://a.b.c/d/../../.."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://./."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://.."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://../.."));
+	ck_assert_int_eq(-EINVAL, uri_create(&uri, UT_HTTPS, "https://../../.."));
 
 	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "rsync://a.b.c/d"));
 	ck_assert_int_eq(ENOTHTTPS, uri_create(&uri, UT_HTTPS, "http://a.b.c/d"));
@@ -116,13 +144,13 @@ START_TEST(check_caged)
 
 	ck_assert_int_eq(0, uri_create(&notification, UT_HTTPS, "https://a.b.c/d/e.xml"));
 	ck_assert_int_eq(0, uri_create(&uri, UT_CAGED, "rsync://x.y.z/v/w.cer"));
-	ck_assert_str_eq("rrdp/https/a.b.c/d/e.xml/rsync/x.y.z/v/w.cer", uri_get_local(uri));
+	ck_assert_str_eq("rrdp/a.b.c/d/e.xml/x.y.z/v/w.cer", uri_get_local(uri));
 	uri_refput(uri);
 	uri_refput(notification);
 
-	ck_assert_int_eq(0, uri_create(&notification, UT_RSYNC, "rsync://a.b.c"));
-	ck_assert_int_eq(0, uri_create(&uri, UT_CAGED, "https://w"));
-	ck_assert_str_eq("rrdp/rsync/a.b.c/https/w", uri_get_local(uri));
+	ck_assert_int_eq(0, uri_create(&notification, UT_HTTPS, "https://a.b.c"));
+	ck_assert_int_eq(0, uri_create(&uri, UT_CAGED, "rsync://w"));
+	ck_assert_str_eq("rrdp/a.b.c/w", uri_get_local(uri));
 	uri_refput(uri);
 	uri_refput(notification);
 }