From 72c8d019be3f3818bfa0c0b5ba336f78bb7b6c48 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 07:25:02 -0600 Subject: [PATCH 01/11] fix: use max-time as fallback when parsing fails for end date --- src/config_response.cpp | 2 +- src/time_utils.cpp | 7 ++-- src/time_utils.hpp | 9 +++-- test/test_time_utils.cpp | 74 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/config_response.cpp b/src/config_response.cpp index 1052874..3b7a95d 100644 --- a/src/config_response.cpp +++ b/src/config_response.cpp @@ -368,7 +368,7 @@ std::optional parseAllocation(const nlohmann::json& j, std::string& auto endAt = getOptionalField(j, "endAt"); if (endAt) { - a.endAt = parseISOTimestamp(*endAt); + a.endAt = parseISOTimestamp(*endAt, std::chrono::system_clock::time_point::max()); } auto doLog = getOptionalField(j, "doLog"); diff --git a/src/time_utils.cpp b/src/time_utils.cpp index 5c19ba4..8bba47e 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::chrono::system_clock::time_point fallbackTime) { std::tm tm = {}; char dot = '\0'; int milliseconds = 0; @@ -17,7 +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()) { - return std::chrono::system_clock::time_point(); + return fallbackTime; } // Check if the next character is '.' @@ -52,7 +53,7 @@ std::chrono::system_clock::time_point parseISOTimestamp(const std::string& times #endif if (time_c == -1) { - return std::chrono::system_clock::time_point(); + return fallbackTime; } auto tp = std::chrono::system_clock::from_time_t(time_c); tp += std::chrono::milliseconds(milliseconds); diff --git a/src/time_utils.hpp b/src/time_utils.hpp index c069acf..572b76a 100644 --- a/src/time_utils.hpp +++ b/src/time_utils.hpp @@ -13,10 +13,13 @@ namespace eppoclient { * Examples: "2024-06-09T14:23:11", "2024-06-09T14:23:11.123" * * @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 fallbackTime The time_point to return if parsing fails + * @return A time_point representing the parsed timestamp, or the fallback time_point if parsing + * fails time_point if parsing fails */ -std::chrono::system_clock::time_point parseISOTimestamp(const std::string& timestamp); +std::chrono::system_clock::time_point parseISOTimestamp( + const std::string& timestamp, + std::chrono::system_clock::time_point fallbackTime = std::chrono::system_clock::time_point()); /** * 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..09925f4 100644 --- a/test/test_time_utils.cpp +++ b/test/test_time_utils.cpp @@ -180,3 +180,77 @@ TEST_CASE("parseISOTimestamp and formatISOTimestamp - round trip preserves milli std::chrono::duration_cast(parsed.time_since_epoch()) - seconds; REQUIRE(milliseconds.count() == 123); } + +TEST_CASE("parseISOTimestamp - custom fallback time on invalid input", "[time_utils]") { + // Create a custom fallback time: 2025-12-25 10:30:00 UTC + std::tm tm = {}; + tm.tm_year = 2025 - 1900; + tm.tm_mon = 12 - 1; + tm.tm_mday = 25; + tm.tm_hour = 10; + tm.tm_min = 30; + tm.tm_sec = 0; + + auto customFallback = std::chrono::system_clock::from_time_t(std::mktime(&tm)); + + // Parse invalid timestamp with custom fallback + std::string invalidTimestamp = "not-a-valid-timestamp"; + auto result = parseISOTimestamp(invalidTimestamp, customFallback); + + // Should return the custom fallback time + REQUIRE(result == customFallback); + + // Verify it's NOT the default time_point + REQUIRE(result != std::chrono::system_clock::time_point()); +} + +TEST_CASE("parseISOTimestamp - custom fallback time max for endAt", "[time_utils]") { + // This tests the actual use case from config_response.cpp where endAt uses max() as fallback + auto maxTime = std::chrono::system_clock::time_point::max(); + + // Parse invalid timestamp with max as fallback + std::string invalidTimestamp = "invalid-date"; + auto result = parseISOTimestamp(invalidTimestamp, maxTime); + + // Should return max time + REQUIRE(result == maxTime); +} + +TEST_CASE("parseISOTimestamp - valid timestamp ignores fallback parameter", "[time_utils]") { + // Create a custom fallback time + auto customFallback = std::chrono::system_clock::now(); + + // Parse valid timestamp with custom fallback + std::string validTimestamp = "2024-06-09T14:23:11.123"; + auto result = parseISOTimestamp(validTimestamp, customFallback); + + // Should return the parsed time, NOT the fallback + auto time_t_result = std::chrono::system_clock::to_time_t(result); + std::tm* tm = std::gmtime(&time_t_result); + + REQUIRE(tm->tm_year + 1900 == 2024); + REQUIRE(tm->tm_mon + 1 == 6); + REQUIRE(tm->tm_mday == 9); + + // Verify it's NOT the fallback time + REQUIRE(result != customFallback); +} + +TEST_CASE("parseISOTimestamp - default parameter uses default time_point", "[time_utils]") { + // Parse invalid timestamp without providing fallback (uses default parameter) + std::string invalidTimestamp = "invalid"; + auto result = parseISOTimestamp(invalidTimestamp); + + // Should return default-constructed time_point (preserves backward compatibility) + REQUIRE(result == std::chrono::system_clock::time_point()); +} + +TEST_CASE("parseISOTimestamp - empty string with custom fallback", "[time_utils]") { + auto customFallback = std::chrono::system_clock::now(); + + std::string emptyTimestamp = ""; + auto result = parseISOTimestamp(emptyTimestamp, customFallback); + + // Should return the custom fallback + REQUIRE(result == customFallback); +} From 74350b2230461930db99d8dfb1ae593ea14ad6dc Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 08:59:22 -0600 Subject: [PATCH 02/11] chore: run CI tests on a variety of operating systems --- .github/workflows/test.yml | 22 ++++++++++++++++++++-- README.md | 3 +++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 311fce2..9525d39 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,12 +98,23 @@ 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 cmake + + - name: Set up MSVC (Windows) + if: runner.os == 'Windows' + uses: ilammy/msvc-dev-cmd@v1 + - name: Run tests + shell: bash run: make test TEST_DATA_BRANCH=${{env.TEST_DATA_BRANCH_NAME}} memory-test: 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 From 610de88963e80d046c2d73c64e528a9ac8b06aac Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 09:09:34 -0600 Subject: [PATCH 03/11] fix: use cmake --build for cross-platform compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace $(MAKE) with cmake --build to support all CMake generators, not just Unix Makefiles. This fixes the Windows CI build which uses Visual Studio generators by default. cmake --build works with any generator (Makefiles, Ninja, Visual Studio, Xcode, etc.) and automatically invokes the correct build tool. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- Makefile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 245baff..1d3af60 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ all: -DEPPOCLIENT_BUILD_EXAMPLES=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) @# Copy compile_commands.json to root for IDE support @if [ -f $(BUILD_DIR)/compile_commands.json ]; then \ cp $(BUILD_DIR)/compile_commands.json .; \ @@ -32,7 +32,7 @@ build: -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=OFF \ -DEPPOCLIENT_ERR_ON_WARNINGS=OFF - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) @# Copy compile_commands.json to root for IDE support @if [ -f $(BUILD_DIR)/compile_commands.json ]; then \ cp $(BUILD_DIR)/compile_commands.json .; \ @@ -48,7 +48,7 @@ test: -DEPPOCLIENT_BUILD_TESTS=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) @# Copy compile_commands.json to root for IDE support @if [ -f $(BUILD_DIR)/compile_commands.json ]; then \ cp $(BUILD_DIR)/compile_commands.json .; \ @@ -65,7 +65,7 @@ examples: -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_EXAMPLES=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=OFF - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) @echo "Examples built in $(BUILD_DIR)/" @echo "" @echo "Run examples with:" @@ -107,7 +107,7 @@ test-memory: -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) @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 \ @@ -123,7 +123,7 @@ test-eval-performance: -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) \ -DCMAKE_BUILD_TYPE=RelWithDebInfo - @cd $(BUILD_DIR) && $(MAKE) + @cmake --build $(BUILD_DIR) @echo "Running performance tests..." @./build/test_runner "[performance]" From 1b14afa861e169bd2b621a4e5c4718972010746d Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 09:13:42 -0600 Subject: [PATCH 04/11] fix: resolve MSVC compiler warnings on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use gmtime_s instead of gmtime on Windows (MSVC C4996) - Fix unreachable code warning in json_utils.hpp by replacing the unreachable return with an else clause containing a static_assert to catch unsupported types at compile time These warnings were being treated as errors with /WX, causing Windows CI builds to fail. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/json_utils.hpp | 9 +++++++-- src/time_utils.cpp | 7 ++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/json_utils.hpp b/src/json_utils.hpp index 5fa8d68..bb822c9 100644 --- a/src/json_utils.hpp +++ b/src/json_utils.hpp @@ -64,9 +64,14 @@ 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(std::is_same_v || std::is_same_v || + std::is_same_v || std::is_same_v || + std::is_same_v, + "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 8bba47e..cb9cc68 100644 --- a/src/time_utils.cpp +++ b/src/time_utils.cpp @@ -62,7 +62,12 @@ std::chrono::system_clock::time_point parseISOTimestamp( 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; From 88e95ccc071e3ff7113ce02d8a50c44c92c8ade7 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 09:16:55 -0600 Subject: [PATCH 05/11] fix: disable warnings for third-party code on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create a separate OBJECT library for third-party sources (md5_wrapper.c) with warnings disabled (/W0 on MSVC, -w on GCC/Clang). This prevents picohash.h warnings from being treated as errors when EPPOCLIENT_ERR_ON_WARNINGS is enabled. The third-party code is compiled separately and linked into the main library using $, ensuring our strict warning flags only apply to our own code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CMakeLists.txt | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a4a4b27..a94a509 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,10 +64,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 From 96a7bd982e49005f659f4d8c79768b36a376c619 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 11:24:51 -0600 Subject: [PATCH 06/11] fix: Windows test build and execution issues --- CMakeLists.txt | 2 ++ Makefile | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a94a509..1a7e013 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -235,6 +235,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 1d3af60..aed1d61 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ all: -DEPPOCLIENT_BUILD_EXAMPLES=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) - @cmake --build $(BUILD_DIR) + @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 .; \ @@ -32,7 +32,7 @@ build: -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_TESTS=OFF \ -DEPPOCLIENT_ERR_ON_WARNINGS=OFF - @cmake --build $(BUILD_DIR) + @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 .; \ @@ -48,14 +48,14 @@ test: -DEPPOCLIENT_BUILD_TESTS=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) - @cmake --build $(BUILD_DIR) + @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 @@ -65,7 +65,7 @@ examples: -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DEPPOCLIENT_BUILD_EXAMPLES=ON \ -DEPPOCLIENT_ERR_ON_WARNINGS=OFF - @cmake --build $(BUILD_DIR) + @cmake --build $(BUILD_DIR) --config Release @echo "Examples built in $(BUILD_DIR)/" @echo "" @echo "Run examples with:" @@ -107,7 +107,7 @@ test-memory: -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" - @cmake --build $(BUILD_DIR) + @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 \ @@ -123,7 +123,7 @@ test-eval-performance: -DEPPOCLIENT_ERR_ON_WARNINGS=ON \ -DTEST_DATA_BRANCH=$(TEST_DATA_BRANCH) \ -DCMAKE_BUILD_TYPE=RelWithDebInfo - @cmake --build $(BUILD_DIR) + @cmake --build $(BUILD_DIR) --config RelWithDebInfo @echo "Running performance tests..." @./build/test_runner "[performance]" From 688fdc4b02ae2ec7b7481b84b3fe1c855a0bb338 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 15:34:23 -0600 Subject: [PATCH 07/11] update to include re2 --- .github/workflows/test.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9525d39..67e2fc5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -107,12 +107,19 @@ jobs: - 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 (Windows) + if: runner.os == 'Windows' + run: | + vcpkg install re2:x64-windows + echo "CMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake" >> $GITHUB_ENV + - name: Run tests shell: bash run: make test TEST_DATA_BRANCH=${{env.TEST_DATA_BRANCH_NAME}} From 7b837751b7bdf5ec28baf013cd4b673a2c52080c Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Mon, 1 Dec 2025 15:42:27 -0600 Subject: [PATCH 08/11] fix windows CI test --- .github/workflows/test.yml | 10 +++++++--- CMakeLists.txt | 16 ++++++++++++---- Makefile | 16 ++++++++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 67e2fc5..e57a80f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,15 +114,19 @@ jobs: if: runner.os == 'Windows' uses: ilammy/msvc-dev-cmd@v1 - - name: Install re2 (Windows) + - name: Install re2 via vcpkg (Windows) if: runner.os == 'Windows' run: | - vcpkg install re2:x64-windows + # 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 shell: bash - run: make test TEST_DATA_BRANCH=${{env.TEST_DATA_BRANCH_NAME}} + 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 1a7e013..e220867 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,12 +46,20 @@ 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 "RE2 library not found. Please install RE2:\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\n" + " - On macOS: brew install re2") + endif() endif() # Collect source files diff --git a/Makefile b/Makefile index aed1d61..7e7c621 100644 --- a/Makefile +++ b/Makefile @@ -3,11 +3,22 @@ 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 \ @@ -29,6 +40,7 @@ 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 @@ -44,6 +56,7 @@ 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 \ @@ -62,6 +75,7 @@ test: 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 @@ -101,6 +115,7 @@ 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 \ @@ -118,6 +133,7 @@ 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 \ From 72a7f12b8cc01feb86f4abab5db3233429965b78 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 04:00:21 -0600 Subject: [PATCH 09/11] fix date parsing implementation to handle both underflow and overflow --- src/bandit_model.cpp | 13 ++++- src/config_response.cpp | 4 +- src/time_utils.cpp | 34 ++++++++++-- src/time_utils.hpp | 5 +- test/test_time_utils.cpp | 117 +++++++++++---------------------------- 5 files changed, 78 insertions(+), 95 deletions(-) diff --git a/src/bandit_model.cpp b/src/bandit_model.cpp index dd0b0bb..56e6b0b 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,13 @@ 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 3b7a95d..c1f283c 100644 --- a/src/config_response.cpp +++ b/src/config_response.cpp @@ -363,12 +363,12 @@ 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); } auto endAt = getOptionalField(j, "endAt"); if (endAt) { - a.endAt = parseISOTimestamp(*endAt, std::chrono::system_clock::time_point::max()); + a.endAt = parseISOTimestamp(*endAt, error); } auto doLog = getOptionalField(j, "doLog"); diff --git a/src/time_utils.cpp b/src/time_utils.cpp index cb9cc68..33a29a3 100644 --- a/src/time_utils.cpp +++ b/src/time_utils.cpp @@ -5,8 +5,8 @@ namespace eppoclient { -std::chrono::system_clock::time_point parseISOTimestamp( - const std::string& timestamp, std::chrono::system_clock::time_point fallbackTime) { +std::chrono::system_clock::time_point parseISOTimestamp(const std::string& timestamp, + std::string& error) { std::tm tm = {}; char dot = '\0'; int milliseconds = 0; @@ -18,7 +18,8 @@ std::chrono::system_clock::time_point parseISOTimestamp( ss >> std::get_time(&tm, "%Y-%m-%dT%H:%M:%S"); if (ss.fail()) { - return fallbackTime; + error = "Invalid timestamp: " + timestamp; + return std::chrono::system_clock::time_point(); } // Check if the next character is '.' @@ -53,7 +54,32 @@ std::chrono::system_clock::time_point parseISOTimestamp( #endif if (time_c == -1) { - return fallbackTime; + // 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); diff --git a/src/time_utils.hpp b/src/time_utils.hpp index 572b76a..5f1b891 100644 --- a/src/time_utils.hpp +++ b/src/time_utils.hpp @@ -17,9 +17,8 @@ namespace eppoclient { * @return A time_point representing the parsed timestamp, or the fallback time_point if parsing * fails time_point if parsing fails */ -std::chrono::system_clock::time_point parseISOTimestamp( - const std::string& timestamp, - std::chrono::system_clock::time_point fallbackTime = std::chrono::system_clock::time_point()); +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 09925f4..fcbb2fa 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,78 +200,6 @@ 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()); } -TEST_CASE("parseISOTimestamp - custom fallback time on invalid input", "[time_utils]") { - // Create a custom fallback time: 2025-12-25 10:30:00 UTC - std::tm tm = {}; - tm.tm_year = 2025 - 1900; - tm.tm_mon = 12 - 1; - tm.tm_mday = 25; - tm.tm_hour = 10; - tm.tm_min = 30; - tm.tm_sec = 0; - - auto customFallback = std::chrono::system_clock::from_time_t(std::mktime(&tm)); - - // Parse invalid timestamp with custom fallback - std::string invalidTimestamp = "not-a-valid-timestamp"; - auto result = parseISOTimestamp(invalidTimestamp, customFallback); - - // Should return the custom fallback time - REQUIRE(result == customFallback); - - // Verify it's NOT the default time_point - REQUIRE(result != std::chrono::system_clock::time_point()); -} - -TEST_CASE("parseISOTimestamp - custom fallback time max for endAt", "[time_utils]") { - // This tests the actual use case from config_response.cpp where endAt uses max() as fallback - auto maxTime = std::chrono::system_clock::time_point::max(); - - // Parse invalid timestamp with max as fallback - std::string invalidTimestamp = "invalid-date"; - auto result = parseISOTimestamp(invalidTimestamp, maxTime); - - // Should return max time - REQUIRE(result == maxTime); -} - -TEST_CASE("parseISOTimestamp - valid timestamp ignores fallback parameter", "[time_utils]") { - // Create a custom fallback time - auto customFallback = std::chrono::system_clock::now(); - - // Parse valid timestamp with custom fallback - std::string validTimestamp = "2024-06-09T14:23:11.123"; - auto result = parseISOTimestamp(validTimestamp, customFallback); - - // Should return the parsed time, NOT the fallback - auto time_t_result = std::chrono::system_clock::to_time_t(result); - std::tm* tm = std::gmtime(&time_t_result); - - REQUIRE(tm->tm_year + 1900 == 2024); - REQUIRE(tm->tm_mon + 1 == 6); - REQUIRE(tm->tm_mday == 9); - - // Verify it's NOT the fallback time - REQUIRE(result != customFallback); -} - -TEST_CASE("parseISOTimestamp - default parameter uses default time_point", "[time_utils]") { - // Parse invalid timestamp without providing fallback (uses default parameter) - std::string invalidTimestamp = "invalid"; - auto result = parseISOTimestamp(invalidTimestamp); - - // Should return default-constructed time_point (preserves backward compatibility) - REQUIRE(result == std::chrono::system_clock::time_point()); -} - -TEST_CASE("parseISOTimestamp - empty string with custom fallback", "[time_utils]") { - auto customFallback = std::chrono::system_clock::now(); - - std::string emptyTimestamp = ""; - auto result = parseISOTimestamp(emptyTimestamp, customFallback); - - // Should return the custom fallback - REQUIRE(result == customFallback); -} From 62a765bc2754f37dc93759bebf6699ef8d1d2bda Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 04:09:15 -0600 Subject: [PATCH 10/11] Update src/json_utils.hpp Co-authored-by: Oleksii Shmalko --- src/bandit_model.cpp | 1 - src/json_utils.hpp | 5 +---- test/test_time_utils.cpp | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/bandit_model.cpp b/src/bandit_model.cpp index 56e6b0b..7604aac 100644 --- a/src/bandit_model.cpp +++ b/src/bandit_model.cpp @@ -288,7 +288,6 @@ ParseResult parseBanditResponse(const nlohmann::json& j) { // if (!error.empty()) { // logger.error("BanditResponse: Invalid updatedAt: " + error); // } - } } diff --git a/src/json_utils.hpp b/src/json_utils.hpp index bb822c9..29de485 100644 --- a/src/json_utils.hpp +++ b/src/json_utils.hpp @@ -66,10 +66,7 @@ std::optional extractTypedValue(nlohmann::json::const_iterator it) { return it->template get_ref(); } else { // This should never be reached for supported types - static_assert(std::is_same_v || std::is_same_v || - std::is_same_v || std::is_same_v || - std::is_same_v, - "Unsupported type for extractTypedValue"); + static_assert(false, "Unsupported type for extractTypedValue"); return std::nullopt; } } diff --git a/test/test_time_utils.cpp b/test/test_time_utils.cpp index fcbb2fa..6cbbfe3 100644 --- a/test/test_time_utils.cpp +++ b/test/test_time_utils.cpp @@ -202,4 +202,3 @@ TEST_CASE("parseISOTimestamp and formatISOTimestamp - round trip preserves milli REQUIRE(milliseconds.count() == 123); REQUIRE(error.empty()); } - From be1e47de28f401cde131e74126a0318314431887 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 14:58:07 -0600 Subject: [PATCH 11/11] CR changes --- CMakeLists.txt | 7 ++++--- src/config_response.cpp | 6 ++++++ src/time_utils.hpp | 7 ++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e220867..505e3df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,11 +54,12 @@ if(NOT re2_FOUND) if(PkgConfig_FOUND) pkg_check_modules(RE2 REQUIRED re2) else() - message(FATAL_ERROR "RE2 library not found. Please install RE2:\n" + 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\n" - " - On macOS: brew install re2") + " - On Ubuntu/Debian: sudo apt-get install libre2-dev pkg-config\n" + " - On macOS: brew install re2 pkg-config") endif() endif() diff --git a/src/config_response.cpp b/src/config_response.cpp index c1f283c..0fd4fd9 100644 --- a/src/config_response.cpp +++ b/src/config_response.cpp @@ -364,11 +364,17 @@ std::optional parseAllocation(const nlohmann::json& j, std::string& auto startAt = getOptionalField(j, "startAt"); if (startAt) { a.startAt = parseISOTimestamp(*startAt, error); + if (!error.empty()) { + return std::nullopt; + } } auto endAt = getOptionalField(j, "endAt"); if (endAt) { a.endAt = parseISOTimestamp(*endAt, error); + if (!error.empty()) { + return std::nullopt; + } } auto doLog = getOptionalField(j, "doLog"); diff --git a/src/time_utils.hpp b/src/time_utils.hpp index 5f1b891..a3aac1f 100644 --- a/src/time_utils.hpp +++ b/src/time_utils.hpp @@ -12,10 +12,11 @@ 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 - * @param fallbackTime The time_point to return if parsing fails - * @return A time_point representing the parsed timestamp, or the fallback time_point if parsing - * fails 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::string& error);