a float cap or timeout refuses a bool, nan, inf or a value that only rounds to zero (BACKLOG #3432)

changelog.d/3432.fixed.md

@@ -0,0 +1,36 @@
+- **A float cap or timeout refuses a bool, a NaN, an infinity, or text that only rounds to zero.**
+  Before this fix, a bare `float()` read `True` as one second and took `nan` and `inf`. A float cap
+  such as `receive_timeout`, `idle_timeout_seconds`, `max_frame_seconds` or a
+  `max_messages_per_second` rate read text such as `"1e-400"` as off, because `float()` rounds it to
+  `0.0`, so the guard it sets would not have run. Now one reader, `float_setting` in
+  `transports/base.py`, reads every float cap that `cap_setting` or `positive_cap` reads, and
+  `poll_seconds`. Each refusal names the key and withholds the value. The reader's docstring holds
+  the full rule. (`BACKLOG #3432`)
+  - A float cap is off only for an exact zero, such as `0`, `"0"`, `"0.0"` or `"0_0"`. `"0e0"` is
+    refused, as it already was on an integer cap.
+  - A `connections.toml` literal `1e-400` still reads as zero. The TOML parser turns it into `0.0`
+    before any reader sees it.
+  - The reader also replaces bare `float()` reads of `timeout_seconds` and `connect_timeout` on the
+    MLLP, TCP, X12 and DICOM connections.
+  - It reads `timeout_seconds` on the FHIR, FHIR lookup, REST, SOAP, DICOMweb, Email and Direct
+    connections.
+  - On those timeouts, `false` used to read as a zero-second timeout and `true` as one second.
+    Both now fail at build. A float cap read through `cap_setting`, such as `receive_timeout`,
+    already refused a bool (`BACKLOG #3431`).
+  - It reads `acquire_timeout` on the DATABASE connections, the DATABASE lookup and a DATABASE
+    reference source. `messagefoundry check` now refuses at least a bool, a NaN or `"1e-400"` as
+    a reference source's `acquire_timeout`. That covers a literal, and an `env()` value when it
+    resolves at check time. A zero or negative one still passes.
+  - It reads the REMOTEFILE `connect_timeout`, the File source's `min_age_seconds` and the Timer
+    source's `interval_seconds`.
+  - It reads the OAuth2 and SMART token timeouts and expiry skews.
+  - It reads the HTTP source's `reply_timeout` and `reply_write_timeout`. An `HttpSource` built
+    directly now refuses a zero there, where a numeric `0` took the 30-second default. `Http()` and
+    `connections.toml` already refused it.
+  - The `Http` factory's own check of those two keys now refuses a bool, a NaN, an infinity and an
+    integer too large for a float.
+  - The other timeouts above are checked for type only. A zero or negative `timeout_seconds`,
+    `connect_timeout` or `acquire_timeout` is still not refused at build.
+  - Range refusals withhold the value too. At least `positive_cap`, `resolve_poll_ceiling` and
+    `poll_interval` do, and so do the HTTP `max_header_bytes` and File `max_decompressed_members`
+    checks. So do the MLLP `max_connections_per_host` and `max_frame_seconds` checks.

messagefoundry/config/wiring.py

@@ -37,6 +37,7 @@
 import inspect
 import ipaddress
 import logging
+import math
 import os
 import re
 import sys
@@ -875,6 +876,23 @@
             check_db_connect_timeout(connect_timeout, "DatabaseRef")
         except ValueError as exc:
             raise WiringError(str(exc)) from None
+    # The same holds for acquire_timeout: a bool or a NaN would otherwise be refused only at the first
+    # sync, after start, so `messagefoundry check` passed it (BACKLOG #3432). The sync reads it again,
+    # by the same rule, because an env() ref is resolved only then.
+    if not isinstance(acquire_timeout, EnvRef):
+        # Imported here: messagefoundry.transports imports this module at its top level.
+        from messagefoundry.transports.base import float_setting
+
+        # The refusal is raised outside the handler, so it carries no exception chain.
+        refusal: str | None = None
+        try:
+            float_setting(
+                acquire_timeout, knob="acquire_timeout", transport="DATABASE reference source"
+            )
+        except ValueError as exc:
+            refusal = str(exc)
+        if refusal is not None:
+            raise WiringError(refusal)
     return ReferenceSourceSpec(
         "database",
         {
@@ -2464,7 +2482,7 @@
         oc.spec.settings["capture_response_headers"] = [*current, "content-type"]
 
 
 def _validate_sync_reply(settings: Mapping[str, Any]) -> None:
     """Refuse a synchronous-reply configuration that cannot work, at **factory** time (ADR 0154 D4).
 
     Factory-local checks only — everything decidable from this one ``Http()`` call, with no store, no
@@ -2494,10 +2512,29 @@
 
     for name in ("reply_timeout", "reply_write_timeout"):
         value = settings[name]
-        if not isinstance(value, int | float) or value <= 0:
+        # A bool is an int, and NaN and inf pass `<= 0`, so each is refused by name (BACKLOG #3432).
+        # The connector reads the same keys by the rule in transports/base.py float_setting.
+        # math.isfinite() raises OverflowError on an int too large for a float, such as 10**400 or a
+        # long TOML integer, so that int is refused by name here instead, outside the handler.
+        too_large = False
+        if isinstance(value, int) and not isinstance(value, bool):
+            try:
+                float(value)
+            except OverflowError:
+                too_large = True
+        if too_large:
+            raise WiringError(
+                f"Http {name} is not a valid float value (value withheld): it is too large for a float"
+            )
+        if (
+            isinstance(value, bool)
+            or not isinstance(value, int | float)
+            or not math.isfinite(value)
+            or value <= 0
+        ):
             raise WiringError(
-                f"Http {name} must be a positive number of seconds — got {value!r}. It bounds a "
-                "blocked HTTP turn; an unbounded or zero budget is not a timeout"
+                f"Http {name} must be a finite positive number of seconds (value withheld). It "
+                "bounds a blocked HTTP turn; an unbounded or zero budget is not a timeout"
             )
 
     if settings["reply_on_timeout"] not in ("504", "202"):

messagefoundry/pipeline/reference_sync.py

@@ -53,7 +53,7 @@
 from messagefoundry.pipeline.cluster import ClusterCoordinator, NullCoordinator
 from messagefoundry.store import Store
 from messagefoundry.store.base import DEFAULT_STORE_ACQUIRE_TIMEOUT, acquire_pooled
-from messagefoundry.transports.base import count_setting
+from messagefoundry.transports.base import count_setting, float_setting
 from messagefoundry.transports.egress import check_lookup_allowed
 
 __all__ = ["ReferenceSyncRunner", "ReferenceSyncError"]
@@ -111,6 +111,19 @@ def reference_connection_name(name: str) -> str:
     return f"reference:{name}"
 
 
+def reference_acquire_timeout(settings: Mapping[str, Any]) -> float:
+    """Read a DATABASE reference source's ``acquire_timeout`` by the rule in
+    :func:`~messagefoundry.transports.base.float_setting`, or refuse it (BACKLOG #3432).
+
+    One reader for the sync and for ``build_check_registry``, so an ``env()`` value the sync would
+    refuse fails ``messagefoundry check`` too. ``DatabaseRef`` reads a literal the same way."""
+    return float_setting(
+        settings.get("acquire_timeout", DEFAULT_STORE_ACQUIRE_TIMEOUT),
+        knob="acquire_timeout",
+        transport="DATABASE reference source",
+    )
+
+
 def database_source_dsn(settings: Mapping[str, Any], *, connection: str | None = None) -> str:
     """Build a DATABASE reference source's DSN, refusing weakened TLS under the AMBIENT hop posture.
 
@@ -164,6 +177,14 @@ async def _load_database_source(
         dsn = database_source_dsn(
             settings, connection=None if name is None else reference_connection_name(name)
         )
+    # BACKLOG #1052: bound the borrow. This pool is throwaway (closed in the finally below), but the
+    # acquire was unbounded, so an unresponsive server could hold the reference-sync runner's pass
+    # open indefinitely — and the runner is a single sequential loop over every declared set, so one
+    # wedged source would stall the sync of all the others. A StoreAcquireTimeout is an Exception, so
+    # the runner's existing per-set source-failure handler catches it, keeps the last-good snapshot
+    # and alerts, exactly as it does for any other source error.
+    # Read before the pool is made, so a refused value leaves no pool unclosed (BACKLOG #3432).
+    acquire_timeout = reference_acquire_timeout(settings)
     pool = await _make_pool(
         dsn,
         count_setting(
@@ -172,13 +193,6 @@ async def _load_database_source(
         autocommit=True,
         login_timeout=_login_timeout(settings, "DATABASE reference source", dialect="sqlserver"),
     )
-    # BACKLOG #1052: bound the borrow. This pool is throwaway (closed in the finally below), but the
-    # acquire was unbounded, so an unresponsive server could hold the reference-sync runner's pass
-    # open indefinitely — and the runner is a single sequential loop over every declared set, so one
-    # wedged source would stall the sync of all the others. A StoreAcquireTimeout is an Exception, so
-    # the runner's existing per-set source-failure handler catches it, keeps the last-good snapshot
-    # and alerts, exactly as it does for any other source error.
-    acquire_timeout = float(settings.get("acquire_timeout", DEFAULT_STORE_ACQUIRE_TIMEOUT))
     try:
         conn = await acquire_pooled(pool, timeout=acquire_timeout, backend="reference:database")
         try:

messagefoundry/pipeline/wiring_runner.py

@@ -169,7 +169,11 @@
     DeliveryPhaseTiming,
     delivery_phase_timing_enabled,
 )
-from messagefoundry.pipeline.reference_sync import database_source_dsn, reference_connection_name
+from messagefoundry.pipeline.reference_sync import (
+    database_source_dsn,
+    reference_acquire_timeout,
+    reference_connection_name,
+)
 from messagefoundry.pipeline.reply_wait import ReplyRendezvous
 from messagefoundry.pipeline.sandbox import (
     GraphShape,
@@ -218,6 +222,7 @@
 )
 from messagefoundry.transports.database import DatabaseLookupExecutor
 from messagefoundry.transports.fhir import FhirLookupExecutor
+from messagefoundry.transports.http_listener import reply_seconds
 from messagefoundry.transports.mllp import build_ack
 
 __all__ = ["DrParkedError", "NotDeployedError", "RegistryRunner", "ShardLaneOwnershipError"]
@@ -2071,7 +2076,7 @@ def _make_sync_reply_resolver(self, ic: InboundConnection) -> SyncReplyResolver
             self.store,
             self._reply_rendezvous,
             destination=str(settings["reply_from"]),
-            timeout=float(settings.get("reply_timeout") or 30.0),
+            timeout=reply_seconds(settings, "reply_timeout"),
             content_type=str(settings.get("reply_content_type") or "passthrough"),
             on_timeout=str(settings.get("reply_on_timeout") or "504"),
             metrics=metrics,
@@ -10848,6 +10853,9 @@ def _build_check_connectors(
         except WiringError:
             continue
         database_source_dsn(rsettings, connection=reference_connection_name(rname))
+        # BACKLOG #3432: the sync reads acquire_timeout by this rule, so an env() value it would
+        # refuse fails check here rather than as a failed sync after start.
+        reference_acquire_timeout(rsettings)
     resolved_fhir_lookups: dict[str, dict[str, Any]] = {}
     for fname, fspec in registry.fhir_lookups.items():
         resolved_fhir_lookups[fname] = _fhir_lookup_settings(fspec, env_values, egress)