From f2291a8332a5f28f517cb2a6dc3039b8257c57b0 Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Tue, 18 Oct 2022 14:39:07 -0700 Subject: [PATCH 1/3] Fix file caching --- src/source/Signaling/FileCache.c | 20 +++-- tst/SignalingApiFunctionalityTest.cpp | 106 ++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/source/Signaling/FileCache.c b/src/source/Signaling/FileCache.c index 9414ae4d7f..8bf758e507 100644 --- a/src/source/Signaling/FileCache.c +++ b/src/source/Signaling/FileCache.c @@ -146,7 +146,7 @@ STATUS signalingCacheLoadFromFile(PCHAR channelName, PCHAR region, SIGNALING_CHA /* Assume channel name and region has been validated */ if (STRCMP(entries[i].channelName, channelName) == 0 && STRCMP(entries[i].region, region) == 0 && entries[i].role == role) { cacheFound = TRUE; - *pSignalingFileCacheEntry = entries[i]; + MEMCPY(pSignalingFileCacheEntry, &entries[i], sizeof(SignalingFileCacheEntry)); } } } @@ -172,7 +172,7 @@ STATUS signalingCacheSaveToFile(PSignalingFileCacheEntry pSignalingFileCacheEntr UINT32 entryCount = ARRAY_SIZE(entries), i, serializedCacheEntryLen; UINT64 fileSize = 0; PCHAR fileBuffer = NULL; - PSignalingFileCacheEntry pExistingCacheEntry = NULL; + BOOL newEntry = TRUE; CHAR serializedCacheEntry[MAX_SERIALIZED_SIGNALING_CACHE_ENTRY_LEN]; CHK(cacheFilePath != NULL && pSignalingFileCacheEntry != NULL, STATUS_NULL_ARG); @@ -199,20 +199,26 @@ STATUS signalingCacheSaveToFile(PSignalingFileCacheEntry pSignalingFileCacheEntr entryCount = 0; } - for (i = 0; pExistingCacheEntry == NULL && i < entryCount; ++i) { + for (i = 0; i < entryCount; ++i) { /* Assume channel name and region has been validated */ if (STRCMP(entries[i].channelName, pSignalingFileCacheEntry->channelName) == 0 && STRCMP(entries[i].region, pSignalingFileCacheEntry->region) == 0 && entries[i].role == pSignalingFileCacheEntry->role) { - pExistingCacheEntry = &entries[i]; + newEntry = FALSE; + break; } } /* at this point i is at most entryCount */ - CHK_WARN(entryCount < MAX_SIGNALING_CACHE_ENTRY_COUNT, STATUS_INVALID_OPERATION, - "Failed to store signaling cache because max entry count of %u reached", MAX_SIGNALING_CACHE_ENTRY_COUNT); + if (entryCount >= MAX_SIGNALING_CACHE_ENTRY_COUNT) { + DLOGW("Overwrote 32nd entry to store signaling cache because max entry count of %u reached", MAX_SIGNALING_CACHE_ENTRY_COUNT); + i = MAX_SIGNALING_CACHE_ENTRY_COUNT - 1; + newEntry = FALSE; + } entries[i] = *pSignalingFileCacheEntry; - entryCount++; + if (newEntry) { + entryCount++; + } for (i = 0; i < entryCount; ++i) { serializedCacheEntryLen = diff --git a/tst/SignalingApiFunctionalityTest.cpp b/tst/SignalingApiFunctionalityTest.cpp index c88e812e55..fd39af55b6 100644 --- a/tst/SignalingApiFunctionalityTest.cpp +++ b/tst/SignalingApiFunctionalityTest.cpp @@ -3397,6 +3397,112 @@ TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateCache) EXPECT_EQ(STATUS_SUCCESS, signalingCacheSaveToFile(&testEntry, DEFAULT_CACHE_FILE_PATH)); } +TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateMultiChannelCache) +{ + FREMOVE(DEFAULT_CACHE_FILE_PATH); + srand(GETTIME()); + + SignalingFileCacheEntry testEntry; + BOOL cacheFound = FALSE; + int additionalEntries = rand()%15 + 2; + char testWssEndpoint[MAX_SIGNALING_ENDPOINT_URI_LEN + 1] = {0}; + char testHttpsEndpoint[MAX_SIGNALING_ENDPOINT_URI_LEN + 1] = {0}; + char testRegion[MAX_REGION_NAME_LEN + 1] = {0}; + char testChannelArn[MAX_ARN_LEN + 1] = {0}; + char testChannel[MAX_CHANNEL_NAME_LEN + 1] = {0}; + int time = GETTIME() / HUNDREDS_OF_NANOS_IN_A_SECOND; + int append = 0; + int i = 0; + + for(i = 0; i < MAX_SIGNALING_CACHE_ENTRY_COUNT + additionalEntries; i++) { + testEntry.role = SIGNALING_CHANNEL_ROLE_TYPE_VIEWER; + MEMSET(testWssEndpoint, 0, MAX_SIGNALING_ENDPOINT_URI_LEN+1); + MEMSET(testHttpsEndpoint, 0, MAX_SIGNALING_ENDPOINT_URI_LEN+1); + MEMSET(testRegion, 0, MAX_REGION_NAME_LEN + 1); + MEMSET(testChannelArn, 0, MAX_ARN_LEN+1); + MEMSET(testChannel, 0, MAX_CHANNEL_NAME_LEN+1); + + append = rand()%5; + SPRINTF(testWssEndpoint, "%s%d", "testWssEndpoint", append); + SPRINTF(testHttpsEndpoint, "%s%d", "testHttpsEndpoint", append); + SPRINTF(testRegion, "%s%d", "testRegion", append); + SPRINTF(testChannelArn, "%s%d", "testChannelArn", append); + SPRINTF(testChannel, "%s%d", "testChannel", append); + + STRCPY(testEntry.wssEndpoint, testWssEndpoint); + STRCPY(testEntry.httpsEndpoint, testHttpsEndpoint); + STRCPY(testEntry.region, testRegion); + STRCPY(testEntry.channelArn, testChannelArn); + STRCPY(testEntry.channelName, testChannel); + testEntry.creationTsEpochSeconds = time + i; + EXPECT_EQ(STATUS_SUCCESS, signalingCacheSaveToFile(&testEntry, DEFAULT_CACHE_FILE_PATH)); + } + testEntry.creationTsEpochSeconds = 0; + EXPECT_EQ(STATUS_SUCCESS, signalingCacheLoadFromFile(testChannel, testRegion, SIGNALING_CHANNEL_ROLE_TYPE_VIEWER, &testEntry, &cacheFound, DEFAULT_CACHE_FILE_PATH)); + EXPECT_EQ(TRUE, cacheFound); + EXPECT_EQ(time + i - 1, testEntry.creationTsEpochSeconds); + EXPECT_EQ(0, STRCMP(testEntry.wssEndpoint, testWssEndpoint)); + EXPECT_EQ(0, STRCMP(testEntry.httpsEndpoint, testHttpsEndpoint)); + EXPECT_EQ(0, STRCMP(testEntry.region, testRegion)); + EXPECT_EQ(0, STRCMP(testEntry.channelArn, testChannelArn)); + EXPECT_EQ(0, STRCMP(testEntry.channelName, testChannel)); + + FREMOVE(DEFAULT_CACHE_FILE_PATH); +} + +TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateFullMultiChannelCache) +{ + FREMOVE(DEFAULT_CACHE_FILE_PATH); + srand(GETTIME()); + + SignalingFileCacheEntry testEntry; + BOOL cacheFound = FALSE; + int additionalEntries = rand()%15 + 2; + char testWssEndpoint[MAX_SIGNALING_ENDPOINT_URI_LEN + 1] = {0}; + char testHttpsEndpoint[MAX_SIGNALING_ENDPOINT_URI_LEN + 1] = {0}; + char testRegion[MAX_REGION_NAME_LEN + 1] = {0}; + char testChannelArn[MAX_ARN_LEN + 1] = {0}; + char testChannel[MAX_CHANNEL_NAME_LEN + 1] = {0}; + int time = GETTIME() / HUNDREDS_OF_NANOS_IN_A_SECOND; + int append = 0; + int i = 0; + + for(i = 0; i < MAX_SIGNALING_CACHE_ENTRY_COUNT + additionalEntries; i++) { + testEntry.role = SIGNALING_CHANNEL_ROLE_TYPE_VIEWER; + MEMSET(testWssEndpoint, 0, MAX_SIGNALING_ENDPOINT_URI_LEN+1); + MEMSET(testHttpsEndpoint, 0, MAX_SIGNALING_ENDPOINT_URI_LEN+1); + MEMSET(testRegion, 0, MAX_REGION_NAME_LEN + 1); + MEMSET(testChannelArn, 0, MAX_ARN_LEN+1); + MEMSET(testChannel, 0, MAX_CHANNEL_NAME_LEN+1); + + append = i; + SPRINTF(testWssEndpoint, "%s%d", "testWssEndpoint", append); + SPRINTF(testHttpsEndpoint, "%s%d", "testHttpsEndpoint", append); + SPRINTF(testRegion, "%s%d", "testRegion", append); + SPRINTF(testChannelArn, "%s%d", "testChannelArn", append); + SPRINTF(testChannel, "%s%d", "testChannel", append); + + STRCPY(testEntry.wssEndpoint, testWssEndpoint); + STRCPY(testEntry.httpsEndpoint, testHttpsEndpoint); + STRCPY(testEntry.region, testRegion); + STRCPY(testEntry.channelArn, testChannelArn); + STRCPY(testEntry.channelName, testChannel); + testEntry.creationTsEpochSeconds = time + i; + EXPECT_EQ(STATUS_SUCCESS, signalingCacheSaveToFile(&testEntry, DEFAULT_CACHE_FILE_PATH)); + } + testEntry.creationTsEpochSeconds = 0; + EXPECT_EQ(STATUS_SUCCESS, signalingCacheLoadFromFile(testChannel, testRegion, SIGNALING_CHANNEL_ROLE_TYPE_VIEWER, &testEntry, &cacheFound, DEFAULT_CACHE_FILE_PATH)); + EXPECT_EQ(TRUE, cacheFound); + EXPECT_EQ(time + i - 1, testEntry.creationTsEpochSeconds); + EXPECT_EQ(0, STRCMP(testEntry.wssEndpoint, testWssEndpoint)); + EXPECT_EQ(0, STRCMP(testEntry.httpsEndpoint, testHttpsEndpoint)); + EXPECT_EQ(0, STRCMP(testEntry.region, testRegion)); + EXPECT_EQ(0, STRCMP(testEntry.channelArn, testChannelArn)); + EXPECT_EQ(0, STRCMP(testEntry.channelName, testChannel)); + + //FREMOVE(DEFAULT_CACHE_FILE_PATH); +} + TEST_F(SignalingApiFunctionalityTest, receivingIceConfigOffer) { if (!mAccessKeyIdSet) { From d43765184a81d7c38bc8b7c6ec65cdd8f4ced9ba Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Thu, 20 Oct 2022 09:14:00 -0700 Subject: [PATCH 2/3] Update test to count entries, remove unnecessary memcpy --- src/source/Signaling/FileCache.c | 2 +- tst/SignalingApiFunctionalityTest.cpp | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/source/Signaling/FileCache.c b/src/source/Signaling/FileCache.c index 8bf758e507..08b43dee56 100644 --- a/src/source/Signaling/FileCache.c +++ b/src/source/Signaling/FileCache.c @@ -146,7 +146,7 @@ STATUS signalingCacheLoadFromFile(PCHAR channelName, PCHAR region, SIGNALING_CHA /* Assume channel name and region has been validated */ if (STRCMP(entries[i].channelName, channelName) == 0 && STRCMP(entries[i].region, region) == 0 && entries[i].role == role) { cacheFound = TRUE; - MEMCPY(pSignalingFileCacheEntry, &entries[i], sizeof(SignalingFileCacheEntry)); + *pSignalingFileCacheEntry = entries[i]; } } } diff --git a/tst/SignalingApiFunctionalityTest.cpp b/tst/SignalingApiFunctionalityTest.cpp index fd39af55b6..d3a1d19875 100644 --- a/tst/SignalingApiFunctionalityTest.cpp +++ b/tst/SignalingApiFunctionalityTest.cpp @@ -3413,6 +3413,11 @@ TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateMultiChannelCache) int time = GETTIME() / HUNDREDS_OF_NANOS_IN_A_SECOND; int append = 0; int i = 0; + char * fileBuffer; + UINT64 fileSize; + UINT32 entryCount; + SignalingFileCacheEntry entries[MAX_SIGNALING_CACHE_ENTRY_COUNT]; + const int TEST_CHANNEL_COUNT = 5; for(i = 0; i < MAX_SIGNALING_CACHE_ENTRY_COUNT + additionalEntries; i++) { testEntry.role = SIGNALING_CHANNEL_ROLE_TYPE_VIEWER; @@ -3422,7 +3427,7 @@ TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateMultiChannelCache) MEMSET(testChannelArn, 0, MAX_ARN_LEN+1); MEMSET(testChannel, 0, MAX_CHANNEL_NAME_LEN+1); - append = rand()%5; + append = rand()%TEST_CHANNEL_COUNT; SPRINTF(testWssEndpoint, "%s%d", "testWssEndpoint", append); SPRINTF(testHttpsEndpoint, "%s%d", "testHttpsEndpoint", append); SPRINTF(testRegion, "%s%d", "testRegion", append); @@ -3447,6 +3452,18 @@ TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateMultiChannelCache) EXPECT_EQ(0, STRCMP(testEntry.channelArn, testChannelArn)); EXPECT_EQ(0, STRCMP(testEntry.channelName, testChannel)); + //Check size to ensure entries are properly overwriting each other + EXPECT_EQ(STATUS_SUCCESS, readFile(DEFAULT_CACHE_FILE_PATH, FALSE, NULL, &fileSize)); + + EXPECT_LT(0, fileSize); + /* +1 for null terminator */ + fileBuffer = (char*)MEMCALLOC(1, (fileSize + 1) * SIZEOF(CHAR)); + EXPECT_EQ(STATUS_SUCCESS, readFile(DEFAULT_CACHE_FILE_PATH, FALSE, (PBYTE) fileBuffer, &fileSize)); + EXPECT_EQ(STATUS_SUCCESS, deserializeSignalingCacheEntries(fileBuffer, fileSize, entries, &entryCount, DEFAULT_CACHE_FILE_PATH)); + EXPECT_LT(entryCount, TEST_CHANNEL_COUNT+1); + + free(fileBuffer); + FREMOVE(DEFAULT_CACHE_FILE_PATH); } @@ -3500,7 +3517,7 @@ TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateFullMultiChannelCache) EXPECT_EQ(0, STRCMP(testEntry.channelArn, testChannelArn)); EXPECT_EQ(0, STRCMP(testEntry.channelName, testChannel)); - //FREMOVE(DEFAULT_CACHE_FILE_PATH); + FREMOVE(DEFAULT_CACHE_FILE_PATH); } TEST_F(SignalingApiFunctionalityTest, receivingIceConfigOffer) From 3b6db2a2ad5a2fe1dbdb68ec57cad76c48c0cf09 Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Thu, 20 Oct 2022 09:31:46 -0700 Subject: [PATCH 3/3] UT uses MEMFREE --- tst/SignalingApiFunctionalityTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tst/SignalingApiFunctionalityTest.cpp b/tst/SignalingApiFunctionalityTest.cpp index d3a1d19875..274bbe89ed 100644 --- a/tst/SignalingApiFunctionalityTest.cpp +++ b/tst/SignalingApiFunctionalityTest.cpp @@ -3458,11 +3458,12 @@ TEST_F(SignalingApiFunctionalityTest, fileCachingUpdateMultiChannelCache) EXPECT_LT(0, fileSize); /* +1 for null terminator */ fileBuffer = (char*)MEMCALLOC(1, (fileSize + 1) * SIZEOF(CHAR)); + EXPECT_TRUE((fileBuffer != NULL)); EXPECT_EQ(STATUS_SUCCESS, readFile(DEFAULT_CACHE_FILE_PATH, FALSE, (PBYTE) fileBuffer, &fileSize)); EXPECT_EQ(STATUS_SUCCESS, deserializeSignalingCacheEntries(fileBuffer, fileSize, entries, &entryCount, DEFAULT_CACHE_FILE_PATH)); EXPECT_LT(entryCount, TEST_CHANNEL_COUNT+1); - free(fileBuffer); + MEMFREE(fileBuffer); FREMOVE(DEFAULT_CACHE_FILE_PATH); }