Merged PR 922452: Fix to file IO write mode

This was uncovered on a test on an integration test on Windows that resulted in \n being replaced by \r\n when writing to a file. - Fixed to file IO write mode and added more tests for the specific scenario. - Minor cleanups in the test code. Shoutout to Raj who helped debug this.
Federico-Figalli · Jun 25, 2018 · 81b5a7f · 81b5a7f
1 parent fd5ace2
commit 81b5a7f
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 23 deletions.
diff --git a/edgelet/hsm-sys/azure-iot-hsm-c/src/hsm_utils.c b/edgelet/hsm-sys/azure-iot-hsm-c/src/hsm_utils.c
@@ -244,7 +244,7 @@ static bool is_windows()
     #endif
 }
 
-static int write_ascii_buffer_into_file
+static int write_buffer_into_file
 (
     const char *file_name,
     const void *input_buffer,
@@ -257,7 +257,7 @@ static int write_ascii_buffer_into_file
     if (is_windows() || !make_private)
     {
         FILE *file_handle;
-        if ((file_handle = fopen(file_name, "w")) == NULL)
+        if ((file_handle = fopen(file_name, "wb")) == NULL)
         {
             LOG_ERROR("Could not open file for writing %s", file_name);
             result = HSM_UTIL_ERROR;
@@ -534,7 +534,7 @@ int write_cstring_to_file(const char* file_name, const char* data)
     }
     else
     {
-        result = write_ascii_buffer_into_file(file_name, data, strlen(data), false);
+        result = write_buffer_into_file(file_name, data, strlen(data), false);
     }
 
     return result;
@@ -567,7 +567,7 @@ int write_buffer_to_file
     }
     else
     {
-        result = write_ascii_buffer_into_file(file_name, data, data_size, make_private);
+        result = write_buffer_into_file(file_name, data, data_size, make_private);
     }
 
     return result;

diff --git a/edgelet/hsm-sys/azure-iot-hsm-c/tests/edge_hsm_util_int/edge_hsm_util_int.c b/edgelet/hsm-sys/azure-iot-hsm-c/tests/edge_hsm_util_int/edge_hsm_util_int.c
@@ -10,16 +10,6 @@
 #include "azure_c_shared_utility/gballoc.h"
 #include "hsm_log.h"
 
-// //#############################################################################
-// // Declare and enable MOCK definitions
-// //#############################################################################
-
-#define TEST_FILE_ALPHA "test_alpha.txt"
-#define TEST_FILE_NUMERIC "test_numeric.txt"
-#define TEST_FILE_BAD "test_bad.txt"
-#define TEST_FILE_EMPTY "test_empty.txt"
-#define TEST_WRITE_FILE "test_write_data.txt"
-#define TEST_WRITE_FILE_FOR_DELETE "test_write_data_del.txt"
 
 //#############################################################################
 // Interface(s) under test
@@ -31,6 +21,20 @@
 // Test defines and data
 //#############################################################################
 
+#define TEST_FILE_ALPHA "test_alpha.txt"
+#define TEST_FILE_ALPHA_NEWLINE "test_alpha_newline.txt"
+#define TEST_FILE_NUMERIC "test_numeric.txt"
+#define TEST_FILE_NUMERIC_NEWLINE "test_numeric_newline.txt"
+#define TEST_FILE_BAD "test_bad.txt"
+#define TEST_FILE_EMPTY "test_empty.txt"
+#define TEST_WRITE_FILE "test_write_data.txt"
+#define TEST_WRITE_FILE_FOR_DELETE "test_write_data_del.txt"
+
+static char ALPHA[] = "ABCD";
+static char ALPHA_NEWLINE[] = "AB\nCD\n";
+static unsigned char NUMERIC[] = {'1', '2', '3', '4'};
+static unsigned char NUMERIC_NEWLINE[] = {'1', '2', '\n', '4', '5', '\n'};
+
 static TEST_MUTEX_HANDLE g_testByTest;
 static TEST_MUTEX_HANDLE g_dllByDll;
 
@@ -61,7 +65,7 @@ int test_helper_write_data_to_file
 {
     FILE *file_handle;
     int result;
-    if ((file_handle = fopen(file_name, "w")) == NULL)
+    if ((file_handle = fopen(file_name, "wb")) == NULL)
     {
         LOG_ERROR("Could not open file for write %s", file_name);
         result = __LINE__;
@@ -105,10 +109,10 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
             g_testByTest = TEST_MUTEX_CREATE();
             ASSERT_IS_NOT_NULL(g_testByTest);
 
-            char alpha[] = "ABCD";
-            ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_ALPHA, alpha, strlen(alpha)));
-            unsigned char numeric[] = {'1', '2', '3', '4'};
-            ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_NUMERIC, numeric, sizeof(numeric)));
+            ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_ALPHA, ALPHA, strlen(ALPHA)));
+            ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_ALPHA_NEWLINE, ALPHA_NEWLINE, strlen(ALPHA_NEWLINE)));
+            ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_NUMERIC, NUMERIC, sizeof(NUMERIC)));
+            ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_NUMERIC_NEWLINE, NUMERIC_NEWLINE, sizeof(NUMERIC_NEWLINE)));
             ASSERT_ARE_EQUAL(int, 0, test_helper_write_data_to_file(TEST_FILE_EMPTY, NULL, 0));
             test_helper_setup_homedir();
         }
