Skip to content

Commit 7997788

Browse files
committed
fix: reject non-finite timeout env overrides; test env paths
Address CodeRabbit review on #1320: - _env_float now requires math.isfinite(value): "inf"/"Infinity"/"1e309"/ "nan" are positive-or-parseable but would produce unusable socket/timeout behaviour, so they now fall back to the default like other invalid input. - test_config_default_values clears the two env vars first so ambient env can't mask the defaults. - Added coverage for valid overrides and for invalid/zero/negative/ non-finite values falling back to defaults.
1 parent d8d06cb commit 7997788

2 files changed

Lines changed: 29 additions & 5 deletions

File tree

Server/src/core/config.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,23 @@
33
This file contains all configurable parameters for the server.
44
"""
55

6+
import math
67
import os
78
from dataclasses import dataclass, field
89

910

1011
def _env_float(name: str, default: float) -> float:
11-
"""Read a positive float from an environment variable, falling back to default.
12+
"""Read a positive, finite float from an environment variable.
1213
13-
Invalid or non-positive values are ignored so a bad override can't disable
14-
the timeout entirely.
14+
Invalid, non-positive, or non-finite values (e.g. "inf", "1e309", "nan")
15+
are ignored so a bad override can't disable the timeout or produce unusable
16+
socket/timeout behaviour.
1517
"""
1618
raw = os.environ.get(name)
1719
if raw:
1820
try:
1921
value = float(raw.strip())
20-
if value > 0:
22+
if math.isfinite(value) and value > 0:
2123
return value
2224
except (TypeError, ValueError):
2325
pass

Server/tests/test_core_infrastructure_characterization.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,8 +654,11 @@ def error_tool():
654654
class TestServerConfigDefaults:
655655
"""Tests for ServerConfig default values."""
656656

657-
def test_config_default_values(self):
657+
def test_config_default_values(self, monkeypatch):
658658
"""Verify ServerConfig has expected default values."""
659+
# Clear env overrides so the defaults aren't masked by the ambient env.
660+
monkeypatch.delenv("UNITY_MCP_CONNECTION_TIMEOUT", raising=False)
661+
monkeypatch.delenv("UNITY_MCP_COMMAND_TOTAL_TIMEOUT", raising=False)
659662
config = ServerConfig()
660663

661664
assert config.unity_host == "127.0.0.1"
@@ -670,6 +673,25 @@ def test_config_default_values(self):
670673
assert config.max_heartbeat_frames == 16
671674
assert config.heartbeat_timeout == 2.0
672675

676+
def test_timeout_env_overrides_are_honored(self, monkeypatch):
677+
"""Valid env overrides for the stdio timeouts are applied."""
678+
monkeypatch.setenv("UNITY_MCP_CONNECTION_TIMEOUT", "120.5")
679+
monkeypatch.setenv("UNITY_MCP_COMMAND_TOTAL_TIMEOUT", "240")
680+
config = ServerConfig()
681+
682+
assert config.connection_timeout == 120.5
683+
assert config.command_total_timeout == 240.0
684+
685+
@pytest.mark.parametrize("bad_value", ["0", "-5", "abc", "", "inf", "Infinity", "1e309", "nan"])
686+
def test_timeout_env_invalid_values_fall_back(self, monkeypatch, bad_value):
687+
"""Invalid, non-positive, or non-finite overrides preserve the defaults."""
688+
monkeypatch.setenv("UNITY_MCP_CONNECTION_TIMEOUT", bad_value)
689+
monkeypatch.setenv("UNITY_MCP_COMMAND_TOTAL_TIMEOUT", bad_value)
690+
config = ServerConfig()
691+
692+
assert config.connection_timeout == 300.0
693+
assert config.command_total_timeout == 600.0
694+
673695
def test_config_logging_defaults(self):
674696
"""Verify logging configuration defaults."""
675697
config = ServerConfig()

0 commit comments

Comments
 (0)