Sanitize fragments_to_string() errorcodes, add tests for frags w/o fmetadata

Addresses issue#10

4 files changed, 64 insertions, 13 deletions

diff --git a/Makefile.am b/Makefile.am
index cf64273..4191beb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -36,12 +36,12 @@ test: check
 	$(eval DYLD_LIBRARY_PATH := DYLD_LIBRARY_PATH="$(subst / ,/:,$(SODIRS))")
 	$(eval DYLD_FALLBACK_LIBRARY_PATH := DYLD_FALLBACK_LIBRARY_PATH="$(subst / ,/:,$(SODIRS))")
 	@$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) \
+		./test/liberasurecode_test
+	@$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) \
 		./test/alg_sig_test
 	@$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) \
 		./test/test_xor_hd_code
 	@$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) \
-		./test/liberasurecode_test
-	@$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) \
 		./test/libec_slap
  
 VALGRIND_EXEC_COMMAND = $(LIBTOOL_COMMAND) valgrind --tool=memcheck \
diff --git a/src/erasurecode.c b/src/erasurecode.c
index 2c6a30b..1544ab7 100644
--- a/src/erasurecode.c
+++ b/src/erasurecode.c
@@ -566,7 +566,10 @@ int liberasurecode_decode(int desc,
                                   available_fragments, num_fragments,
                                   out_data, out_data_len);
 
