Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions include/ut_kvp.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,16 @@ void ut_kvp_destroyInstance(ut_kvp_instance_t *pInstance);
* This function opens the specified KVP file, reads its contents, and parses the key-value pairs into the given KVP instance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also update about url here

*
* @param[in] pInstance - Handle to the KVP instance where the parsed data will be stored.
* @param[in] fileName - Null-terminated string containing the path to the KVP file.
* @param[in] fileNameOrUrl - Null-terminated string containing the path to the KVP file/URL.
*
* @returns Status of the operation (`ut_kvp_status_t`):
* @retval UT_KVP_STATUS_SUCCESS - The file was opened and parsed successfully.
* @retval UT_KVP_STATUS_FILE_OPEN_ERROR - The file could not be opened.
* @retval UT_KVP_STATUS_INVALID_PARAM - One or more parameters are invalid (e.g., null pointer).
* @retval UT_KVP_STATUS_NULL_PARAM - One or more parameters are NULL (e.g., null pointer).
* @retval UT_KVP_STATUS_PARSING_ERROR - An error occurred while parsing the file contents.
* @retval UT_KVP_STATUS_INVALID_INSTANCE - The provided `pInstance` is not a valid KVP instance.
*/
ut_kvp_status_t ut_kvp_open(ut_kvp_instance_t *pInstance, char *fileName);
ut_kvp_status_t ut_kvp_open(ut_kvp_instance_t *pInstance, const char *fileNameOrUrl);

/**
* @brief Parses a KVP payload from a caller-owned memory block into an instance.
Expand Down Expand Up @@ -269,4 +269,4 @@ unsigned char* ut_kvp_getDataBytes(ut_kvp_instance_t *pInstance, const char *psz
}
#endif

#endif /* __UT_KVP_H__ */
#endif /* __UT_KVP_H__ */
87 changes: 77 additions & 10 deletions src/ut_kvp.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ ut_kvp_instance_t *gKVP_Instance = NULL;
#define UT_KVP_MAGIC (0xdeadbeef)
#define UT_KVP_MAX_INCLUDE_DEPTH 5

#define UT_KVP_HTTPS_PREFIX "https://"
#define UT_KVP_HTTP_PREFIX "http://"
#define UT_KVP_FILE_PREFIX "file://"

// Automatically calculate lengths by subtracting the '\0' null terminator
#define UT_KVP_HTTPS_PREFIX_LEN (sizeof(UT_KVP_HTTPS_PREFIX) - 1)
#define UT_KVP_HTTP_PREFIX_LEN (sizeof(UT_KVP_HTTP_PREFIX) - 1)
#define UT_KVP_FILE_PREFIX_LEN (sizeof(UT_KVP_FILE_PREFIX) - 1)

typedef struct
{
uint32_t magic;
Expand Down Expand Up @@ -95,28 +104,86 @@ void ut_kvp_destroyInstance(ut_kvp_instance_t *pInstance)
pInternal = NULL;
}

ut_kvp_status_t ut_kvp_open(ut_kvp_instance_t *pInstance, char *fileName)
static bool is_url(const char *input)
{
if (pInstance == NULL)
if (input == NULL)
{
return false;
}

// Check for http://
if (strncmp(input, UT_KVP_HTTP_PREFIX, UT_KVP_HTTP_PREFIX_LEN) == 0)
{
return true;
}

// Check for https://
if (strncmp(input, UT_KVP_HTTPS_PREFIX, UT_KVP_HTTPS_PREFIX_LEN) == 0)
{
return true;
}

// No matches found
return false;
}

