From 2ad084e7ea71c5c07015132e311d02df1edc66e7 Mon Sep 17 00:00:00 2001 From: Ben51Degrees Date: Thu, 10 Dec 2020 14:16:17 +0000 Subject: [PATCH 1/3] TEST: Added tests for corrupted data handliing. --- ...iftyOne.DeviceDetection.Hash.Tests.vcxproj | 1 + ...DeviceDetection.Hash.Tests.vcxproj.filters | 3 + test/hash/EngineHashInitTests.cpp | 277 ++++++++++++++++++ 3 files changed, 281 insertions(+) create mode 100644 test/hash/EngineHashInitTests.cpp diff --git a/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj b/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj index feae1968..e5d81344 100644 --- a/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj +++ b/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj @@ -67,6 +67,7 @@ + diff --git a/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj.filters b/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj.filters index d2df2158..9645f0b1 100644 --- a/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj.filters +++ b/VisualStudio/FiftyOne.DeviceDetection.Hash.Tests/FiftyOne.DeviceDetection.Hash.Tests.vcxproj.filters @@ -61,6 +61,9 @@ Source Files + + Source Files + diff --git a/test/hash/EngineHashInitTests.cpp b/test/hash/EngineHashInitTests.cpp new file mode 100644 index 00000000..b23382da --- /dev/null +++ b/test/hash/EngineHashInitTests.cpp @@ -0,0 +1,277 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2019 51 Degrees Mobile Experts Limited, 5 Charlotte Close, + * Caversham, Reading, Berkshire, United Kingdom RG4 7BY. + * + * This Original Work is licensed under the European Union Public Licence (EUPL) + * v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +#include "../Constants.hpp" +#include "../EngineDeviceDetectionTests.hpp" +#include "../../src/hash/EngineHash.hpp" +#include +#include + +using namespace FiftyoneDegrees::Common; +using namespace FiftyoneDegrees::DeviceDetection; +using namespace FiftyoneDegrees::DeviceDetection::Hash; + +class EngineHashInitTests : public Base { +public: + const char* badVersionFileName = "badVersion.hash"; + const char* badDataFileName = "badData.hash"; + const char* smallDataFileName = "smallData.hash"; + const int32_t badVersion[4] = { 1, 2, 3, 4 }; + const int32_t goodVersion[4] = { 4, 1, 0, 0 }; + virtual void SetUp() { + Base::SetUp(); + writeTestFiles(); + } + virtual void TearDown() { + removeTestFiles(); + Base::TearDown(); + } + +private: + void writeTestFiles() { + void* nullHeader = + calloc(1, sizeof(fiftyoneDegreesDataSetHashHeader)); + + ofstream badVersionFile(badVersionFileName, ios::out | ios::binary); + if (badVersionFile.is_open()) { + badVersionFile.write((char*)badVersion, sizeof(int32_t) * 4); + badVersionFile.write((char*)nullHeader, sizeof(fiftyoneDegreesDataSetHashHeader)); + badVersionFile.close(); + } + else { + cout << "Unable to open file"; + } + + ofstream badDataFile(badDataFileName, ios::out | ios::binary); + if (badDataFile.is_open()) { + badDataFile.write((char*)goodVersion, sizeof(int32_t) * 4); + badDataFile.write((char*)nullHeader, sizeof(fiftyoneDegreesDataSetHashHeader)); + badDataFile.close(); + } + else { + cout << "Unable to open file"; + } + + ofstream smallDataFile(smallDataFileName, ios::out | ios::binary); + if (smallDataFile.is_open()) { + smallDataFile.write((char*)nullHeader, sizeof(byte)); + smallDataFile.close(); + } + else { + cout << "Unable to open file"; + } + free(nullHeader); + } + void removeTestFiles() { + if (remove(badVersionFileName) != 0) { + cout << "Error deleting file"; + } + if (remove(badDataFileName) != 0) { + cout << "Error deleting file"; + } + if (remove(smallDataFileName) != 0) { + cout << "Error deleting file"; + } + } +}; + +/** + * Check that when initializing from a file which has the wrong version, + * the correct error is thrown, and memory is cleaned up. + */ +TEST_F(EngineHashInitTests, WrongDataVersion_File) { + ConfigHash config; + RequiredPropertiesConfig properties; + try { + EngineHash* engine = new EngineHash(badVersionFileName, &config, &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception &e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_INCORRECT_VERSION, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + + } +} + +/** + * Check that when initializing from a file which has the correct version + * but corrupted data, the correct error is thrown, and memory is cleaned up. + */ +TEST_F(EngineHashInitTests, BadData_File) { + ConfigHash config; + RequiredPropertiesConfig properties; + try { + EngineHash* engine = new EngineHash(badDataFileName, &config, &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + + } +} + +/** + * Check that when initializing from a file which is too small and does not + * contain enough data to fill the header, the correct error is thrown, + * and memory is cleaned up. + */ +TEST_F(EngineHashInitTests, SmallData_File) { + ConfigHash config; + RequiredPropertiesConfig properties; + try { + EngineHash* engine = new EngineHash(smallDataFileName, &config, &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + + } +} + +/** + * Check that when initializing from memory which has the wrong version, + * the correct error is thrown, and memory is cleaned up. + */ +TEST_F(EngineHashInitTests, WrongDataVersion_Memory) { + ConfigHash config; + RequiredPropertiesConfig properties; + void* mem = fiftyoneDegreesMalloc(sizeof(fiftyoneDegreesDataSetHashHeader)); + ifstream file(badVersionFileName, ios::out | ios::binary); + if (file.is_open()) { + file.read((char*)mem, sizeof(fiftyoneDegreesDataSetHashHeader)); + file.close(); + } + else { + cout << "Unable to open file"; + } + + try { + EngineHash* engine = new EngineHash( + mem, + (long)sizeof(fiftyoneDegreesDataSetHashHeader), + &config, + &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_INCORRECT_VERSION, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + } + fiftyoneDegreesFree(mem); +} + +/** + * Check that when initializing from memory which has the correct version + * but corrupted data, the correct error is thrown, and memory is cleaned up. + */ +TEST_F(EngineHashInitTests, BadData_Memory) { + ConfigHash config; + RequiredPropertiesConfig properties; + void* mem = fiftyoneDegreesMalloc(sizeof(fiftyoneDegreesDataSetHashHeader)); + ifstream file(badDataFileName, ios::out | ios::binary); + if (file.is_open()) { + file.read((char*)mem, sizeof(fiftyoneDegreesDataSetHashHeader)); + file.close(); + } + else { + cout << "Unable to open file"; + } + + try { + EngineHash* engine = new EngineHash( + mem, + (long)sizeof(fiftyoneDegreesDataSetHashHeader), + &config, + &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + } + fiftyoneDegreesFree(mem); +} + +/** + * Check that when initializing from memory which is too small and does not + * contain enough data to fill the header, the correct error is thrown, + * and memory is cleaned up. + */ +TEST_F(EngineHashInitTests, SmallData_Memory) { + ConfigHash config; + RequiredPropertiesConfig properties; + void* mem = fiftyoneDegreesMalloc(sizeof(fiftyoneDegreesDataSetHashHeader)); + ifstream file(smallDataFileName, ios::out | ios::binary); + if (file.is_open()) { + file.read((char*)mem, sizeof(fiftyoneDegreesDataSetHashHeader)); + file.close(); + } + else { + cout << "Unable to open file"; + } + + try { + EngineHash* engine = new EngineHash( + mem, + (long)sizeof(fiftyoneDegreesDataSetHashHeader), + &config, + &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + + } +} \ No newline at end of file From 01f56a8a8ce220d62646ac733052a1eb18cfbb28 Mon Sep 17 00:00:00 2001 From: Ben51Degrees Date: Wed, 30 Dec 2020 13:21:15 +0000 Subject: [PATCH 2/3] TEST: Updated tests for initialisation with bad data. This also includes fixes for scenarios which were not handled correctly. Including corrupt data which results in a failed collection, this now returns `CORRUPT_DATA`. If construction from memory fails, then the copied data is now cleaned up by the C++ layer. --- src/common-cxx | 2 +- src/hash/EngineHash.cpp | 1 + src/hash/hash.c | 4 +- test/hash/EngineHashInitTests.cpp | 177 ++++++++++++++++++------------ 4 files changed, 110 insertions(+), 74 deletions(-) diff --git a/src/common-cxx b/src/common-cxx index f6d1aa94..a7d32f59 160000 --- a/src/common-cxx +++ b/src/common-cxx @@ -1 +1 @@ -Subproject commit f6d1aa94618b40f532445e0e4f8653bfbef28456 +Subproject commit a7d32f59b096212768a0d3a877bf380f99f87e26 diff --git a/src/hash/EngineHash.cpp b/src/hash/EngineHash.cpp index 3375e672..61606afe 100644 --- a/src/hash/EngineHash.cpp +++ b/src/hash/EngineHash.cpp @@ -74,6 +74,7 @@ EngineHash::EngineHash( (size_t)length, exception); if (status != SUCCESS) { + Free(dataCopy); throw StatusCodeException(status); } EXCEPTION_THROW; diff --git a/src/hash/hash.c b/src/hash/hash.c index 317f9983..18549d96 100644 --- a/src/hash/hash.c +++ b/src/hash/hash.c @@ -100,7 +100,7 @@ dataSet->t = CollectionCreateFromMemory( \ reader, \ dataSet->header.t); \ if (dataSet->t == NULL) { \ - return INVALID_COLLECTION_CONFIG; \ + return CORRUPT_DATA; \ } #define COLLECTION_CREATE_FILE(t,f) \ @@ -111,7 +111,7 @@ dataSet->t = CollectionCreateFromFile( \ dataSet->header.t, \ f); \ if (dataSet->t == NULL) { \ - return INVALID_COLLECTION_CONFIG; \ + return CORRUPT_DATA; \ } /** diff --git a/test/hash/EngineHashInitTests.cpp b/test/hash/EngineHashInitTests.cpp index b23382da..fb71d062 100644 --- a/test/hash/EngineHashInitTests.cpp +++ b/test/hash/EngineHashInitTests.cpp @@ -46,15 +46,84 @@ class EngineHashInitTests : public Base { Base::TearDown(); } + /** + * Check that when initializing from a file which has the wrong version, + * the correct error is thrown, and memory is cleaned up. + */ + void wrongDataVersionFile(ConfigHash* config) { + RequiredPropertiesConfig properties; + try { + EngineHash* engine = new EngineHash(badVersionFileName, config, &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_INCORRECT_VERSION, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + + } + } + + /** + * Check that when initializing from a file which has the correct version + * but corrupted data, the correct error is thrown, and memory is cleaned up. + */ + void badDataFile(ConfigHash* config) { + RequiredPropertiesConfig properties; + try { + EngineHash* engine = new EngineHash(badDataFileName, config, &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + } + } + + /** + * Check that when initializing from a file which is too small and does not + * contain enough data to fill the header, the correct error is thrown, + * and memory is cleaned up. + */ + void smallDataFile(ConfigHash *config) { + RequiredPropertiesConfig properties; + try { + EngineHash* engine = new EngineHash(smallDataFileName, config, &properties); + FAIL() << L"No exception was thrown"; + } + catch (exception & e) { + const char* expected = fiftyoneDegreesStatusGetMessage( + FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, + NULL); + ASSERT_STREQ( + e.what(), + expected); + fiftyoneDegreesFree((void*)expected); + + } + } + private: void writeTestFiles() { - void* nullHeader = - calloc(1, sizeof(fiftyoneDegreesDataSetHashHeader)); + void* garbledHeader = + malloc(sizeof(fiftyoneDegreesDataSetHashHeader)); + for (int i = 0; i < sizeof(fiftyoneDegreesDataSetHashHeader); i++) { + ((byte*)garbledHeader)[i] = 12; + } ofstream badVersionFile(badVersionFileName, ios::out | ios::binary); if (badVersionFile.is_open()) { badVersionFile.write((char*)badVersion, sizeof(int32_t) * 4); - badVersionFile.write((char*)nullHeader, sizeof(fiftyoneDegreesDataSetHashHeader)); + badVersionFile.write((char*)garbledHeader, sizeof(fiftyoneDegreesDataSetHashHeader)); badVersionFile.close(); } else { @@ -64,7 +133,7 @@ class EngineHashInitTests : public Base { ofstream badDataFile(badDataFileName, ios::out | ios::binary); if (badDataFile.is_open()) { badDataFile.write((char*)goodVersion, sizeof(int32_t) * 4); - badDataFile.write((char*)nullHeader, sizeof(fiftyoneDegreesDataSetHashHeader)); + badDataFile.write((char*)garbledHeader, sizeof(fiftyoneDegreesDataSetHashHeader)); badDataFile.close(); } else { @@ -73,13 +142,13 @@ class EngineHashInitTests : public Base { ofstream smallDataFile(smallDataFileName, ios::out | ios::binary); if (smallDataFile.is_open()) { - smallDataFile.write((char*)nullHeader, sizeof(byte)); + smallDataFile.write((char*)garbledHeader, sizeof(byte)); smallDataFile.close(); } else { cout << "Unable to open file"; } - free(nullHeader); + free(garbledHeader); } void removeTestFiles() { if (remove(badVersionFileName) != 0) { @@ -94,76 +163,42 @@ class EngineHashInitTests : public Base { } }; -/** - * Check that when initializing from a file which has the wrong version, - * the correct error is thrown, and memory is cleaned up. - */ -TEST_F(EngineHashInitTests, WrongDataVersion_File) { - ConfigHash config; - RequiredPropertiesConfig properties; - try { - EngineHash* engine = new EngineHash(badVersionFileName, &config, &properties); - FAIL() << L"No exception was thrown"; - } - catch (exception &e) { - const char* expected = fiftyoneDegreesStatusGetMessage( - FIFTYONE_DEGREES_STATUS_INCORRECT_VERSION, - NULL); - ASSERT_STREQ( - e.what(), - expected); - fiftyoneDegreesFree((void*)expected); - - } +#define TEST_WRONG_VERSION_FILE(c) \ +TEST_F(EngineHashInitTests,WrongDataVersion_File_##c) { \ + ConfigHash config; \ + config.set##c##(); \ + wrongDataVersionFile(&config); \ } -/** - * Check that when initializing from a file which has the correct version - * but corrupted data, the correct error is thrown, and memory is cleaned up. - */ -TEST_F(EngineHashInitTests, BadData_File) { - ConfigHash config; - RequiredPropertiesConfig properties; - try { - EngineHash* engine = new EngineHash(badDataFileName, &config, &properties); - FAIL() << L"No exception was thrown"; - } - catch (exception & e) { - const char* expected = fiftyoneDegreesStatusGetMessage( - FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, - NULL); - ASSERT_STREQ( - e.what(), - expected); - fiftyoneDegreesFree((void*)expected); +TEST_WRONG_VERSION_FILE(HighPerformance); +TEST_WRONG_VERSION_FILE(LowMemory); +TEST_WRONG_VERSION_FILE(Balanced); +TEST_WRONG_VERSION_FILE(BalancedTemp); - } +#define TEST_BAD_DATA_FILE(c) \ +TEST_F(EngineHashInitTests,BadData_File_##c) { \ + ConfigHash config; \ + config.set##c##(); \ + badDataFile(&config); \ } -/** - * Check that when initializing from a file which is too small and does not - * contain enough data to fill the header, the correct error is thrown, - * and memory is cleaned up. - */ -TEST_F(EngineHashInitTests, SmallData_File) { - ConfigHash config; - RequiredPropertiesConfig properties; - try { - EngineHash* engine = new EngineHash(smallDataFileName, &config, &properties); - FAIL() << L"No exception was thrown"; - } - catch (exception & e) { - const char* expected = fiftyoneDegreesStatusGetMessage( - FIFTYONE_DEGREES_STATUS_CORRUPT_DATA, - NULL); - ASSERT_STREQ( - e.what(), - expected); - fiftyoneDegreesFree((void*)expected); +TEST_BAD_DATA_FILE(HighPerformance); +TEST_BAD_DATA_FILE(LowMemory); +TEST_BAD_DATA_FILE(Balanced); +TEST_BAD_DATA_FILE(BalancedTemp); - } +#define TEST_SMALL_DATA_FILE(c) \ +TEST_F(EngineHashInitTests,SmallData_File_##c) { \ + ConfigHash config; \ + config.set##c##(); \ + smallDataFile(&config); \ } +TEST_SMALL_DATA_FILE(HighPerformance); +TEST_SMALL_DATA_FILE(LowMemory); +TEST_SMALL_DATA_FILE(Balanced); +TEST_SMALL_DATA_FILE(BalancedTemp); + /** * Check that when initializing from memory which has the wrong version, * the correct error is thrown, and memory is cleaned up. @@ -259,7 +294,7 @@ TEST_F(EngineHashInitTests, SmallData_Memory) { try { EngineHash* engine = new EngineHash( mem, - (long)sizeof(fiftyoneDegreesDataSetHashHeader), + (long)sizeof(byte), &config, &properties); FAIL() << L"No exception was thrown"; @@ -272,6 +307,6 @@ TEST_F(EngineHashInitTests, SmallData_Memory) { e.what(), expected); fiftyoneDegreesFree((void*)expected); - } -} \ No newline at end of file + fiftyoneDegreesFree(mem); +} From 7ae557aa3da22146bc0d4c6a07f33264fdc1abcc Mon Sep 17 00:00:00 2001 From: Ben51Degrees Date: Thu, 31 Dec 2020 09:41:35 +0000 Subject: [PATCH 3/3] BUG: Macro pasting was not handled correctly in GCC. --- test/hash/EngineHashInitTests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/hash/EngineHashInitTests.cpp b/test/hash/EngineHashInitTests.cpp index fb71d062..21815698 100644 --- a/test/hash/EngineHashInitTests.cpp +++ b/test/hash/EngineHashInitTests.cpp @@ -166,7 +166,7 @@ class EngineHashInitTests : public Base { #define TEST_WRONG_VERSION_FILE(c) \ TEST_F(EngineHashInitTests,WrongDataVersion_File_##c) { \ ConfigHash config; \ - config.set##c##(); \ + config.set##c(); \ wrongDataVersionFile(&config); \ } @@ -178,7 +178,7 @@ TEST_WRONG_VERSION_FILE(BalancedTemp); #define TEST_BAD_DATA_FILE(c) \ TEST_F(EngineHashInitTests,BadData_File_##c) { \ ConfigHash config; \ - config.set##c##(); \ + config.set##c(); \ badDataFile(&config); \ } @@ -190,7 +190,7 @@ TEST_BAD_DATA_FILE(BalancedTemp); #define TEST_SMALL_DATA_FILE(c) \ TEST_F(EngineHashInitTests,SmallData_File_##c) { \ ConfigHash config; \ - config.set##c##(); \ + config.set##c(); \ smallDataFile(&config); \ }