diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 311fce2..e57a80f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -82,7 +82,14 @@ jobs: cmake --build build test: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + + name: Test on ${{ matrix.os }} steps: - name: Checkout code @@ -91,13 +98,35 @@ jobs: repository: ${{ github.event.pull_request.head.repo.full_name || 'Eppo-exp/cpp-sdk' }} ref: ${{ env.SDK_BRANCH_NAME }} - - name: Set up build environment + - name: Set up build environment (Ubuntu) + if: runner.os == 'Linux' run: | sudo apt-get update sudo apt-get install -y build-essential g++ cmake pkg-config libre2-dev + - name: Set up build environment (macOS) + if: runner.os == 'macOS' + run: | + brew install re2 + brew install cmake + + - name: Set up MSVC (Windows) + if: runner.os == 'Windows' + uses: ilammy/msvc-dev-cmd@v1 + + - name: Install re2 via vcpkg (Windows) + if: runner.os == 'Windows' + run: | + # Use the standalone C:/vcpkg (bash on Windows accepts forward slashes) + /c/vcpkg/vcpkg install re2:x64-windows + echo "CMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake" >> $GITHUB_ENV + shell: bash + - name: Run tests - run: make test TEST_DATA_BRANCH=${{env.TEST_DATA_BRANCH_NAME}} + shell: bash + run: | + echo "CMAKE_TOOLCHAIN_FILE is: $CMAKE_TOOLCHAIN_FILE" + make test TEST_DATA_BRANCH=${{env.TEST_DATA_BRANCH_NAME}} memory-test: runs-on: ubuntu-latest diff --git a/CMakeLists.txt b/CMakeLists.txt index a4a4b27..505e3df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,12 +46,21 @@ set(CMAKE_C_STANDARD 99) set(CMAKE_C_STANDARD_REQUIRED ON) # Find RE2 library (required for safe regex handling without exceptions) -# Try CONFIG mode first (for modern RE2 installations) +# Try CONFIG mode first (for modern RE2 installations with CMake support) find_package(re2 CONFIG) if(NOT re2_FOUND) - # Fall back to PkgConfig for system installations - find_package(PkgConfig REQUIRED) - pkg_check_modules(RE2 REQUIRED re2) + # Fall back to PkgConfig for system installations (Linux/macOS) + find_package(PkgConfig) + if(PkgConfig_FOUND) + pkg_check_modules(RE2 REQUIRED re2) + else() + message(FATAL_ERROR "Could not find RE2: CMake config not found and pkg-config not available.\n" + "Please install RE2 with CMake support or ensure pkg-config is installed:\n" + " - On Windows: Use vcpkg and set CMAKE_TOOLCHAIN_FILE environment variable\n" + " vcpkg install re2:x64-windows\n" + " - On Ubuntu/Debian: sudo apt-get install libre2-dev pkg-config\n" + " - On macOS: brew install re2 pkg-config") + endif() endif() # Collect source files @@ -64,10 +73,22 @@ set(THIRD_PARTY_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/third_party/md5_wrapper.c" ) +# Create a separate library for third-party code to avoid applying our warning flags +add_library(eppoclient_third_party OBJECT ${THIRD_PARTY_SOURCES}) +target_include_directories(eppoclient_third_party PUBLIC + $ +) +# Disable warnings for third-party code +if(MSVC) + target_compile_options(eppoclient_third_party PRIVATE /W0) +else() + target_compile_options(eppoclient_third_party PRIVATE -w) +endif() + # Create the library add_library(eppoclient STATIC ${EPPOCLIENT_SOURCES} - ${THIRD_PARTY_SOURCES} + $ ) # Link RE2 library @@ -223,6 +244,8 @@ if(EPPOCLIENT_BUILD_TESTS) # Re-enable exceptions for test code (Catch2 requires them) if(MSVC) target_compile_options(test_runner PRIVATE /EHsc /U_HAS_EXCEPTIONS) + # Disable CRT security warnings for test code (gmtime, etc.) + target_compile_definitions(test_runner PRIVATE _CRT_SECURE_NO_WARNINGS) else() target_compile_options(test_runner PRIVATE -fexceptions) endif() diff --git a/Makefile b/Makefile index 245baff..7e7c621 100644 --- a/Makefile +++ b/Makefile @@ -3,17 +3,28 @@ BUILD_DIR = build # Test data branch (override with: make test TEST_DATA_BRANCH=my-branch) TEST_DATA_BRANCH ?= main +# CMake toolchain file (for vcpkg on Windows, set via environment variable) +CMAKE_TOOLCHAIN_FILE ?= + +# If CMAKE_TOOLCHAIN_FILE is set, add it to cmake args with proper quoting +ifneq ($(CMAKE_TOOLCHAIN_FILE),) + CMAKE_TOOLCHAIN_ARG := -DCMAKE_TOOLCHAIN_FILE="$(CMAKE_TOOLCHAIN_FILE)" +else + CMAKE_TOOLCHAIN_ARG := +endif + # Default target - comprehensive development build (library + tests + examples) .PHONY: all all: @mkdir -p $(BUILD_DIR) @cd $(BUILD_DIR) && cmake .. \ + $(CMAKE_TOOLCHAIN_ARG) \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=ON \ -DEPPOCLIENT_BUILD_EXAMPLES=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) --config Debug @# Copy compile_commands.json to root for IDE support @if [ -f $(BUILD_DIR)/compile_commands.json ]; then \ cp $(BUILD_DIR)/compile_commands.json .; \ @@ -29,10 +40,11 @@ all: build: @mkdir -p $(BUILD_DIR) @cd $(BUILD_DIR) && cmake .. \ + $(CMAKE_TOOLCHAIN_ARG) \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=OFF \ -DEPPOCLIENT_ERR_ON_WARNINGS=OFF - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) --config Release @# Copy compile_commands.json to root for IDE support @if [ -f $(BUILD_DIR)/compile_commands.json ]; then \ cp $(BUILD_DIR)/compile_commands.json .; \ @@ -44,28 +56,30 @@ build: test: @mkdir -p $(BUILD_DIR) @cd $(BUILD_DIR) && cmake .. \ + $(CMAKE_TOOLCHAIN_ARG) \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) --config Debug @# Copy compile_commands.json to root for IDE support @if [ -f $(BUILD_DIR)/compile_commands.json ]; then \ cp $(BUILD_DIR)/compile_commands.json .; \ echo "Updated compile_commands.json for IDE support"; \ fi @echo "Running tests..." - @cd $(BUILD_DIR) && ctest -V + @cd $(BUILD_DIR) && ctest -V -C Debug # Build all examples .PHONY: examples examples: @mkdir -p $(BUILD_DIR) @cd $(BUILD_DIR) && cmake .. \ + $(CMAKE_TOOLCHAIN_ARG) \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_EXAMPLES=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=OFF - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) --config Release @echo "Examples built in $(BUILD_DIR)/" @echo "" @echo "Run examples with:" @@ -101,13 +115,14 @@ clean: test-memory: @mkdir -p $(BUILD_DIR) @cd $(BUILD_DIR) && cmake .. \ + $(CMAKE_TOOLCHAIN_ARG) \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) \ -DCMAKE_CXX_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g -O0" \ -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g -O0" - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) --config Debug @echo "Running tests with AddressSanitizer and UndefinedBehaviorSanitizer..." @env ASAN_OPTIONS=halt_on_error=1:strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:print_stats=1 \ UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 \ @@ -118,12 +133,13 @@ test-memory: test-eval-performance: @mkdir -p $(BUILD_DIR) @cd $(BUILD_DIR) && cmake .. \ + $(CMAKE_TOOLCHAIN_ARG) \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) \ -DCMAKE_BUILD_TYPE=RelWithDebInfo - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) --config RelWithDebInfo @echo "Running performance tests..." @./build/test_runner "[performance]" diff --git a/README.md b/README.md index 61e971a..8ab5c95 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ See [ARCHITECTURE.md](ARCHITECTURE.md) for detailed architecture support and cro - CMake 3.14+ (if using CMake) - Make (if using Make) - RE2 - Google's safe regex library +- 64-bit architecture (x86_64, aarch64/ARM64) #### Installing RE2 @@ -114,6 +115,8 @@ sudo make install Other dependencies (nlohmann/json, semver, etc.) are vendored and require no installation. +**Note:** Only 64-bit architectures are supported. 32-bit (x86) builds are not supported. + ## Quick Start ### 1. Initialize the SDK diff --git a/src/bandit_model.cpp b/src/bandit_model.cpp index dd0b0bb..7604aac 100644 --- a/src/bandit_model.cpp +++ b/src/bandit_model.cpp @@ -178,7 +178,10 @@ std::optional parseBanditConfiguration(const nlohmann::json bc.modelData = *modelData; if (j.contains("updatedAt") && j["updatedAt"].is_string()) { - bc.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref()); + bc.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref(), error); + if (!error.empty()) { + error = "BanditConfiguration: Invalid updatedAt: " + error; + } } return bc; @@ -279,7 +282,12 @@ ParseResult parseBanditResponse(const nlohmann::json& j) { if (!j["updatedAt"].is_string()) { result.errors.push_back("BanditResponse: 'updatedAt' field must be a string"); } else { - br.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref()); + std::string error; + br.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref(), error); + // TODO: log error + // if (!error.empty()) { + // logger.error("BanditResponse: Invalid updatedAt: " + error); + // } } } diff --git a/src/config_response.cpp b/src/config_response.cpp index 1052874..0fd4fd9 100644 --- a/src/config_response.cpp +++ b/src/config_response.cpp @@ -363,12 +363,18 @@ std::optional parseAllocation(const nlohmann::json& j, std::string& // Parse optional timestamp fields using getOptionalField auto startAt = getOptionalField(j, "startAt"); if (startAt) { - a.startAt = parseISOTimestamp(*startAt); + a.startAt = parseISOTimestamp(*startAt, error); + if (!error.empty()) { + return std::nullopt; + } } auto endAt = getOptionalField(j, "endAt"); if (endAt) { - a.endAt = parseISOTimestamp(*endAt); + a.endAt = parseISOTimestamp(*endAt, error); + if (!error.empty()) { + return std::nullopt; + } } auto doLog = getOptionalField(j, "doLog"); diff --git a/src/json_utils.hpp b/src/json_utils.hpp index 5fa8d68..29de485 100644 --- a/src/json_utils.hpp +++ b/src/json_utils.hpp @@ -64,9 +64,11 @@ std::optional extractTypedValue(nlohmann::json::const_iterator it) { if (!it->is_boolean()) return std::nullopt; return it->template get_ref(); + } else { + // This should never be reached for supported types + static_assert(false, "Unsupported type for extractTypedValue"); + return std::nullopt; } - - return std::nullopt; } // Helper to get an optional field with type validation diff --git a/src/time_utils.cpp b/src/time_utils.cpp index 5c19ba4..33a29a3 100644 --- a/src/time_utils.cpp +++ b/src/time_utils.cpp @@ -5,7 +5,8 @@ namespace eppoclient { -std::chrono::system_clock::time_point parseISOTimestamp(const std::string& timestamp) { +std::chrono::system_clock::time_point parseISOTimestamp(const std::string& timestamp, + std::string& error) { std::tm tm = {}; char dot = '\0'; int milliseconds = 0; @@ -17,6 +18,7 @@ std::chrono::system_clock::time_point parseISOTimestamp(const std::string& times ss >> std::get_time(&tm, "%Y-%m-%dT%H:%M:%S"); if (ss.fail()) { + error = "Invalid timestamp: " + timestamp; return std::chrono::system_clock::time_point(); } @@ -52,7 +54,32 @@ std::chrono::system_clock::time_point parseISOTimestamp(const std::string& times #endif if (time_c == -1) { - return std::chrono::system_clock::time_point(); + // Validate that we have at least 4 characters and they're all digits + if (timestamp.length() < 4) { + error = "Invalid timestamp: " + timestamp; + return std::chrono::system_clock::time_point(); + } + + std::string yearStr = timestamp.substr(0, 4); + for (char c : yearStr) { + if (!std::isdigit(static_cast(c))) { + error = "Invalid timestamp: " + timestamp; + return std::chrono::system_clock::time_point(); + } + } + + // Validate that the 5th character is a dash (if it exists) + if (timestamp.length() > 4 && timestamp[4] != '-') { + error = "Invalid timestamp: " + timestamp; + return std::chrono::system_clock::time_point(); + } + + int year = std::stoi(yearStr); + if (year < 1970) { + return std::chrono::system_clock::time_point::min(); + } else { + return std::chrono::system_clock::time_point::max(); + } } auto tp = std::chrono::system_clock::from_time_t(time_c); tp += std::chrono::milliseconds(milliseconds); @@ -61,7 +88,12 @@ std::chrono::system_clock::time_point parseISOTimestamp(const std::string& times std::string formatISOTimestamp(const std::chrono::system_clock::time_point& tp) { auto tt = std::chrono::system_clock::to_time_t(tp); - std::tm tm = *std::gmtime(&tt); + std::tm tm = {}; +#ifdef _WIN32 + gmtime_s(&tm, &tt); +#else + tm = *std::gmtime(&tt); +#endif // Add milliseconds auto ms = std::chrono::duration_cast(tp.time_since_epoch()) % 1000; diff --git a/src/time_utils.hpp b/src/time_utils.hpp index c069acf..a3aac1f 100644 --- a/src/time_utils.hpp +++ b/src/time_utils.hpp @@ -12,11 +12,14 @@ namespace eppoclient { * Supports ISO8601 timestamps with optional millisecond precision. * Examples: "2024-06-09T14:23:11", "2024-06-09T14:23:11.123" * + * Handles edge cases including date underflow and overflow scenarios. + * * @param timestamp The ISO 8601 formatted timestamp string - * @return A time_point representing the parsed timestamp, or a default-constructed - * time_point if parsing fails + * @param error Output parameter that will contain an error message if parsing fails + * @return A time_point representing the parsed timestamp */ -std::chrono::system_clock::time_point parseISOTimestamp(const std::string& timestamp); +std::chrono::system_clock::time_point parseISOTimestamp(const std::string& timestamp, + std::string& error); /** * Format a system_clock::time_point into an ISO 8601 timestamp string. diff --git a/test/test_time_utils.cpp b/test/test_time_utils.cpp index b577d9c..6cbbfe3 100644 --- a/test/test_time_utils.cpp +++ b/test/test_time_utils.cpp @@ -7,7 +7,8 @@ using namespace eppoclient; TEST_CASE("parseISOTimestamp - valid timestamp without milliseconds", "[time_utils]") { std::string timestamp = "2024-06-09T14:23:11"; - auto result = parseISOTimestamp(timestamp); + std::string error; + auto result = parseISOTimestamp(timestamp, error); // Convert back to time_t to check the values auto time_t_result = std::chrono::system_clock::to_time_t(result); @@ -19,11 +20,13 @@ TEST_CASE("parseISOTimestamp - valid timestamp without milliseconds", "[time_uti REQUIRE(tm->tm_hour == 14); REQUIRE(tm->tm_min == 23); REQUIRE(tm->tm_sec == 11); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp - valid timestamp with milliseconds", "[time_utils]") { std::string timestamp = "2024-06-09T14:23:11.123"; - auto result = parseISOTimestamp(timestamp); + std::string error; + auto result = parseISOTimestamp(timestamp, error); // Convert back to time_t to check the values auto time_t_result = std::chrono::system_clock::to_time_t(result); @@ -41,45 +44,53 @@ TEST_CASE("parseISOTimestamp - valid timestamp with milliseconds", "[time_utils] auto milliseconds = std::chrono::duration_cast(result.time_since_epoch()) - seconds; REQUIRE(milliseconds.count() == 123); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp - timestamp with single digit milliseconds", "[time_utils]") { std::string timestamp = "2024-06-09T14:23:11.5"; - auto result = parseISOTimestamp(timestamp); + std::string error; + auto result = parseISOTimestamp(timestamp, error); // Should pad to 500 milliseconds auto seconds = std::chrono::duration_cast(result.time_since_epoch()); auto milliseconds = std::chrono::duration_cast(result.time_since_epoch()) - seconds; REQUIRE(milliseconds.count() == 500); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp - timestamp with two digit milliseconds", "[time_utils]") { std::string timestamp = "2024-06-09T14:23:11.45"; - auto result = parseISOTimestamp(timestamp); + std::string error; + auto result = parseISOTimestamp(timestamp, error); // Should pad to 450 milliseconds auto seconds = std::chrono::duration_cast(result.time_since_epoch()); auto milliseconds = std::chrono::duration_cast(result.time_since_epoch()) - seconds; REQUIRE(milliseconds.count() == 450); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp - timestamp with microseconds (ignores extra precision)", "[time_utils]") { std::string timestamp = "2024-06-09T14:23:11.123456"; - auto result = parseISOTimestamp(timestamp); + std::string error; + auto result = parseISOTimestamp(timestamp, error); // Should only use first 3 digits (123 milliseconds) auto seconds = std::chrono::duration_cast(result.time_since_epoch()); auto milliseconds = std::chrono::duration_cast(result.time_since_epoch()) - seconds; REQUIRE(milliseconds.count() == 123); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp - timestamp with Z suffix", "[time_utils]") { std::string timestamp = "2024-06-09T14:23:11Z"; - auto result = parseISOTimestamp(timestamp); + std::string error; + auto result = parseISOTimestamp(timestamp, error); // Convert back to time_t to check the values auto time_t_result = std::chrono::system_clock::to_time_t(result); @@ -91,30 +102,37 @@ TEST_CASE("parseISOTimestamp - timestamp with Z suffix", "[time_utils]") { REQUIRE(tm->tm_hour == 14); REQUIRE(tm->tm_min == 23); REQUIRE(tm->tm_sec == 11); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp - invalid timestamp returns default time_point", "[time_utils]") { std::string invalidTimestamp = "not-a-timestamp"; - auto result = parseISOTimestamp(invalidTimestamp); + std::string error; + auto result = parseISOTimestamp(invalidTimestamp, error); // Should return default-constructed time_point REQUIRE(result == std::chrono::system_clock::time_point()); + REQUIRE(!error.empty()); } TEST_CASE("parseISOTimestamp - empty string returns default time_point", "[time_utils]") { std::string emptyTimestamp = ""; - auto result = parseISOTimestamp(emptyTimestamp); + std::string error; + auto result = parseISOTimestamp(emptyTimestamp, error); // Should return default-constructed time_point REQUIRE(result == std::chrono::system_clock::time_point()); + REQUIRE(!error.empty()); } TEST_CASE("parseISOTimestamp - malformed date returns default time_point", "[time_utils]") { std::string malformedTimestamp = "2024-13-40T25:99:99"; - auto result = parseISOTimestamp(malformedTimestamp); + std::string error; + auto result = parseISOTimestamp(malformedTimestamp, error); // Should return default-constructed time_point REQUIRE(result == std::chrono::system_clock::time_point()); + REQUIRE(!error.empty()); } TEST_CASE("formatISOTimestamp - formats time_point to ISO 8601 string", "[time_utils]") { @@ -157,11 +175,13 @@ TEST_CASE("parseISOTimestamp and formatISOTimestamp - round trip without millise std::string original = "2024-06-09T14:23:11Z"; // Parse and format - auto parsed = parseISOTimestamp(original); + std::string error; + auto parsed = parseISOTimestamp(original, error); std::string formatted = formatISOTimestamp(parsed); // The formatted result will include .000 for milliseconds even if original didn't have them REQUIRE(formatted == "2024-06-09T14:23:11.000Z"); + REQUIRE(error.empty()); } TEST_CASE("parseISOTimestamp and formatISOTimestamp - round trip preserves milliseconds", @@ -169,7 +189,8 @@ TEST_CASE("parseISOTimestamp and formatISOTimestamp - round trip preserves milli std::string original = "2024-06-09T14:23:11.123"; // Parse and format - auto parsed = parseISOTimestamp(original); + std::string error; + auto parsed = parseISOTimestamp(original, error); std::string formatted = formatISOTimestamp(parsed); REQUIRE(formatted == "2024-06-09T14:23:11.123Z"); @@ -179,4 +200,5 @@ TEST_CASE("parseISOTimestamp and formatISOTimestamp - round trip preserves milli auto milliseconds = std::chrono::duration_cast(parsed.time_since_epoch()) - seconds; REQUIRE(milliseconds.count() == 123); + REQUIRE(error.empty()); }