-        if (ret == 0) {
+        if (ret == -1) {
+            /* Ignore - not necessarily an error
+             * (see fragments_to_string() in src/erasurecode_preprocessing.c) */
+        } else if (ret <= 0) {
             /* We were able to get the original data without decoding! */
             goto out;
         }
diff --git a/src/erasurecode_preprocessing.c b/src/erasurecode_preprocessing.c
index b157d69..a9a7b55 100644
--- a/src/erasurecode_preprocessing.c
+++ b/src/erasurecode_preprocessing.c
@@ -139,14 +139,14 @@ int prepare_fragments_for_decode(
             data[i] = alloc_fragment_buffer(fragment_size - sizeof(fragment_header_t));
             if (NULL == data[i]) {
                 log_error("Could not allocate data buffer!");
-                return -1;
+                return -ENOMEM;
             }
             *realloc_bm = *realloc_bm | (1 << i);
         } else if (!is_addr_aligned((unsigned long)data[i], 16)) {
             char *tmp_buf = alloc_fragment_buffer(fragment_size - sizeof(fragment_header_t));
             if (NULL == tmp_buf) {
                 log_error("Could not allocate temp buffer!");
-                return -1;
+                return -ENOMEM;
             }
             memcpy(tmp_buf, data[i], fragment_size);
             data[i] = tmp_buf;
@@ -158,12 +158,12 @@ int prepare_fragments_for_decode(
             orig_data_size = get_orig_data_size(data[i]);
             if (orig_data_size < 0) {
                 log_error("Invalid orig_data_size in fragment header!");
-                return -1;
+                return -EBADHEADER;
             }
             payload_size = get_fragment_payload_size(data[i]);
             if (orig_data_size < 0) {
                 log_error("Invalid fragment_size in fragment header!");
-                return -1;
+                return -EBADHEADER;
             }
        }
     }
@@ -178,14 +178,14 @@ int prepare_fragments_for_decode(
             parity[i] = alloc_fragment_buffer(fragment_size-sizeof(fragment_header_t));
             if (NULL == parity[i]) {
                 log_error("Could not allocate parity buffer!");
-                return -1;
+                return -ENOMEM;
             }
             *realloc_bm = *realloc_bm | (1 << (k + i));
         } else if (!is_addr_aligned((unsigned long)parity[i], 16)) {
             char *tmp_buf = alloc_fragment_buffer(fragment_size-sizeof(fragment_header_t));
             if (NULL == tmp_buf) {
                 log_error("Could not allocate temp buffer!");
-                return -1;
+                return -ENOMEM;
             }
             memcpy(tmp_buf, parity[i], fragment_size);
             parity[i] = tmp_buf;
@@ -286,7 +286,7 @@ int fragments_to_string(int k, int m,
         data_size = get_fragment_payload_size(fragments[i]);
         if ((index < 0) || (data_size < 0)) {
             log_error("Invalid fragment header information!");
-            ret = -EINVALIDPARAMS;
+            ret = -EBADHEADER;
             goto out;
         }
 
@@ -296,7 +296,7 @@ int fragments_to_string(int k, int m,
         } else {
             if (get_orig_data_size(fragments[i]) != orig_data_size) {
                 log_error("Inconsistent orig_data_size in fragment header!");
-                ret = -EINVALIDPARAMS;
+                ret = -EBADHEADER;
                 goto out;
             }
         }
diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c
index e92795e..d1ad516 100644
--- a/test/liberasurecode_test.c
+++ b/test/liberasurecode_test.c
@@ -177,6 +177,31 @@ int *create_skips_array(struct ec_args *args, int skip)
     return buf;
 }
 
+static int create_fake_frags_no_meta(char ***array, int num_frags,
+                                     const char *data, int data_len)
+{
+    int _num_frags = 0;
+    int i = 0;
+    char **ptr = NULL;
+
+    *array = malloc(num_frags * sizeof(char *));
+    if (array == NULL) {
+        _num_frags = -1;
+        goto out;
+    }
+
+    // add data and parity frags
+    ptr = *array;
+    for (i = 0; i < num_frags; i++) {
+        *ptr = (char *) malloc(data_len);
+        strncpy(*ptr++, data, data_len);
+        _num_frags++;
+    }
+
+out:
+    return _num_frags;
+}
+
 static int create_frags_array(char ***array,
                               char **data,
                               char **parity,
@@ -391,6 +416,7 @@ static void test_decode_invalid_args()
     int *skips = create_skips_array(&null_args, -1);
     char *decoded_data = NULL;
     uint64_t decoded_data_len = 0;
+    const char *fake_data = " ";
 
     desc = liberasurecode_instance_create(EC_BACKEND_NULL, &null_args);
     if (-EBACKENDNOTAVAIL == desc) {
@@ -398,12 +424,34 @@ static void test_decode_invalid_args()
         return;
     }
     assert(desc > 0);
+
+    // test with invalid fragments (no metadata headers)
+    num_avail_frags = create_fake_frags_no_meta(&avail_frags, (null_args.k +
+                                                null_args.m),
+                                                fake_data, strlen(fake_data));
+    assert(num_avail_frags > 0);
+
+    rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
+                               strlen(fake_data), 1,
+                               &decoded_data, &decoded_data_len);
+    assert(rc == -EBADHEADER);
+
+    // test with num_fragments < (k)
+    num_avail_frags = create_fake_frags_no_meta(&avail_frags, (null_args.k - 1),
+                                                " ", 1);
+    assert(num_avail_frags > 0);
+    rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
+                               strlen(fake_data), 1,
+                               &decoded_data, &decoded_data_len);
+    assert(rc == -EINSUFFFRAGS);
+
     rc = liberasurecode_encode(desc, orig_data, orig_data_size,
             &encoded_data, &encoded_parity, &encoded_fragment_len);
-    assert(0 == rc);
+    assert(rc == 0);
 
     num_avail_frags = create_frags_array(&avail_frags, encoded_data,
                                          encoded_parity, &null_args, skips);
+    assert(num_avail_frags > 0);
 
     rc = liberasurecode_decode(-1, avail_frags, num_avail_frags,
                                encoded_fragment_len, 1,
@@ -698,7 +746,7 @@ static void encode_decode_test_impl(const ec_backend_id_t be_id,
 
     num_avail_frags = create_frags_array(&avail_frags, encoded_data,
                                          encoded_parity, args, skip);
-    assert(num_avail_frags != -1);
+    assert(num_avail_frags > 0);
     rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
                                encoded_fragment_len, 1,
                                &decoded_data, &decoded_data_len);