ut_kvp_status_t ut_kvp_open(ut_kvp_instance_t *pInstance, const char *fileNameOrUrl)
{
ut_kvp_instance_internal_t *pInternal = validateInstance(pInstance);

// Validate KVP instance handle
if (pInternal == NULL)
{
return UT_KVP_STATUS_INVALID_INSTANCE;
}

ut_kvp_instance_internal_t *pInternal = validateInstance(pInstance);
if (fileName == NULL)
// Validate fileNameOrUrl parameter
if (fileNameOrUrl == NULL)
{
UT_LOG_ERROR("Invalid Param [fileName]");
return UT_KVP_STATUS_INVALID_PARAM;
UT_LOG_ERROR("NULL PARAM [fileNameOrUrl]");
return UT_KVP_STATUS_NULL_PARAM;
}

// Determine if input is a URL(e.g., http:// or https://)
bool bFilenameIsAUrl = is_url(fileNameOrUrl);

// Handle URL-based input
if(bFilenameIsAUrl == true)
{
char *pYaml = NULL;

pYaml = malloc(UT_KVP_MAX_ELEMENT_SIZE);

if ( pYaml == NULL )
{
UT_LOG_ERROR("Malloc was not able to provide memory\n");
return UT_KVP_STATUS_PARSING_ERROR;
}

snprintf(pYaml, UT_KVP_MAX_ELEMENT_SIZE, "include: %s\n", fileNameOrUrl);

// Pass the dynamically allocated YAML string to openMemory() for parsing
ut_kvp_status_t status = ut_kvp_openMemory(pInstance, pYaml, strlen(pYaml));
free(pYaml);
return status;
}

// Handle file-based input
if (strncmp(fileNameOrUrl, UT_KVP_FILE_PREFIX, UT_KVP_FILE_PREFIX_LEN) == 0)
{
// Skip "file://"
fileNameOrUrl = fileNameOrUrl + UT_KVP_FILE_PREFIX_LEN;
}

if (access(fileName, F_OK) != 0)
// Verify that the file is accessible
if (access(fileNameOrUrl, F_OK) != 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this fail if the filename is a URL? Since the URL hasn't been downloaded you can't call access on it?

Why don't you get the error UT_KVP_STATUS_FILE_OPEN_ERROR on URL? in your testing is there a gap in your testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL path does not reach the access() check.
The logic flow ensures that:

if(bFilenameIsAUrl == true)

1.If the input is a URL (is_url(...) == true), the function immediately enters the 'if' block, constructs an in-memory YAML string, calls ut_kvp_openMemory(), and returns.
So URLs never fall through to the access() path.

2.The access(fileNameOrUrl, F_OK) check is executed only for non-URL file-based inputs.

So the code does not attempt to call access() on a URL, and therefore we do not get UT_KVP_STATUS_FILE_OPEN_ERROR for URL cases.

{
UT_LOG_ERROR("[%s] cannot be accesed", fileName);
UT_LOG_ERROR("[%s] cannot be accessed", fileNameOrUrl);
return UT_KVP_STATUS_FILE_OPEN_ERROR;
}

// Load the new document
struct fy_document *newDoc = fy_document_build_from_file(NULL, fileName);
struct fy_document *newDoc = fy_document_build_from_file(NULL, fileNameOrUrl);
if (newDoc == NULL || fy_document_resolve(newDoc) != 0)
{
if (newDoc)
Expand Down Expand Up @@ -1109,7 +1176,7 @@ static struct fy_node* process_include(const char *filename, int depth, struct f
return NULL;
}

if (strncmp(filename, "http:", 5) == 0 || strncmp(filename, "https:", 6) == 0)
if (strncmp(filename, UT_KVP_HTTP_PREFIX, UT_KVP_HTTP_PREFIX_LEN) == 0 || strncmp(filename, UT_KVP_HTTPS_PREFIX, UT_KVP_HTTPS_PREFIX_LEN) == 0)
{
// URL include
mChunk.memory = malloc(1);
Expand Down
24 changes: 23 additions & 1 deletion tests/src/ut_control_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,26 @@ cd "$(dirname "$0")"

export LD_LIBRARY_PATH=/usr/lib:/lib:/home/root:${MY_DIR}

LSAN_OPTIONS="suppressions=../../lsan.supp print_suppressions=1" ./ut_control_test $@
HTTP_PORT=8000
HTTP_PID=""

# Check if HTTP server is already running
if ! lsof -iTCP:${HTTP_PORT} -sTCP:LISTEN >/dev/null 2>&1; then
python3 -m http.server ${HTTP_PORT} &
HTTP_PID=$!
fi

# Cleanup only if this script started the server
cleanup() {
if [[ -n "$HTTP_PID" ]]; then
kill "$HTTP_PID" 2>/dev/null || true
wait "$HTTP_PID" 2>/dev/null || true
fi
}
trap cleanup EXIT

# Run test
LSAN_OPTIONS="suppressions=../../lsan.supp print_suppressions=1" ./ut_control_test "$@"
UT_STATUS=$?

exit $UT_STATUS
38 changes: 37 additions & 1 deletion tests/src/ut_test_kvp.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
#define KVP_VALID_TEST_SEQUENCE_INCLUDE_YAML "assets/include/sequence-include.yaml"
#define KVP_VALID_TEST_RESOLVE_YAML_TAGS_YAML "assets/yaml_tags.yaml"
#define KVP_VALID_TEST_RESOLVE_YAML_TAGS_IN_SEQUENCE_YAML "assets/yaml_tags_in_sequence.yaml"
#define KVP_VALID_TEST_URL_HTTPS "https://raw.githubusercontent.com/rdkcentral/ut-control/main/tests/src/assets/include/2s.yaml"
#define KVP_VALID_TEST_URI_HTTP "http://localhost:8000/assets/yaml_tags.yaml"
#define KVP_VALID_TEST_URI_FILE "file://assets/yaml_tags.yaml"
#define KVP_VALID_TEST_NOT_VALID_URL_HTTPS "HTTPS://raw.githubusercontent.com/rdkcentral/ut-control/main/tests/src/assets/include/2s.yaml"
#define KVP_VALID_TEST_NOT_VALID_URI_HTTP "HTTP://localhost:8000/assets/yaml_tags.yaml"
#define KVP_VALID_TEST_NOT_VALID_URI_FILE "FILE://assets/yaml_tags.yaml"

static ut_kvp_instance_t *gpMainTestInstance = NULL;
static UT_test_suite_t *gpKVPSuite = NULL;
Expand Down Expand Up @@ -108,7 +114,7 @@ void test_ut_kvp_open( void )
/* Negative Read Test, NULL PARAM */
UT_LOG_STEP("ut_kvp_open( pInstance, NULL ) - Negative");
status = ut_kvp_open( pInstance, NULL);
UT_ASSERT( status == UT_KVP_STATUS_INVALID_PARAM );
UT_ASSERT( status == UT_KVP_STATUS_NULL_PARAM );

/* Filename doesn't exist */
UT_LOG_STEP("ut_kvp_open( pInstance, %s - filename doesn't exist ) - Negative", KVP_VALID_TEST_NO_FILE);
Expand All @@ -120,11 +126,41 @@ void test_ut_kvp_open( void )
status = ut_kvp_open( pInstance, KVP_VALID_TEST_ZERO_LENGTH_YAML_FILE);
UT_ASSERT( status == UT_KVP_STATUS_PARSING_ERROR );

/* Negative Read Test, KVP_VALID_TEST_NOT_VALID_URL_HTTPS PARAM */
UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_URL_HTTPS ) - Negative");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_URL_HTTPS);
printf("Status = %d\n", status);
UT_ASSERT( status == UT_KVP_STATUS_FILE_OPEN_ERROR );

/* Negative Read Test, KVP_VALID_TEST_NOT_VALID_URI_HTTP PARAM */
UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_URI_HTTP ) - Negative");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_URI_HTTP);
printf("Status = %d\n", status);
UT_ASSERT( status == UT_KVP_STATUS_FILE_OPEN_ERROR );

/* Negative Read Test, KVP_VALID_TEST_NOT_VALID_URI_FILE PARAM */
UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_URI_FILE ) - Negative");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_URI_FILE);
printf("Status = %d\n", status);
UT_ASSERT( status == UT_KVP_STATUS_FILE_OPEN_ERROR );

/* Positive Tests */
UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_YAML_FILE ) - Positive");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_YAML_FILE);
UT_ASSERT( status == UT_KVP_STATUS_SUCCESS );

UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_URL_HTTPS ) - Positive");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_URL_HTTPS);
UT_ASSERT( status == UT_KVP_STATUS_SUCCESS );

UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_URI_HTTP ) - Positive");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_URI_HTTP);
UT_ASSERT( status == UT_KVP_STATUS_SUCCESS );

UT_LOG_STEP("ut_kvp_open( pInstance, KVP_VALID_TEST_URI_FILE ) - Positive");
status = ut_kvp_open( pInstance, KVP_VALID_TEST_URI_FILE);
UT_ASSERT( status == UT_KVP_STATUS_SUCCESS );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add comments on the purpose of each test and what your trying to achieve.

Both Positive & Negative testing are always required.

And add comments to be the goals of the tests specifically.

  1. Valid Filenames, for open file:// &
  2. Valid Filanemes for https:// & http://

Malformed URLS, and invalid filenames must also be checked..

CAPS HTTPS/HTTP/FILE should also be failing and tested.

Ensure that both params are validated.

Whilst you can test the ut_kvp_open() function, how do you know it's read the right data in?
You need to add another tests to ut_kvp_open() & read data to prove it's working then ut_kvp_close().

That's a L2 test, and also if not already existing will require a test, I would suggest having a test for points 1) & 2) above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated


UT_LOG_STEP("ut_kvp_open( pInstance, %s ) - Postive", KVP_VALID_TEST_NOT_VALID_YAML_FORMATTED_FILE);
status = ut_kvp_open( pInstance, KVP_VALID_TEST_NOT_VALID_YAML_FORMATTED_FILE);
UT_ASSERT( status == UT_KVP_STATUS_SUCCESS );
Expand Down