@@ -117,6 +121,8 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
         {
             delete_file_if_exists(TEST_FILE_ALPHA);
             delete_file_if_exists(TEST_FILE_NUMERIC);
+            delete_file_if_exists(TEST_FILE_ALPHA_NEWLINE);
+            delete_file_if_exists(TEST_FILE_NUMERIC_NEWLINE);
             delete_file_if_exists(TEST_FILE_EMPTY);
 
             TEST_MUTEX_DESTROY(g_testByTest);
@@ -139,8 +145,8 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
         TEST_FUNCTION(read_file_into_cstring_smoke)
         {
             // arrange
-            char *expected_string = "ABCD";
-            size_t expected_string_size = 5;
+            char *expected_string = ALPHA;
+            size_t expected_string_size = sizeof(ALPHA);
 
             // act
             size_t output_size = 0;
@@ -156,6 +162,26 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
             free(output_string);
         }
 
+        TEST_FUNCTION(read_file_into_cstring_with_newline_smoke)
+        {
+            // arrange
+            char *expected_string = ALPHA_NEWLINE;
+            size_t expected_string_size = sizeof(ALPHA_NEWLINE);
+
+            // act
+            size_t output_size = 0;
+            char *output_string = read_file_into_cstring(TEST_FILE_ALPHA_NEWLINE, &output_size);
+
+            // assert
+            ASSERT_IS_NOT_NULL(output_string);
+            int cmp_result = strcmp(expected_string, output_string);
+            ASSERT_ARE_EQUAL_WITH_MSG(int, 0, cmp_result, "Line:" TOSTRING(__LINE__));
+            ASSERT_ARE_EQUAL_WITH_MSG(size_t, expected_string_size, output_size, "Line:" TOSTRING(__LINE__));
+
+            // cleanup
+            free(output_string);
+        }
+
         TEST_FUNCTION(read_file_into_cstring_non_existant_file_returns_null)
         {
             // arrange
@@ -210,8 +236,8 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
         TEST_FUNCTION(read_file_into_cbuffer_smoke)
         {
             // arrange
-            unsigned char expected_buffer[] = {'1', '2', '3', '4'};
-            size_t expected_buffer_size = 4;
+            unsigned char *expected_buffer = NUMERIC;
+            size_t expected_buffer_size = sizeof(NUMERIC);
 
             // act
             size_t output_size = 0;
@@ -227,6 +253,26 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
             free(output_buffer);
         }
 
+        TEST_FUNCTION(read_file_into_cbuffer_newline_smoke)
+        {
+            // arrange
+            unsigned char *expected_buffer = NUMERIC_NEWLINE;
+            size_t expected_buffer_size = sizeof(NUMERIC_NEWLINE);
+
+            // act
+            size_t output_size = 0;
+            unsigned char *output_buffer = read_file_into_buffer(TEST_FILE_NUMERIC_NEWLINE, &output_size);
+
+            // assert
+            ASSERT_IS_NOT_NULL(output_buffer);
+            int cmp_result = memcmp(expected_buffer, output_buffer, expected_buffer_size);
+            ASSERT_ARE_EQUAL_WITH_MSG(int, 0, cmp_result, "Line:" TOSTRING(__LINE__));
+            ASSERT_ARE_EQUAL_WITH_MSG(size_t, expected_buffer_size, output_size, "Line:" TOSTRING(__LINE__));
+
+            // cleanup
+            free(output_buffer);
+        }
+
         TEST_FUNCTION(read_file_into_cbuffer_invalid_params_returns_null)
         {
             // arrange
@@ -321,6 +367,30 @@ BEGIN_TEST_SUITE(edge_hsm_util_int_tests)
             free(output_string);
         }
 
+        TEST_FUNCTION(concat_files_to_cstring_newline_smoke)
+        {
+            // arrange
+            char *expected_string = "AB\nCD\n12\n45\n";
+            size_t expected_string_size = 13;
+            const char *files[] = {
+                TEST_FILE_ALPHA_NEWLINE,
+                TEST_FILE_NUMERIC_NEWLINE
+            };
+
+            // act
+            char *output_string = concat_files_to_cstring(files, sizeof(files)/sizeof(files[0]));
+            size_t output_size = strlen(output_string) + 1;
+
+            // assert
+            ASSERT_IS_NOT_NULL(output_string);
+            int cmp_result = strcmp(expected_string, output_string);
+            ASSERT_ARE_EQUAL_WITH_MSG(int, 0, cmp_result, "Line:" TOSTRING(__LINE__));
+            ASSERT_ARE_EQUAL_WITH_MSG(size_t, expected_string_size, output_size, "Line:" TOSTRING(__LINE__));
+
+            // cleanup
+            free(output_string);
+        }
+
         TEST_FUNCTION(concat_files_to_cstring_with_empty_file_smoke)
         {
             // arrange