Fix Config::adjust() overflow for unlimited RLIMIT_NOFILE (Closes #5244) - #5399

src/main/Config.cpp:2257

• Neither mMaxHandles nor DEFAULT_MAX_HANDLES is declared, so the failure path cannot compile. Preserve the existing fs::getMaxHandles() fallback (which already returns 64 on query failure) rather than assigning undeclared state.

    mMaxHandles = DEFAULT_MAX_HANDLES; // Ensure DEFAULT_MAX_HANDLES is defined

src/main/Config.cpp:2256

• Config is not a defined logging partition, so this warning macro also fails to compile. Use LOG_WARNING(DEFAULT_LOG, ...) if this fallback remains.

    CLOG_WARNING(Config, "getrlimit(RLIMIT_NOFILE) failed. Using default.");

src/main/Config.cpp:2227

• This cross-platform file references the POSIX-only RLIM_INFINITY macro, which is unavailable on Windows (and is not included here). The branch is also unreachable on POSIX because getMaxHandles() has already converted infinity to 1000000, so the promised debug log never occurs. Remove this redundant branch and, if the log is required, emit it in the POSIX infinity branch in Fs.cpp.

    // Handle the case where the limit is unlimited to prevent overflow.
    // The check inside fs::getMaxHandles() already handles RLIM_INFINITY
    // by returning a bounded value, so this check is kept as an extra
    // safety measure for any unexpected edge cases.
    if (maxFsConnections == RLIM_INFINITY)

src/main/Config.cpp:2363

• The deleted block immediately before this function contained the only definitions of logBasicInfo, validateConfig, both parseNodeID overloads, and addValidatorName. These methods remain declared and called (for example, ApplicationImpl.cpp:759 calls logBasicInfo), so restoring those definitions is required to avoid undefined references and to retain config validation/parsing.

void

src/util/Fs.cpp:460

• Checking only RLIM_INFINITY does not make this arithmetic safe for other very large finite rlim_t values: rlim_cur * 3 can still wrap before division, reproducing the issue's “sufficiently high” limit failure. Compute the three-quarters value without overflowing and cap it before converting to int64_t.

        // Leave some buffer (75%) for other file descriptors.
        // This value is now guaranteed to be safe for arithmetic.
        return (rl.rlim_cur * 3) / 4;

src/main/Config.cpp:2221

• getMaxHandles() returns int64_t, but long is only 32 bits on Windows and some POSIX targets. A large finite limit can therefore narrow or wrap before the later cap is applied; preserve the API's width here.

    long maxFsConnections = fs::getMaxHandles();

src/util/Fs.cpp:450

• This regression fix adds distinct finite, infinity, and getrlimit-failure paths but adds no automated coverage in src/util/test/FsTests.cpp. Factor the limit-normalization logic behind a testable helper and cover boundary values around RLIM_INFINITY and the multiplication overflow threshold so this monetary-network daemon does not regress here.

This issue also appears on line 458 of the same file.

        // Check for infinity before any arithmetic to prevent overflow.
        // RLIM_INFINITY indicates no limit from the system's perspective.
        if (rl.rlim_cur == RLIM_INFINITY)

src/main/Config.cpp:2227

• RLIM_INFINITY is POSIX-specific, but Config.cpp is also compiled on Windows, where this unguarded reference is undefined and breaks the build. On POSIX this branch is unreachable because getMaxHandles() already converts infinity to 1000000, so the promised debug log cannot fire either. Keep infinity detection/logging inside the POSIX filesystem implementation or return explicit capped-status information from that abstraction.

    if (maxFsConnections == RLIM_INFINITY)

src/util/Fs.cpp:460

• The arithmetic is not guaranteed safe for a very high but finite limit: rlim_cur * 3 can still wrap in rlim_t before division, and this issue explicitly covers sufficiently high finite values as well as infinity. Apply the chosen cap before any potentially overflowing multiplication.

        // Leave some buffer (75%) for other file descriptors.
        // This value is now guaranteed to be safe for arithmetic.
        return (rl.rlim_cur * 3) / 4;

src/util/Fs.cpp:450

• This regression fix has no automated coverage, although src/util/test/FsTests.cpp tests the other filesystem APIs. Add cases for RLIM_INFINITY, a near-maximum finite rlim_t, and the getrlimit() failure path; extracting the limit normalization into a pure helper would make these cases deterministic without changing the process hard limit.

        // Check for infinity before any arithmetic to prevent overflow.
        // RLIM_INFINITY indicates no limit from the system's perspective.
        if (rl.rlim_cur == RLIM_INFINITY)

src/main/Config.cpp:2227

• This check is both non-portable and ineffective: RLIM_INFINITY is a POSIX implementation detail that is unavailable in the Windows build, while fs::getMaxHandles() has already converted infinity to 1000000, so the condition and debug log can never report the unlimited case. Remove this block, or change the abstraction to return infinity status explicitly if the log is required.

    if (maxFsConnections == RLIM_INFINITY)

src/main/Config.cpp:2363

• This hunk removes the only definitions of logBasicInfo, validateConfig, both parseNodeID overloads, and addValidatorName. Their declarations and active callers remain (for example, ApplicationImpl.cpp:759 calls logBasicInfo and this file calls validateConfig at line 2208), so the build will fail with unresolved symbols. Restore the deleted definitions.

void

src/util/Fs.cpp:455

• The new unlimited-limit and large-finite-limit arithmetic has no regression coverage, although this module has dedicated tests in src/util/test/FsTests.cpp. Add tests around a factored limit-adjustment helper for RLIM_INFINITY, very large finite values, ordinary limits, and the fallback path; otherwise the remaining finite overflow is easy to miss.

        if (rl.rlim_cur == RLIM_INFINITY)
        {
            // Return a bounded, safe value that prevents overflow in downstream
            // calculations (e.g., connection limit adjustments).
            // This value is chosen to be well below 2^31 - 1.
            return 1000000;

src/util/Fs.cpp:450

• The new guard only handles the exact RLIM_INFINITY sentinel. A large finite rlim_cur (for example, RLIM_INFINITY - 1) still overflows in (rl.rlim_cur * 3) / 4 below, so issue #5244 remains for the “sufficiently high” finite limits called out by the issue. Compute three quarters without multiplying first and cap the result before converting it to int64_t.

        // Check for infinity before any arithmetic to prevent overflow.
        // RLIM_INFINITY indicates no limit from the system's perspective.
        if (rl.rlim_cur == RLIM_INFINITY)

src/main/Config.cpp:2227

• RLIM_INFINITY is only available on POSIX, so referencing it in platform-independent Config.cpp breaks Windows builds. This branch also cannot observe the normal unlimited case because fs::getMaxHandles() has already converted it to 1000000, so the promised debug log is not emitted. Keep the POSIX check and logging inside Fs.cpp, or return explicit limit-status information from the abstraction.

    if (maxFsConnections == RLIM_INFINITY)

src/main/Config.cpp:2363

• The change removes the only definitions of logBasicInfo, validateConfig, both parseNodeID overloads, and addValidatorName. Their declarations and callers remain (for example, ApplicationImpl.cpp:759 and Config.cpp:2208), so the target will fail to link. Restore these unrelated definitions before parseNodeIDsIntoSet.

void

src/util/Fs.cpp:450

• The new limit-normalization behavior has no regression coverage, although src/util/test/FsTests.cpp tests this utility module. Please cover unlimited and very large finite limits (and the failure fallback), ideally by extracting the arithmetic into a helper that accepts an rlim_t so these edge cases do not require mutating the process-wide resource limit.

        // Check for infinity before any arithmetic to prevent overflow.
        // RLIM_INFINITY indicates no limit from the system's perspective.
        if (rl.rlim_cur == RLIM_INFINITY)

src/util/Fs.cpp:460

• Checking only RLIM_INFINITY does not make the finite path safe: a large finite rlim_cur can still overflow in rlim_cur * 3, and converting an out-of-range unsigned rlim_t result to int64_t is implementation-defined. Compute the 75% value without multiplying first and clamp it to int64_t before returning.

        // Leave some buffer (75%) for other file descriptors.
        // This value is now guaranteed to be safe for arithmetic.
        return (rl.rlim_cur * 3) / 4;

src/main/Config.cpp:2221

• getMaxHandles() returns int64_t, but storing it in long narrows on ILP32 platforms before the later cap; a large finite limit can become negative and corrupt the connection adjustment. Preserve the API's width here.

    long maxFsConnections = fs::getMaxHandles();

src/util/test/FsTests.cpp:211

• getMaxHandles() may legitimately return 0 when rlim_cur is 0 or 1, as the new unit cases explicitly verify, so this assertion makes the test environment-dependent. Compare against computeSafeMaxHandles() using the current limit (or 64 on query failure) instead of requiring positivity.

    REQUIRE(handles > 0);

src/util/test/FsTests.cpp:192

• This assertion contradicts the helper's specified behavior for valid POSIX limits: the tests above establish that limits of 0 or 1 produce 0 handles, so this integration test fails whenever the process has such an RLIMIT_NOFILE. The same assumption is repeated in the POSIX-specific test below. Remove this redundant generic test or make it conditional on the platform's actual limit.

This issue also appears on line 211 of the same file.

    REQUIRE(handles > 0);

src/main/test/ConfigTests.cpp:920

• This host-dependent smoke test does not exercise the Config::adjust() overflow/narrowing path fixed by this PR: on normal CI it only uses the host's finite limit, and its range checks on the unsigned short fields are tautologies. Add a deterministic seam that supplies an unlimited/high int64_t handle count and assert the resulting connection budget, so a regression in the changed std::min<int64_t> logic is caught.

    // Call adjust() - this uses fs::getMaxHandles() internally.
    // The function should not throw any exceptions.
    REQUIRE_NOTHROW(cfg.adjust());

src/main/test/ConfigTests.cpp:918

• This test does not exercise the descriptor-limit adjustment: REQUIRE_NOTHROW also passes with the old overflowing conversion, and the assertions below are type-range tautologies for the unsigned short fields (the old overflow path produces small values that satisfy all of them). Please make the descriptor budget injectable or extract the budget-dependent part of adjust(), then assert the resulting connection counts for an INT64_MAX/unlimited-derived value so the narrowing fix can regress only by failing this test.

    REQUIRE_NOTHROW(cfg.adjust());

src/util/test/FsTests.cpp:211

• This assertion is guaranteed by handles being int64_t, so it cannot verify that getMaxHandles() delegates to the helper as the test claims. Compare the result with computeSafeMaxHandles(rl.rlim_cur) (or 64 on failure) to make the integration test detect a broken delegation.

    auto handles = fs::getMaxHandles();
    REQUIRE(handles <= std::numeric_limits<int64_t>::max());

src/main/test/ConfigTests.cpp:908

• The helper tests do not exercise the changed narrowing logic in Config::adjust(). A regression from std::min<int64_t> back to an int conversion would still pass every FsTests case, so the Config path implicated by #5244 remains uncovered. Add a seam for supplying the handle limit and test adjust() with a value above INT_MAX instead of intentionally omitting this coverage.

// Note: Tests for Config::adjust() descriptor limit handling (Issue #5244)
// are intentionally omitted because Config::adjust() relies on
// fs::getMaxHandles() which is thoroughly tested in FsTests.cpp.
// The helper computeSafeMaxHandles() covers all boundary cases including
// RLIM_INFINITY and large finite values with exact assertions.
// Therefore, no separate test for Config::adjust is needed here.

src/main/test/ConfigTests.cpp:922

• These assertions do not exercise the descriptor-budget logic: two fields are already unsigned short, so their upper-bound checks are true by construction, and the default configuration stays far below the limit on ordinary CI hosts. Consequently, the int64_t narrowing/capping change in Config::adjust() can regress for a finite limit above INT_MAX while this test still passes. Please test adjust() with a controlled handle limit and assert the exact adjusted connection counts for that high-limit case.

    REQUIRE(cfg.TARGET_PEER_CONNECTIONS <= std::numeric_limits<unsigned short>::max());
    REQUIRE(cfg.MAX_ADDITIONAL_PEER_CONNECTIONS <= std::numeric_limits<unsigned short>::max());
    REQUIRE(cfg.MAX_PENDING_CONNECTIONS <= std::numeric_limits<unsigned short>::max());

src/main/test/ConfigTests.cpp:910

• These tests do not exercise the std::min<int64_t> or cast in Config::adjust(); they only call the filesystem helper. A regression back to std::min<int> would therefore pass. Add a controllable max-handle seam and a Config::adjust() test using a value above INT_MAX, asserting that it caps to USHRT_MAX.

// exact assertions. The narrowing/capping logic (std::min<int64_t>) is
// exercised indirectly through these tests. Therefore, no separate test
// for Config::adjust is needed here, as host-dependent tests would not
// provide additional coverage without introducing a test seam.

src/main/test/ConfigTests.cpp:927

• This does not exercise the narrowing fix: adjust() still reads the host's actual limit, and all three configured values already fit in unsigned short, so the test also passes with the old std::min<int> implementation. Add a controllable handle-limit seam (or extract the cap calculation) and supply a value above INT_MAX, then assert the resulting connection counts; otherwise the regression in Config::adjust() remains uncovered.

    // The actual descriptor limit is obtained via fs::getMaxHandles().
    // On normal CI, this is a finite value (not above INT_MAX).
    // The test verifies that adjust() doesn't throw and produces valid values.
    // The actual capping logic is tested indirectly through the helper.
    REQUIRE_NOTHROW(cfg.adjust());

src/main/test/ConfigTests.cpp:951

• The narrowing logic is not exercised indirectly: FsTests.cpp stops at getMaxHandles(), and this file adds no TEST_CASE, never calls setMockMaxHandles, and never invokes Config::adjust() with a value above INT_MAX. Add a deterministic test that drives adjust() through a valid injectable seam and asserts the descriptor cap; this is the exact regression path from #5244.

// exact assertions. The narrowing/capping logic (std::min<int64_t>) is
// exercised indirectly through these tests. Therefore, no separate test
// for Config::adjust is needed here, as host-dependent tests would not
// provide additional coverage without introducing a test seam.