Cover the error cases of parsing protocol versions

Also, detect an additional failure type. Thanks, tests!

(How distinctly I recall thee)

diff --git a/src/or/protover.c b/src/or/protover.c
index 3a2d4014cd0dc03d05d0243caef42f12a1eb1727..2bde02c6aefdfd784ec2696bfbcc278e57f17417 100644 (file)
--- a/src/or/protover.c
+++ b/src/or/protover.c
@@ -39,8 +39,10 @@ protocol_type_to_str(protocol_type_t pr)
     if (PROTOCOL_NAMES[i].protover_type == pr)
       return PROTOCOL_NAMES[i].name;
   }
+  /* LCOV_EXCL_START */
   tor_assert_nonfatal_unreached_once();
   return "UNKNOWN";
+  /* LCOV_EXCL_STOP */
 }
 
 /**
@@ -150,6 +152,10 @@ parse_single_entry(const char *s, const char *end_of_entry)
   if (!equals)
     goto error;
 
+  /* The name must be nonempty */
+  if (equals == s)
+    goto error;
+
   out->name = tor_strndup(s, equals-s);
 
   tor_assert(equals < end_of_entry);
@@ -388,7 +394,7 @@ contract_protocol_list(const smartlist_t *proto_strings)
   SMARTLIST_FOREACH_BEGIN(proto_strings, const char *, s) {
     proto_entry_t *ent = parse_single_entry(s, s+strlen(s));
     if (BUG(!ent))
-      continue;
+      continue; // LCOV_EXCL_LINE
     smartlist_t *lst = strmap_get(entry_lists_by_name, ent->name);
     if (!lst) {
       smartlist_add(all_names, ent->name);
@@ -591,11 +597,11 @@ protocol_list_contains(const smartlist_t *protos,
                        protocol_type_t pr, uint32_t ver)
 {
   if (BUG(protos == NULL)) {
-    return 0;
+    return 0; // LCOV_EXCL_LINE
   }
   const char *pr_name = protocol_type_to_str(pr);
   if (BUG(pr_name == NULL)) {
-    return 0;
+    return 0; // LCOV_EXCL_LINE
   }
 
   SMARTLIST_FOREACH_BEGIN(protos, const proto_entry_t *, ent) {

diff --git a/src/test/test_protover.c b/src/test/test_protover.c
index 651d2fd52d328560b38ca517c6246a4ff57c136d..c86c3b2f69a9fbd981beb2baf45dbf6efab8b916 100644 (file)
--- a/src/test/test_protover.c
+++ b/src/test/test_protover.c
@@ -80,6 +80,41 @@ test_protover_parse(void *arg)
   tor_free(re_encoded);
 }
 
+static void
+test_protover_parse_fail(void *arg)
+{
+  (void)arg;
+  smartlist_t *elts;
+
+  /* random junk */
+  elts = parse_protocol_list("!!3@*");
+  tt_assert(elts == NULL);
+
+  /* Missing equals sign in an entry */
+  elts = parse_protocol_list("Link=4 Haprauxymatyve Desc=9");
+  tt_assert(elts == NULL);
+
+  /* Missing word. */
+  elts = parse_protocol_list("Link=4 =3 Desc=9");
+  tt_assert(elts == NULL);
+
+  /* Broken numbers */
+  elts = parse_protocol_list("Link=fred");
+  tt_assert(elts == NULL);
+  elts = parse_protocol_list("Link=1,fred");
+  tt_assert(elts == NULL);
+  elts = parse_protocol_list("Link=1,fred,3");
+  tt_assert(elts == NULL);
+
+  /* Broken range */
+  elts = parse_protocol_list("Link=1,9-8,3");
+  tt_assert(elts == NULL);
+
+ done:
+  ;
+}
+
+
 static void
 test_protover_vote(void *arg)
 {
@@ -154,6 +189,7 @@ test_protover_all_supported(void *arg)
 
 struct testcase_t protover_tests[] = {
   PV_TEST(parse, 0),
+  PV_TEST(parse_fail, 0),
   PV_TEST(vote, 0),
   PV_TEST(all_supported, 0),
   END_OF_TESTCASES