From 570da97a0687c10153ce6b78ac3ce7daa1d8be83 Mon Sep 17 00:00:00 2001 From: pvyawaha Date: Fri, 11 Dec 2020 20:36:46 -0800 Subject: [PATCH 01/21] Fix sudo file path issue --- platform/posix/ota_pal/source/ota_pal_posix.c | 81 ++++++++++++------- 1 file changed, 53 insertions(+), 28 deletions(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 023c537bbf..6a2428d5dd 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -1,5 +1,5 @@ /* - * OTA PAL V2.0.0 (Release Candidate) for POSIX + * OTA PAL for POSIX V2.0.0 * Copyright (C) 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Permission is hereby granted, free of charge, to any person obtaining a copy of @@ -30,6 +30,7 @@ #include #include #include +#include #include "ota.h" #include "ota_pal_posix.h" @@ -57,7 +58,12 @@ static const char signingcredentialSIGNING_CERTIFICATE_PEM[] = "Paste code signi /** * @brief Size of buffer used in file operations on this platform (POSIX). */ -#define OTA_PAL_POSIX_BUF_SIZE ( ( size_t ) 4096U ) +#define OTA_PAL_POSIX_BUF_SIZE ( ( size_t ) 4096U ) + +/** + * @brief Name of the file used for storing platform image state. + */ +#define OTA_PLATFORM_IMAGE_STATE_FILE "/PlatformImageState.txt" /** * @brief Specify the OTA signature algorithm we support on this platform. @@ -374,11 +380,18 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { if( C->pFilePath[ 0 ] != ( uint8_t ) '/' ) { - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - int res = snprintf( realFilePath, OTA_FILE_PATH_LENGTH_MAX, "%s/%s", getenv( "PWD" ), C->pFilePath ); - assert( res >= 0 ); - ( void ) res; /* Suppress the unused variable warning when assert is off. */ + /* Get current directory. */ + char * pFileName = getcwd( realFilePath, OTA_FILE_PATH_LENGTH_MAX ); + + if( pFileName == NULL ) + { + LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); + } + else + { + /* Add the filename . */ + strncat( realFilePath, C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); + } } else { @@ -541,22 +554,28 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, OtaPalMainStatus_t mainErr = OtaPalBadImageState; int32_t subErr = 0; FILE * pPlatformImageState = NULL; - char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ]; - + char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; ( void ) C; if( ( eState != OtaImageStateUnknown ) && ( eState <= OtaLastImageState ) ) { - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - int res = snprintf( imageStateFile, OTA_FILE_PATH_LENGTH_MAX, "%s/%s", getenv( "PWD" ), "PlatformImageState.txt" ); - assert( res >= 0 ); - ( void ) res; /* Suppress the unused variable warning when assert is off. */ + /* Get current directory. */ + char * pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - pPlatformImageState = fopen( imageStateFile, "w+b" ); + if( pFileName == NULL ) + { + LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); + } + else + { + /* Add the filename . */ + strncat( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE, sizeof( OTA_PLATFORM_IMAGE_STATE_FILE ) ); + + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + pPlatformImageState = fopen( imageStateFile, "w+b" ); + } if( pPlatformImageState != NULL ) { @@ -626,23 +645,29 @@ OtaPalStatus_t otaPal_ResetDevice( OtaFileContext_t * const C ) */ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) { - FILE * pPlatformImageState; + FILE * pPlatformImageState = NULL; OtaImageState_t eSavedAgentState = OtaImageStateUnknown; OtaPalImageState_t ePalState = OtaPalImageStateUnknown; - char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ]; + char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - int res = snprintf( imageStateFile, OTA_FILE_PATH_LENGTH_MAX, "%s/%s", getenv( "PWD" ), "PlatformImageState.txt" ); + ( void ) C; - assert( res >= 0 ); - ( void ) res; /* Suppress the unused variable warning when assert is off. */ + /* Get current directory. */ + char * pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); - ( void ) C; + if( pFileName == NULL ) + { + LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); + } + else + { + /* Add the filename . */ + strncat( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE, sizeof( OTA_PLATFORM_IMAGE_STATE_FILE ) ); - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - pPlatformImageState = fopen( imageStateFile, "r+b" ); + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + pPlatformImageState = fopen( imageStateFile, "r+b" ); + } if( pPlatformImageState != NULL ) { From 93f803f76532b42760550394ddf3d72c08b85a5b Mon Sep 17 00:00:00 2001 From: pvyawaha Date: Fri, 11 Dec 2020 20:52:59 -0800 Subject: [PATCH 02/21] fix path for update file --- platform/posix/ota_pal/source/ota_pal_posix.c | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 6a2428d5dd..55eb43630d 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -390,6 +390,7 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) else { /* Add the filename . */ + realFilePath[ strlen( realFilePath ) ] = '/'; strncat( realFilePath, C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); } } From 8dea7a133ee429b34aec5a5ae15c133189fe9c40 Mon Sep 17 00:00:00 2001 From: pvyawaha Date: Fri, 11 Dec 2020 21:07:04 -0800 Subject: [PATCH 03/21] fix compiler warning --- platform/posix/ota_pal/source/ota_pal_posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 55eb43630d..6cf3320f01 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -391,7 +391,7 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { /* Add the filename . */ realFilePath[ strlen( realFilePath ) ] = '/'; - strncat( realFilePath, C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); + strncat( realFilePath, ( char * ) C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); } } else From 05922a295aa4c5577d9d377065d4117d300a7a64 Mon Sep 17 00:00:00 2001 From: Prasad Vyawahare Date: Fri, 11 Dec 2020 21:53:39 -0800 Subject: [PATCH 04/21] Update platform/posix/ota_pal/source/ota_pal_posix.c Co-authored-by: Shubham Divekar --- platform/posix/ota_pal/source/ota_pal_posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 6cf3320f01..87b160537d 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -1,5 +1,5 @@ /* - * OTA PAL for POSIX V2.0.0 + * OTA PAL V2.0.0 (Release Candidate) for POSIX * Copyright (C) 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Permission is hereby granted, free of charge, to any person obtaining a copy of From c6a48f25cbf9f8e18859dee5e6db8cab1a754434 Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Sat, 12 Dec 2020 06:03:29 +0000 Subject: [PATCH 05/21] fix ut --- platform/posix/ota_pal/source/ota_pal_posix.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 87b160537d..d32f5f5459 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -381,7 +381,8 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) if( C->pFilePath[ 0 ] != ( uint8_t ) '/' ) { /* Get current directory. */ - char * pFileName = getcwd( realFilePath, OTA_FILE_PATH_LENGTH_MAX ); + char * pFileName = NULL; + pFileName = getcwd( realFilePath, OTA_FILE_PATH_LENGTH_MAX ); if( pFileName == NULL ) { @@ -562,7 +563,8 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, if( ( eState != OtaImageStateUnknown ) && ( eState <= OtaLastImageState ) ) { /* Get current directory. */ - char * pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); + char * pFileName = NULL; + pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); if( pFileName == NULL ) { @@ -654,7 +656,8 @@ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) ( void ) C; /* Get current directory. */ - char * pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); + char * pFileName = NULL; + pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); if( pFileName == NULL ) { From 130011b6452499fce70a91cf6a46acd4b6d82e51 Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Sat, 12 Dec 2020 06:19:29 +0000 Subject: [PATCH 06/21] fix ut --- platform/posix/ota_pal/source/ota_pal_posix.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index d32f5f5459..ee0260a032 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -373,6 +373,7 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { OtaPalStatus_t result = OTA_PAL_COMBINE_ERR( OtaPalUninitialized, 0 ); char realFilePath[ OTA_FILE_PATH_LENGTH_MAX ]; + char * pFileName = NULL; if( C != NULL ) { @@ -381,7 +382,6 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) if( C->pFilePath[ 0 ] != ( uint8_t ) '/' ) { /* Get current directory. */ - char * pFileName = NULL; pFileName = getcwd( realFilePath, OTA_FILE_PATH_LENGTH_MAX ); if( pFileName == NULL ) @@ -557,13 +557,13 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, int32_t subErr = 0; FILE * pPlatformImageState = NULL; char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; + char * pFileName = NULL; ( void ) C; if( ( eState != OtaImageStateUnknown ) && ( eState <= OtaLastImageState ) ) { /* Get current directory. */ - char * pFileName = NULL; pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); if( pFileName == NULL ) @@ -652,11 +652,11 @@ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) OtaImageState_t eSavedAgentState = OtaImageStateUnknown; OtaPalImageState_t ePalState = OtaPalImageStateUnknown; char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; + char * pFileName = NULL; ( void ) C; /* Get current directory. */ - char * pFileName = NULL; pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); if( pFileName == NULL ) From 2445cb95c1c308f6530daed4039f3934730a7fa0 Mon Sep 17 00:00:00 2001 From: pvyawaha Date: Sat, 12 Dec 2020 12:04:49 -0800 Subject: [PATCH 07/21] Dump platform image state path on error --- platform/posix/ota_pal/source/ota_pal_posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index ee0260a032..3f8c75bcdb 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -613,7 +613,7 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, } else { - LogError( ( "Unable to open image state file. error -- %d", errno ) ); + LogError( ( "Unable to open image state file. Path: %s error: %s", imageStateFile, strerror( errno ) ) ); subErr = errno; } } From 8e4aebff6c0bdd4b81903d113d48b33ea7bfe9b7 Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Sun, 13 Dec 2020 05:23:32 +0000 Subject: [PATCH 08/21] Update submodule ptr --- libraries/aws/ota-for-aws-iot-embedded-sdk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/aws/ota-for-aws-iot-embedded-sdk b/libraries/aws/ota-for-aws-iot-embedded-sdk index 96178c7ce7..cec4c0f857 160000 --- a/libraries/aws/ota-for-aws-iot-embedded-sdk +++ b/libraries/aws/ota-for-aws-iot-embedded-sdk @@ -1 +1 @@ -Subproject commit 96178c7ce7fb89f34da39ab1b3a5bdc66f323c1b +Subproject commit cec4c0f857acd8023789dac07bec49d6a2fc35e8 From 024456d2f4cfa77946c22bd0f1330906d20f0892 Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sat, 12 Dec 2020 22:55:14 -0800 Subject: [PATCH 09/21] Remove snprintf mock since it's not being used --- platform/posix/ota_pal/utest/mocks/stdio_api.h | 7 ------- platform/posix/ota_pal/utest/ota_pal_posix_utest.c | 4 ---- 2 files changed, 11 deletions(-) diff --git a/platform/posix/ota_pal/utest/mocks/stdio_api.h b/platform/posix/ota_pal/utest/mocks/stdio_api.h index c5d159dfac..82b993ae87 100644 --- a/platform/posix/ota_pal/utest/mocks/stdio_api.h +++ b/platform/posix/ota_pal/utest/mocks/stdio_api.h @@ -44,13 +44,6 @@ extern _STDIO_FILE_TYPE * fopen( const char * __filename, /* Close STREAM. */ extern int fclose( _STDIO_FILE_TYPE * __stream ); -/* CMock does not support variadic functions. This alias replaces the original - * function name to get around this issue. */ -extern int snprintf_alias( char * s, - size_t n, - const char * format, - ... ); - extern size_t fread( void * ptr, size_t size, size_t count, diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index ac0a39a585..dfa4026f94 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -1045,7 +1045,6 @@ void test_OTAPAL_GetPlatformImageState_fclose_fails( void ) const int fclose_fail_val = EOF; /* Predefine what functions are expected to be called. */ - snprintf_alias_ExpectAnyArgsAndReturn( snprintf_success_val ); fopen_ExpectAnyArgsAndReturn( fopen_success_val ); fread_ExpectAnyArgsAndReturn( fread_success_val ); fclose_ExpectAnyArgsAndReturn( fclose_fail_val ); @@ -1085,7 +1084,6 @@ void test_OTAPAL_GetPlatformImageState_ValidStates( void ) /* Test the scenario where the platform state is OtaImageStateTesting. */ freadResultingState = OtaImageStateTesting; /* Predefine what functions are expected to be called. */ - snprintf_alias_ExpectAnyArgsAndReturn( snprintf_success_val ); fopen_ExpectAnyArgsAndReturn( fopen_success_val ); fread_ExpectAnyArgsAndReturn( fread_success_val ); fread_ReturnThruPtr_ptr( &freadResultingState ); @@ -1097,7 +1095,6 @@ void test_OTAPAL_GetPlatformImageState_ValidStates( void ) /* Test the scenario where the platform state is OtaImageStateAccepted. */ freadResultingState = OtaImageStateAccepted; /* Predefine what functions are expected to be called. */ - snprintf_alias_ExpectAnyArgsAndReturn( snprintf_success_val ); fopen_ExpectAnyArgsAndReturn( fopen_success_val ); fread_ExpectAnyArgsAndReturn( fread_success_val ); fread_ReturnThruPtr_ptr( &freadResultingState ); @@ -1109,7 +1106,6 @@ void test_OTAPAL_GetPlatformImageState_ValidStates( void ) /* Test the scenario where the platform state is an unexpected value. */ freadResultingState = invalidImageState; /* Predefine what functions are expected to be called. */ - snprintf_alias_ExpectAnyArgsAndReturn( snprintf_success_val ); fopen_ExpectAnyArgsAndReturn( fopen_success_val ); fread_ExpectAnyArgsAndReturn( fread_success_val ); fread_ReturnThruPtr_ptr( &freadResultingState ); From 6355e902a2c31702ca86f5cf62a0c6f6e8e0fa49 Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sat, 12 Dec 2020 22:57:47 -0800 Subject: [PATCH 10/21] Remove remaining usage of snprintf in OTA PAL tests --- platform/lexicon.txt | 1 - platform/posix/ota_pal/utest/ota_config.h | 4 ---- platform/posix/ota_pal/utest/ota_pal_posix_utest.c | 12 ------------ 3 files changed, 17 deletions(-) diff --git a/platform/lexicon.txt b/platform/lexicon.txt index ded26cf631..1660cf3d60 100644 --- a/platform/lexicon.txt +++ b/platform/lexicon.txt @@ -199,7 +199,6 @@ sizeof sleeptimems sni snihostname -snprintf sockaddr sockets_invalid_parameter socketstatus diff --git a/platform/posix/ota_pal/utest/ota_config.h b/platform/posix/ota_pal/utest/ota_config.h index c99c1a5580..35e2f9959d 100644 --- a/platform/posix/ota_pal/utest/ota_config.h +++ b/platform/posix/ota_pal/utest/ota_config.h @@ -167,8 +167,4 @@ * "fwrite". The function declaration for this alias is in "stdio_api.h". */ #define fwrite fwrite_alias -/* CMock does not support variadic functions. This alias replaces the original - * function name to get around this issue. */ -#define snprintf snprintf_alias - #endif /* _OTA_CONFIG_H_ */ diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index dfa4026f94..4a78aaffca 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -79,7 +79,6 @@ typedef enum CRYPTO_free_fn, fopen_fn, fclose_fn, - snprintf_fn, feof_fn, fread_fn, fseek_alias_fn, @@ -275,10 +274,6 @@ static void OTA_PAL_FailSingleMock_stdio( MockFunctionNames_t funcToFail, { static FILE dummyFile; - /* On success, snprintf returns a positive number that is less than the amount of data requested. */ - const int snprintf_success = 0; - const int snprintf_failure = -1; - int snprintf_return; /* On success, fopen returns a FILE address that is not null. */ FILE * const fopen_success = &dummyFile; FILE * const fopen_failure = NULL; @@ -315,9 +310,6 @@ static void OTA_PAL_FailSingleMock_stdio( MockFunctionNames_t funcToFail, fopen_return = ( funcToFail == fopen_fn ) ? fopen_failure : fopen_success; fopen_IgnoreAndReturn( fopen_return ); - snprintf_return = ( funcToFail == snprintf_fn ) ? snprintf_failure : snprintf_success; - snprintf_alias_IgnoreAndReturn( snprintf_return ); - fread_return = ( funcToFail == fread_fn ) ? fread_failure : fread_success; fread_IgnoreAndReturn( fread_return ); fread_ReturnThruPtr_ptr( pFreadStateToSet ); @@ -1033,8 +1025,6 @@ void test_OTAPAL_GetPlatformImageState_fclose_fails( void ) OtaFileContext_t otaFileContext; FILE dummyFile; - /* On success, snprintf returns a positive number that is less than the amount of data requested. */ - const int snprintf_success_val = 0; /* On success, fopen returns a FILE address that is not null. */ FILE * const fopen_success_val = &dummyFile; @@ -1070,8 +1060,6 @@ void test_OTAPAL_GetPlatformImageState_ValidStates( void ) * expected values. */ const OtaImageState_t invalidImageState = OtaLastImageState + 1; - /* On success, snprintf returns a positive number that is less than the amount of data requested. */ - const int snprintf_success_val = 0; /* On success, fopen returns a FILE address that is not null. */ FILE * const fopen_success_val = &dummyFile; From 55d100b4aef4c56bdc64876b092b8ed286093ca4 Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sun, 13 Dec 2020 01:39:30 -0800 Subject: [PATCH 11/21] Add getcwd mock to OTA PAL unit tests * Update OTA PAL include guard names * Add unit test cases for getcwd failing * Add unit test helper functions for getcwd pass/fail --- platform/posix/ota_pal/utest/CMakeLists.txt | 1 + .../posix/ota_pal/utest/mocks/openssl_api.h | 6 +- .../posix/ota_pal/utest/mocks/stdio_api.h | 6 +- .../posix/ota_pal/utest/mocks/unistd_api.h | 29 ++++++++++ .../posix/ota_pal/utest/ota_pal_posix_utest.c | 58 ++++++++++++++++++- 5 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 platform/posix/ota_pal/utest/mocks/unistd_api.h diff --git a/platform/posix/ota_pal/utest/CMakeLists.txt b/platform/posix/ota_pal/utest/CMakeLists.txt index 4308f66697..ebe5cb6e05 100644 --- a/platform/posix/ota_pal/utest/CMakeLists.txt +++ b/platform/posix/ota_pal/utest/CMakeLists.txt @@ -10,6 +10,7 @@ set( project_name "ota_pal" ) list( APPEND mock_list ${CMAKE_CURRENT_LIST_DIR}/mocks/stdio_api.h ${CMAKE_CURRENT_LIST_DIR}/mocks/openssl_api.h + ${CMAKE_CURRENT_LIST_DIR}/mocks/unistd_api.h ) #list the directories your mocks need list( APPEND mock_include_list diff --git a/platform/posix/ota_pal/utest/mocks/openssl_api.h b/platform/posix/ota_pal/utest/mocks/openssl_api.h index 621d29cbfd..db7fd712e2 100644 --- a/platform/posix/ota_pal/utest/mocks/openssl_api.h +++ b/platform/posix/ota_pal/utest/mocks/openssl_api.h @@ -20,8 +20,8 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#ifndef OPENSSL_API_H_ -#define OPENSSL_API_H_ +#ifndef OPENSSL_API_H +#define OPENSSL_API_H #include @@ -134,4 +134,4 @@ extern void * CRYPTO_malloc( size_t num, const char * file, int line ); -#endif /* ifndef OPENSSL_API_H_ */ +#endif /* ifndef OPENSSL_API_H */ diff --git a/platform/posix/ota_pal/utest/mocks/stdio_api.h b/platform/posix/ota_pal/utest/mocks/stdio_api.h index 82b993ae87..2efa90e6fd 100644 --- a/platform/posix/ota_pal/utest/mocks/stdio_api.h +++ b/platform/posix/ota_pal/utest/mocks/stdio_api.h @@ -20,8 +20,8 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#ifndef STDIO_API_H_ -#define STDIO_API_H_ +#ifndef STDIO_API_H +#define STDIO_API_H #include @@ -70,4 +70,4 @@ extern size_t fwrite_alias( const void * __restrict __ptr, size_t __n, _STDIO_FILE_TYPE * __restrict __s ); -#endif /* ifndef STDIO_API_H_ */ +#endif /* ifndef STDIO_API_H */ diff --git a/platform/posix/ota_pal/utest/mocks/unistd_api.h b/platform/posix/ota_pal/utest/mocks/unistd_api.h new file mode 100644 index 0000000000..ee1eed3f42 --- /dev/null +++ b/platform/posix/ota_pal/utest/mocks/unistd_api.h @@ -0,0 +1,29 @@ +/* + * OTA PAL V2.0.0 (Release Candidate) for POSIX + * Copyright (C) 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#ifndef UNISTD_API_H +#define UNISTD_API_H + +extern char * getcwd( char * buf, + size_t size ); + +#endif /* ifndef UNISTD_API_H */ diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index 4a78aaffca..142b6a4d5b 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -40,6 +40,7 @@ #include "ota_pal_posix.h" #include "mock_stdio_api.h" #include "mock_openssl_api.h" +#include "mock_unistd_api.h" /* errno error macro. errno.h can't be included in this file due to mocking. */ #define ENOENT 0x02 @@ -82,7 +83,8 @@ typedef enum feof_fn, fread_fn, fseek_alias_fn, - fwrite_alias_fn + fwrite_alias_fn, + getcwd_fn } MockFunctionNames_t; static void OTA_PAL_FailSingleMock_Except_fread( MockFunctionNames_t funcToFail, @@ -91,6 +93,7 @@ static void OTA_PAL_FailSingleMock_openssl_BIO( MockFunctionNames_t funcToFail ) static void OTA_PAL_FailSingleMock_openssl_X509( MockFunctionNames_t funcToFail ); static void OTA_PAL_FailSingleMock_openssl_EVP( MockFunctionNames_t funcToFail ); static void OTA_PAL_FailSingleMock_openssl_crypto( MockFunctionNames_t funcToFail ); +static void OTA_PAL_FailSingleMock_unistd( MockFunctionNames_t funcToFail ); static void OTA_PAL_FailSingleMock_stdio( MockFunctionNames_t funcToFail, OtaImageState_t * pFreadStateToSet ); static void OTA_PAL_FailSingleMock( MockFunctionNames_t funcToFail, @@ -262,6 +265,16 @@ static void OTA_PAL_FailSingleMock_openssl_crypto( MockFunctionNames_t funcToFai CRYPTO_free_Ignore(); } +static void OTA_PAL_FailSingleMock_unistd( MockFunctionNames_t funcToFail ) +{ + char * getcwd_success = "a"; + char * getcwd_failure = NULL; + char * getcwd_return; + + getcwd_return = ( funcToFail == getcwd_fn ) ? getcwd_failure : getcwd_success; + getcwd_IgnoreAndReturn( getcwd_return ); +} + /** * @brief Helper function specify a single point of failure. This needs to be * updated each time a mocked function is added or removed to the OTA PAL unit @@ -335,6 +348,7 @@ static void OTA_PAL_FailSingleMock( MockFunctionNames_t funcToFail, OTA_PAL_FailSingleMock_openssl_X509( funcToFail ); OTA_PAL_FailSingleMock_openssl_crypto( funcToFail ); OTA_PAL_FailSingleMock_openssl_EVP( funcToFail ); + OTA_PAL_FailSingleMock_unistd( funcToFail ); } /* ====================== OTA PAL ABORT UNIT TESTS ====================== */ @@ -443,7 +457,7 @@ void test_OTAPAL_CreateFileForRx_FailedToCreateFile( void ) testFile.pFile = &placeholder_file; fopen_ExpectAnyArgsAndReturn( NULL ); - + OTA_PAL_FailSingleMock_unistd( none_fn ); /* Create a file that exists with w+b mode */ result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &testFile ) ); TEST_ASSERT_EQUAL( OtaPalRxFileCreateFailed, result ); @@ -460,6 +474,7 @@ void test_OTAPAL_CreateFileForRx_ValidFileHandle( void ) otaFileContext.pFilePath = ( uint8_t * ) "placeholder_path"; + OTA_PAL_FailSingleMock_unistd( none_fn ); fopen_ExpectAnyArgsAndReturn( &placeholder_file ); result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); TEST_ASSERT_EQUAL( OtaPalSuccess, result ); @@ -477,17 +492,37 @@ void test_OTAPAL_CreateFileForRx_PathTypes( void ) /* Test for a leading forward slash in the path. */ otaFileContext.pFilePath = ( uint8_t * ) "/placeholder_path"; + OTA_PAL_FailSingleMock_unistd( none_fn ); fopen_ExpectAnyArgsAndReturn( &placeholder_file ); result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); TEST_ASSERT_EQUAL( OtaPalSuccess, result ); /* Test for no leading forward slash in the path. */ otaFileContext.pFilePath = ( uint8_t * ) "placeholder_path"; + OTA_PAL_FailSingleMock_unistd( none_fn ); fopen_ExpectAnyArgsAndReturn( &placeholder_file ); result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); TEST_ASSERT_EQUAL( OtaPalSuccess, result ); } +/** + * @brief Test that otaPal_CreateFileForRx will return correct result code. + */ +void test_OTAPAL_CreateFileForRx_getcwd_fail( void ) +{ + OtaPalMainStatus_t result; + FILE placeholder_file; + OtaFileContext_t otaFileContext; + OtaImageState_t validState = OtaImageStateTesting; + + + otaFileContext.pFilePath = ( uint8_t * ) "placeholder_path"; + + OTA_PAL_FailSingleMock( getcwd_fn, &validState ); + result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); + TEST_ASSERT_EQUAL( OtaPalSuccess, result ); +} + /* =================== OTA PAL CLOSE FILE UNIT TESTS ==================== */ void test_OTAPAL_CloseFile_NullInput( void ) @@ -935,6 +970,7 @@ void test_OTAPAL_SetPlatformImageState_HappyPath( void ) OtaFileContext_t otaFileContext; OtaImageState_t validState = OtaImageStateTesting; + OTA_PAL_FailSingleMock_unistd( none_fn ); OTA_PAL_FailSingleMock_stdio( none_fn, NULL ); result = otaPal_SetPlatformImageState( &otaFileContext, validState ); TEST_ASSERT_EQUAL( OtaPalSuccess, OTA_PAL_MAIN_ERR( result ) ); @@ -949,6 +985,7 @@ void test_OTAPAL_SetPlatformImageState_fopen_fail( void ) OtaFileContext_t otaFileContext; OtaImageState_t validState = OtaImageStateTesting; + OTA_PAL_FailSingleMock_unistd( none_fn ); OTA_PAL_FailSingleMock_stdio( fopen_fn, NULL ); result = otaPal_SetPlatformImageState( &otaFileContext, validState ); TEST_ASSERT_EQUAL( OtaPalBadImageState, OTA_PAL_MAIN_ERR( result ) ); @@ -963,6 +1000,7 @@ void test_OTAPAL_SetPlatformImageState_fwrite_fail( void ) OtaFileContext_t otaFileContext; OtaImageState_t validState = OtaImageStateTesting; + OTA_PAL_FailSingleMock_unistd( none_fn ); OTA_PAL_FailSingleMock_stdio( fwrite_alias_fn, NULL ); result = otaPal_SetPlatformImageState( &otaFileContext, validState ); TEST_ASSERT_EQUAL( OtaPalBadImageState, OTA_PAL_MAIN_ERR( result ) ); @@ -977,6 +1015,7 @@ void test_OTAPAL_SetPlatformImageState_fclose_fail( void ) OtaFileContext_t otaFileContext; OtaImageState_t validState = OtaImageStateTesting; + OTA_PAL_FailSingleMock_unistd( none_fn ); OTA_PAL_FailSingleMock_stdio( fclose_fn, NULL ); result = otaPal_SetPlatformImageState( &otaFileContext, validState ); TEST_ASSERT_EQUAL( OtaPalBadImageState, OTA_PAL_MAIN_ERR( result ) ); @@ -994,6 +1033,7 @@ void test_OTAPAL_GetPlatformImageState_fopen_fails( void ) OtaPalImageState_t ePalImageState; OtaFileContext_t otaFileContext; + OTA_PAL_FailSingleMock_unistd( none_fn ); OTA_PAL_FailSingleMock_stdio( fopen_fn, NULL ); /* The file failed to close, so it is invalid or in an unknown state. */ ePalImageState = otaPal_GetPlatformImageState( &otaFileContext ); @@ -1010,6 +1050,7 @@ void test_OTAPAL_GetPlatformImageState_fread_fails( void ) OtaPalImageState_t ePalImageState; OtaFileContext_t otaFileContext; + OTA_PAL_FailSingleMock_unistd( none_fn ); OTA_PAL_FailSingleMock_stdio( fread_fn, NULL ); ePalImageState = otaPal_GetPlatformImageState( &otaFileContext ); TEST_ASSERT_EQUAL( OtaPalImageStateInvalid, ePalImageState ); @@ -1035,6 +1076,7 @@ void test_OTAPAL_GetPlatformImageState_fclose_fails( void ) const int fclose_fail_val = EOF; /* Predefine what functions are expected to be called. */ + OTA_PAL_FailSingleMock_unistd( none_fn ); fopen_ExpectAnyArgsAndReturn( fopen_success_val ); fread_ExpectAnyArgsAndReturn( fread_success_val ); fclose_ExpectAnyArgsAndReturn( fclose_fail_val ); @@ -1069,6 +1111,7 @@ void test_OTAPAL_GetPlatformImageState_ValidStates( void ) /* On success, fclose returns a zero. */ const int fclose_success_val = 0; + OTA_PAL_FailSingleMock_unistd( none_fn ); /* Test the scenario where the platform state is OtaImageStateTesting. */ freadResultingState = OtaImageStateTesting; /* Predefine what functions are expected to be called. */ @@ -1102,3 +1145,14 @@ void test_OTAPAL_GetPlatformImageState_ValidStates( void ) ePalImageState = otaPal_GetPlatformImageState( &otaFileContext ); TEST_ASSERT_EQUAL( OtaPalImageStateInvalid, ePalImageState ); } + +void test_OTAPAL_GetPlatformImageState_getcwd_fail( void ) +{ + OtaPalImageState_t ePalImageState; + OtaFileContext_t otaFileContext; + + OTA_PAL_FailSingleMock_unistd( getcwd_fn ); + OTA_PAL_FailSingleMock_stdio( none_fn, NULL ); + ePalImageState = otaPal_GetPlatformImageState( &otaFileContext ); + TEST_ASSERT_EQUAL( OtaPalImageStateValid, ePalImageState ); +} From 8853b8e69824ef6c1e5625f75b323850be2b5fa3 Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sun, 13 Dec 2020 01:48:57 -0800 Subject: [PATCH 12/21] Remove unused variable from OTA PAl tests --- platform/posix/ota_pal/utest/ota_pal_posix_utest.c | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index 142b6a4d5b..553d6e4158 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -511,7 +511,6 @@ void test_OTAPAL_CreateFileForRx_PathTypes( void ) void test_OTAPAL_CreateFileForRx_getcwd_fail( void ) { OtaPalMainStatus_t result; - FILE placeholder_file; OtaFileContext_t otaFileContext; OtaImageState_t validState = OtaImageStateTesting; From 9627a0a24c87cd462211b272287d6466fd956e5e Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Sun, 13 Dec 2020 21:51:07 +0000 Subject: [PATCH 13/21] Extracting file path generation in a separate function --- platform/posix/ota_pal/source/ota_pal_posix.c | 94 ++++++++++--------- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 3f8c75bcdb..155fc28056 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -63,7 +63,7 @@ static const char signingcredentialSIGNING_CERTIFICATE_PEM[] = "Paste code signi /** * @brief Name of the file used for storing platform image state. */ -#define OTA_PLATFORM_IMAGE_STATE_FILE "/PlatformImageState.txt" +#define OTA_PLATFORM_IMAGE_STATE_FILE "PlatformImageState.txt" /** * @brief Specify the OTA signature algorithm we support on this platform. @@ -89,6 +89,15 @@ static OtaPalMainStatus_t Openssl_DigestVerify( EVP_MD_CTX * pSigContext, */ static OtaPalStatus_t otaPal_CheckFileSignature( OtaFileContext_t * const C ); +/** + * @brief Get the absolute file path from the environment. + * + * @param realFilePath Buffer to store the file path + file name. + * @param pFilePath File name to append to the end of current path. + */ +static void getFilePathFromCWD( char * realFilePath, + const char * pFilePath ); + /*-----------------------------------------------------------*/ static EVP_PKEY * Openssl_GetPkeyFromCertificate( uint8_t * pCertFilePath ) @@ -323,6 +332,33 @@ static OtaPalStatus_t otaPal_CheckFileSignature( OtaFileContext_t * const C ) return OTA_PAL_COMBINE_ERR( mainErr, 0 ); } +static void getFilePathFromCWD( char * pCompleteFilePath, + const char * pFileName ) +{ + char * pCurrentDir = NULL; + + /* Get current directory. */ + pCurrentDir = getcwd( pCompleteFilePath, OTA_FILE_PATH_LENGTH_MAX - 1 ); + + if( pCurrentDir == NULL ) + { + LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); + } + else + { + /* Add the filename . */ + if( strlen( pCompleteFilePath ) + strlen( pFileName ) + 2U > OTA_FILE_PATH_LENGTH_MAX ) + { + LogError( ( "Insufficient space to generate file path" ) ); + } + else + { + pCompleteFilePath[ strlen( pCompleteFilePath ) ] = '/'; + strncat( pCompleteFilePath, pFileName, strlen( pFileName ) + 1U ); + } + } +} + /*-----------------------------------------------------------*/ OtaPalStatus_t otaPal_Abort( OtaFileContext_t * const C ) @@ -373,7 +409,7 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { OtaPalStatus_t result = OTA_PAL_COMBINE_ERR( OtaPalUninitialized, 0 ); char realFilePath[ OTA_FILE_PATH_LENGTH_MAX ]; - char * pFileName = NULL; + if( C != NULL ) { @@ -381,19 +417,7 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { if( C->pFilePath[ 0 ] != ( uint8_t ) '/' ) { - /* Get current directory. */ - pFileName = getcwd( realFilePath, OTA_FILE_PATH_LENGTH_MAX ); - - if( pFileName == NULL ) - { - LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); - } - else - { - /* Add the filename . */ - realFilePath[ strlen( realFilePath ) ] = '/'; - strncat( realFilePath, ( char * ) C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); - } + getFilePathFromCWD( realFilePath, ( const char * ) C->pFilePath ); } else { @@ -563,22 +587,12 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, if( ( eState != OtaImageStateUnknown ) && ( eState <= OtaLastImageState ) ) { - /* Get current directory. */ - pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); + /* Get file path for the image state file. */ + getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); - if( pFileName == NULL ) - { - LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); - } - else - { - /* Add the filename . */ - strncat( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE, sizeof( OTA_PLATFORM_IMAGE_STATE_FILE ) ); - - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - pPlatformImageState = fopen( imageStateFile, "w+b" ); - } + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + pPlatformImageState = fopen( imageStateFile, "w+b" ); if( pPlatformImageState != NULL ) { @@ -656,22 +670,12 @@ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) ( void ) C; - /* Get current directory. */ - pFileName = getcwd( imageStateFile, OTA_FILE_PATH_LENGTH_MAX ); + /* Get file path for the image state file. */ + getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); - if( pFileName == NULL ) - { - LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); - } - else - { - /* Add the filename . */ - strncat( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE, sizeof( OTA_PLATFORM_IMAGE_STATE_FILE ) ); - - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - pPlatformImageState = fopen( imageStateFile, "r+b" ); - } + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + pPlatformImageState = fopen( imageStateFile, "r+b" ); if( pPlatformImageState != NULL ) { From 7b0541a3931878c9e5f9726e1edc6857a7b11d7e Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Sun, 13 Dec 2020 21:56:57 +0000 Subject: [PATCH 14/21] Removing unused variables --- platform/posix/ota_pal/source/ota_pal_posix.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 155fc28056..2cd28c0a5d 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -581,7 +581,6 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, int32_t subErr = 0; FILE * pPlatformImageState = NULL; char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; - char * pFileName = NULL; ( void ) C; @@ -666,7 +665,6 @@ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) OtaImageState_t eSavedAgentState = OtaImageStateUnknown; OtaPalImageState_t ePalState = OtaPalImageStateUnknown; char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; - char * pFileName = NULL; ( void ) C; From 5bf5d2524c226e85c8331bc9a93682ab505a876c Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sun, 13 Dec 2020 15:27:36 -0800 Subject: [PATCH 15/21] Add OTA PAL test for long file paths --- .../posix/ota_pal/utest/ota_pal_posix_utest.c | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index 553d6e4158..614a084ef7 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -505,6 +505,34 @@ void test_OTAPAL_CreateFileForRx_PathTypes( void ) TEST_ASSERT_EQUAL( OtaPalSuccess, result ); } +/** + * @brief Test that otaPal_CreateFileForRx will correctly handle a file path + * that is too long. + */ +void test_OTAPAL_CreateFileForRx_InvalidPathLength( void ) +{ + OtaPalMainStatus_t result; + FILE placeholder_file; + OtaFileContext_t otaFileContext; + const size_t invalidLength = OTA_FILE_PATH_LENGTH_MAX + 1U; + char invalidLengthPath[ invalidLength ]; + size_t i; + + /* Test calling getcwd and having it return a path that is too long. */ + for( i = 0U; i < ( invalidLength - 1U ); ++i ) + { + invalidLengthPath[ i ] = 'x'; + } + + invalidLengthPath[ invalidLength - 1U ] = '\0'; + otaFileContext.pFilePath = ( uint8_t * ) "placeholder_path"; + getcwd_ExpectAnyArgsAndReturn( "placeholder_return" ); + getcwd_ReturnArrayThruPtr_buf( invalidLengthPath, invalidLength ); + fopen_ExpectAnyArgsAndReturn( &placeholder_file ); + result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); + TEST_ASSERT_EQUAL( OtaPalSuccess, result ); +} + /** * @brief Test that otaPal_CreateFileForRx will return correct result code. */ @@ -1153,5 +1181,5 @@ void test_OTAPAL_GetPlatformImageState_getcwd_fail( void ) OTA_PAL_FailSingleMock_unistd( getcwd_fn ); OTA_PAL_FailSingleMock_stdio( none_fn, NULL ); ePalImageState = otaPal_GetPlatformImageState( &otaFileContext ); - TEST_ASSERT_EQUAL( OtaPalImageStateValid, ePalImageState ); + TEST_ASSERT_EQUAL( OtaPalImageStateInvalid, ePalImageState ); } From 7a5c6a36ed11d4aafcefd12ff493b1129f4e00c6 Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Mon, 14 Dec 2020 00:04:55 +0000 Subject: [PATCH 16/21] Addressing PR comments --- .../ota_pal/source/include/ota_pal_posix.h | 13 +++++- platform/posix/ota_pal/source/ota_pal_posix.c | 42 ++++++++++++++----- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/platform/posix/ota_pal/source/include/ota_pal_posix.h b/platform/posix/ota_pal/source/include/ota_pal_posix.h index 44bb706ff4..239f8c511d 100644 --- a/platform/posix/ota_pal/source/include/ota_pal_posix.h +++ b/platform/posix/ota_pal/source/include/ota_pal_posix.h @@ -28,13 +28,22 @@ #include "ota.h" - - /** * @brief Maximum file path length on Linux */ #define OTA_FILE_PATH_LENGTH_MAX 512 +/** + * @brief The OTA platform interface status for generating + * absolute file path from the incoming relative file path. + */ +typedef enum OtaPalPathGenStatus +{ + OtaPalFileGenSuccess, /*!< @brief Absolute path generation success. */ + OtaPalCWDFailed, /*!< @brief getcwd failed to output path. */ + OtaPalBufferInsufficient /*!< @brief Buffer insufficient for storing the file path. */ +} OtaPalPathGenStatus_t; + /** * @brief Abort an OTA transfer. * diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 2cd28c0a5d..3e9a9cccaa 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -95,8 +95,8 @@ static OtaPalStatus_t otaPal_CheckFileSignature( OtaFileContext_t * const C ); * @param realFilePath Buffer to store the file path + file name. * @param pFilePath File name to append to the end of current path. */ -static void getFilePathFromCWD( char * realFilePath, - const char * pFilePath ); +static OtaPalPathGenStatus_t getFilePathFromCWD( char * realFilePath, + const char * pFilePath ); /*-----------------------------------------------------------*/ @@ -332,10 +332,11 @@ static OtaPalStatus_t otaPal_CheckFileSignature( OtaFileContext_t * const C ) return OTA_PAL_COMBINE_ERR( mainErr, 0 ); } -static void getFilePathFromCWD( char * pCompleteFilePath, - const char * pFileName ) +static OtaPalPathGenStatus_t getFilePathFromCWD( char * pCompleteFilePath, + const char * pFileName ) { char * pCurrentDir = NULL; + OtaPalPathGenStatus_t status = OtaPalFileGenSuccess; /* Get current directory. */ pCurrentDir = getcwd( pCompleteFilePath, OTA_FILE_PATH_LENGTH_MAX - 1 ); @@ -343,6 +344,7 @@ static void getFilePathFromCWD( char * pCompleteFilePath, if( pCurrentDir == NULL ) { LogError( ( "Failed to get current working directory: %s", strerror( errno ) ) ); + status = OtaPalCWDFailed; } else { @@ -350,13 +352,16 @@ static void getFilePathFromCWD( char * pCompleteFilePath, if( strlen( pCompleteFilePath ) + strlen( pFileName ) + 2U > OTA_FILE_PATH_LENGTH_MAX ) { LogError( ( "Insufficient space to generate file path" ) ); + status = OtaPalBufferInsufficient; } else { - pCompleteFilePath[ strlen( pCompleteFilePath ) ] = '/'; - strncat( pCompleteFilePath, pFileName, strlen( pFileName ) + 1U ); + strcat( pCompleteFilePath, "/" ); + strncat( pCompleteFilePath, pFileName, strlen( pFileName ) ); } } + + return status; } /*-----------------------------------------------------------*/ @@ -409,7 +414,7 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { OtaPalStatus_t result = OTA_PAL_COMBINE_ERR( OtaPalUninitialized, 0 ); char realFilePath[ OTA_FILE_PATH_LENGTH_MAX ]; - + OtaPalPathGenStatus_t status = OtaPalFileGenSuccess; if( C != NULL ) { @@ -417,13 +422,18 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) { if( C->pFilePath[ 0 ] != ( uint8_t ) '/' ) { - getFilePathFromCWD( realFilePath, ( const char * ) C->pFilePath ); + status = getFilePathFromCWD( realFilePath, ( const char * ) C->pFilePath ); } else { ( void ) strncpy( realFilePath, ( const char * ) C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); } + if( status != OtaPalFileGenSuccess ) + { + LogError( ( "Could not generate the absolute path for the file" ) ); + } + /* POSIX port using standard library */ /* coverity[misra_c_2012_rule_21_6_violation] */ C->pFile = fopen( ( const char * ) realFilePath, "w+b" ); @@ -578,6 +588,7 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, OtaImageState_t eState ) { OtaPalMainStatus_t mainErr = OtaPalBadImageState; + OtaPalPathGenStatus_t status = OtaPalFileGenSuccess; int32_t subErr = 0; FILE * pPlatformImageState = NULL; char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; @@ -587,7 +598,12 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, if( ( eState != OtaImageStateUnknown ) && ( eState <= OtaLastImageState ) ) { /* Get file path for the image state file. */ - getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); + status = getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); + + if( status != OtaPalFileGenSuccess ) + { + LogError( ( "Could not generate the absolute path for the file" ) ); + } /* POSIX port using standard library */ /* coverity[misra_c_2012_rule_21_6_violation] */ @@ -664,12 +680,18 @@ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) FILE * pPlatformImageState = NULL; OtaImageState_t eSavedAgentState = OtaImageStateUnknown; OtaPalImageState_t ePalState = OtaPalImageStateUnknown; + OtaPalPathGenStatus_t status = OtaPalFileGenSuccess; char imageStateFile[ OTA_FILE_PATH_LENGTH_MAX ] = { 0 }; ( void ) C; /* Get file path for the image state file. */ - getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); + status = getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); + + if( status != OtaPalFileGenSuccess ) + { + LogError( ( "Could not generate the absolute path for the file" ) ); + } /* POSIX port using standard library */ /* coverity[misra_c_2012_rule_21_6_violation] */ From 4784e65172417f5a4a2651e4cf779a831c35569a Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Mon, 14 Dec 2020 00:28:36 +0000 Subject: [PATCH 17/21] spell fix --- platform/lexicon.txt | 49 +++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/platform/lexicon.txt b/platform/lexicon.txt index 1660cf3d60..0ebf43d6c8 100644 --- a/platform/lexicon.txt +++ b/platform/lexicon.txt @@ -12,6 +12,7 @@ basedefs bio bitmasking blocksize +blocksize bool bootable bootloader @@ -27,6 +28,7 @@ cert cmock com config +config connectsuccessindex const couldn @@ -122,26 +124,45 @@ ota otafile otaimagestateaborted otaimagestateaccepted -otaimagestaterejected -otaimagestatetesting otaimagestateinvalid otaimagestatependingcommit otaimagestaterejected +otaimagestaterejected +otaimagestatetesting otaimagestateunknown otalastimagestate +otapal_closefile +otapalabortfailed +otapalactivatefailed +otapalbadimagestate +otapalbadsignercert +otapalbootinfocreatefailed +otapalbufferinsufficient +otapalcommitfailed +otapalcwdfailed +otapalfileabort +otapalfileclose +otapalfilegensuccess otapalimagestateinvalid otapalimagestatependingcommit otapalimagestateunknown otapalimagestatevalid otapalnullfilecontext +otapalnullfilecontext +otapaloutofmemory otapaloutofmemory otapalrejectfailed +otapalrejectfailed otapalrxfilecreatefailed +otapalrxfilecreatefailed +otapalrxfiletoolarge otapalrxfiletoolarge otapalsignaturecheckfailed +otapalsignaturecheckfailed +otapalsuccess otapalsuccess otapaluninitialized -otapal_closefile +otapaluninitialized paddrinfo palpnprotos param @@ -151,9 +172,11 @@ pcdata pcertfilepath pclientcertpath pdata +pdata pem pfile pfilecontext +pfilecontext pfilepath pformat phostname @@ -238,23 +261,3 @@ variadic vtaskdelay writesize www -blocksize -config -otapalsuccess -otapaluninitialized -otapaloutofmemory -otapalnullfilecontext -otapalsignaturecheckfailed -otapalrxfilecreatefailed -otapalrxfiletoolarge -otapalbootinfocreatefailed -otapalbadsignercert -otapalbadimagestate -otapalabortfailed -otapalrejectfailed -otapalcommitfailed -otapalactivatefailed -otapalfileabort -otapalfileclose -pdata -pfilecontext From a3d852db7cfbb536eddf4de696d8ec0f070a7702 Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Mon, 14 Dec 2020 00:50:39 +0000 Subject: [PATCH 18/21] Refactoring to use status and spell fix --- platform/posix/ota_pal/source/ota_pal_posix.c | 102 ++++++++++-------- 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index 3e9a9cccaa..eacfa11587 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -429,24 +429,27 @@ OtaPalStatus_t otaPal_CreateFileForRx( OtaFileContext_t * const C ) ( void ) strncpy( realFilePath, ( const char * ) C->pFilePath, strlen( ( const char * ) C->pFilePath ) + 1U ); } - if( status != OtaPalFileGenSuccess ) + if( status == OtaPalFileGenSuccess ) { - LogError( ( "Could not generate the absolute path for the file" ) ); - } - - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - C->pFile = fopen( ( const char * ) realFilePath, "w+b" ); + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + C->pFile = fopen( ( const char * ) realFilePath, "w+b" ); - if( C->pFile != NULL ) - { - result = OTA_PAL_COMBINE_ERR( OtaPalSuccess, 0 ); - LogInfo( ( "Receive file created." ) ); + if( C->pFile != NULL ) + { + result = OTA_PAL_COMBINE_ERR( OtaPalSuccess, 0 ); + LogInfo( ( "Receive file created." ) ); + } + else + { + result = OTA_PAL_COMBINE_ERR( OtaPalRxFileCreateFailed, errno ); + LogError( ( "Failed to start operation: Operation already started. failed to open -- %s Path ", C->pFilePath ) ); + } } else { - result = OTA_PAL_COMBINE_ERR( OtaPalRxFileCreateFailed, errno ); - LogError( ( "Failed to start operation: Operation already started. failed to open -- %s Path ", C->pFilePath ) ); + LogError( ( "Could not generate the absolute path for the file" ) ); + result = OTA_PAL_COMBINE_ERR( OtaPalRxFileCreateFailed, 0 ); } } else @@ -600,15 +603,17 @@ OtaPalStatus_t otaPal_SetPlatformImageState( OtaFileContext_t * const C, /* Get file path for the image state file. */ status = getFilePathFromCWD( imageStateFile, OTA_PLATFORM_IMAGE_STATE_FILE ); - if( status != OtaPalFileGenSuccess ) + if( status == OtaPalFileGenSuccess ) + { + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + pPlatformImageState = fopen( imageStateFile, "w+b" ); + } + else { LogError( ( "Could not generate the absolute path for the file" ) ); } - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - pPlatformImageState = fopen( imageStateFile, "w+b" ); - if( pPlatformImageState != NULL ) { /* Write the image state to PlatformImageState.txt. */ @@ -691,51 +696,54 @@ OtaPalImageState_t otaPal_GetPlatformImageState( OtaFileContext_t * const C ) if( status != OtaPalFileGenSuccess ) { LogError( ( "Could not generate the absolute path for the file" ) ); + ePalState = OtaPalImageStateInvalid; } - - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - pPlatformImageState = fopen( imageStateFile, "r+b" ); - - if( pPlatformImageState != NULL ) + else { /* POSIX port using standard library */ /* coverity[misra_c_2012_rule_21_6_violation] */ - if( 1U != fread( &eSavedAgentState, sizeof( OtaImageState_t ), 1, pPlatformImageState ) ) - { - /* If an error occurred reading the file, mark the state as aborted. */ - LogError( ( "Failed to read image state file." ) ); - ePalState = OtaPalImageStateInvalid; - } - else + pPlatformImageState = fopen( imageStateFile, "r+b" ); + + if( pPlatformImageState != NULL ) { - if( eSavedAgentState == OtaImageStateTesting ) + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + if( 1U != fread( &eSavedAgentState, sizeof( OtaImageState_t ), 1, pPlatformImageState ) ) { - ePalState = OtaPalImageStatePendingCommit; + /* If an error occurred reading the file, mark the state as aborted. */ + LogError( ( "Failed to read image state file." ) ); + ePalState = OtaPalImageStateInvalid; } - else if( eSavedAgentState == OtaImageStateAccepted ) + else { - ePalState = OtaPalImageStateValid; + if( eSavedAgentState == OtaImageStateTesting ) + { + ePalState = OtaPalImageStatePendingCommit; + } + else if( eSavedAgentState == OtaImageStateAccepted ) + { + ePalState = OtaPalImageStateValid; + } + else + { + ePalState = OtaPalImageStateInvalid; + } } - else + + /* POSIX port using standard library */ + /* coverity[misra_c_2012_rule_21_6_violation] */ + if( 0 != fclose( pPlatformImageState ) ) { + LogError( ( "Failed to close image state file." ) ); ePalState = OtaPalImageStateInvalid; } } - - /* POSIX port using standard library */ - /* coverity[misra_c_2012_rule_21_6_violation] */ - if( 0 != fclose( pPlatformImageState ) ) + else { - LogError( ( "Failed to close image state file." ) ); - ePalState = OtaPalImageStateInvalid; + /* If no image state file exists, assume a factory image. */ + ePalState = OtaPalImageStateValid; /*lint !e64 Allow assignment. */ } } - else - { - /* If no image state file exists, assume a factory image. */ - ePalState = OtaPalImageStateValid; /*lint !e64 Allow assignment. */ - } return ePalState; } From 28c31fa4acd9a7015293e63087d2dccb1e84dd9f Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sun, 13 Dec 2020 17:08:10 -0800 Subject: [PATCH 19/21] Fix OTA PAL test asserts --- platform/posix/ota_pal/utest/ota_pal_posix_utest.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index 614a084ef7..84bd9b50e2 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -528,13 +528,13 @@ void test_OTAPAL_CreateFileForRx_InvalidPathLength( void ) otaFileContext.pFilePath = ( uint8_t * ) "placeholder_path"; getcwd_ExpectAnyArgsAndReturn( "placeholder_return" ); getcwd_ReturnArrayThruPtr_buf( invalidLengthPath, invalidLength ); - fopen_ExpectAnyArgsAndReturn( &placeholder_file ); result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); - TEST_ASSERT_EQUAL( OtaPalSuccess, result ); + TEST_ASSERT_EQUAL( OtaPalRxFileCreateFailed, result ); } /** - * @brief Test that otaPal_CreateFileForRx will return correct result code. + * @brief Test that otaPal_CreateFileForRx will handle the getcwd command + * failing. */ void test_OTAPAL_CreateFileForRx_getcwd_fail( void ) { @@ -547,7 +547,7 @@ void test_OTAPAL_CreateFileForRx_getcwd_fail( void ) OTA_PAL_FailSingleMock( getcwd_fn, &validState ); result = OTA_PAL_MAIN_ERR( otaPal_CreateFileForRx( &otaFileContext ) ); - TEST_ASSERT_EQUAL( OtaPalSuccess, result ); + TEST_ASSERT_EQUAL( OtaPalRxFileCreateFailed, result ); } /* =================== OTA PAL CLOSE FILE UNIT TESTS ==================== */ @@ -826,7 +826,7 @@ void test_OTAPAL_CloseFile_EVP_DigestVerifyInit_fail( void ) * implementation. It is defined by the "OTA_PAL_POSIX_BUF_SIZE" macro in the * OTA posix PAL implementation .c file. */ -void test_OTAPAL_CloseFile_MaxBlockSize() +void test_OTAPAL_CloseFile_MaxBlockSize( void ) { const size_t OTA_PAL_POSIX_BUF_SIZE = 4096U; OtaPalStatus_t result; @@ -1083,10 +1083,6 @@ void test_OTAPAL_GetPlatformImageState_fread_fails( void ) TEST_ASSERT_EQUAL( OtaPalImageStateInvalid, ePalImageState ); } -/** - * @brief This test validates that the valid states are correctly returned to - * the caller. - * */ void test_OTAPAL_GetPlatformImageState_fclose_fails( void ) { OtaPalImageState_t ePalImageState = OtaPalImageStateUnknown; From 28d0453ba19cff49715936bd796a46a24802f91d Mon Sep 17 00:00:00 2001 From: Joshua Yan <52796499+yanjos-dev@users.noreply.github.com> Date: Sun, 13 Dec 2020 17:12:48 -0800 Subject: [PATCH 20/21] Remove unused variable --- platform/posix/ota_pal/utest/ota_pal_posix_utest.c | 1 - 1 file changed, 1 deletion(-) diff --git a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c index 84bd9b50e2..cfff282519 100644 --- a/platform/posix/ota_pal/utest/ota_pal_posix_utest.c +++ b/platform/posix/ota_pal/utest/ota_pal_posix_utest.c @@ -512,7 +512,6 @@ void test_OTAPAL_CreateFileForRx_PathTypes( void ) void test_OTAPAL_CreateFileForRx_InvalidPathLength( void ) { OtaPalMainStatus_t result; - FILE placeholder_file; OtaFileContext_t otaFileContext; const size_t invalidLength = OTA_FILE_PATH_LENGTH_MAX + 1U; char invalidLengthPath[ invalidLength ]; From 8e4a008c671b3a39f450ac191c011630c4c01f51 Mon Sep 17 00:00:00 2001 From: Shubham Divekar Date: Mon, 14 Dec 2020 02:18:30 +0000 Subject: [PATCH 21/21] Using strcat instead on strncat and ota submodule update --- libraries/aws/ota-for-aws-iot-embedded-sdk | 2 +- platform/posix/ota_pal/source/ota_pal_posix.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/aws/ota-for-aws-iot-embedded-sdk b/libraries/aws/ota-for-aws-iot-embedded-sdk index cec4c0f857..f33fdb0fb0 160000 --- a/libraries/aws/ota-for-aws-iot-embedded-sdk +++ b/libraries/aws/ota-for-aws-iot-embedded-sdk @@ -1 +1 @@ -Subproject commit cec4c0f857acd8023789dac07bec49d6a2fc35e8 +Subproject commit f33fdb0fb0416561ccee70beaabc7663711177a2 diff --git a/platform/posix/ota_pal/source/ota_pal_posix.c b/platform/posix/ota_pal/source/ota_pal_posix.c index eacfa11587..4e209b1dc3 100644 --- a/platform/posix/ota_pal/source/ota_pal_posix.c +++ b/platform/posix/ota_pal/source/ota_pal_posix.c @@ -357,7 +357,7 @@ static OtaPalPathGenStatus_t getFilePathFromCWD( char * pCompleteFilePath, else { strcat( pCompleteFilePath, "/" ); - strncat( pCompleteFilePath, pFileName, strlen( pFileName ) ); + strcat( pCompleteFilePath, pFileName ); } }