diff --git a/changelog.d/3432.fixed.md b/changelog.d/3432.fixed.md new file mode 100644 index 0000000000..9dca86f851 --- /dev/null +++ b/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. diff --git a/messagefoundry/config/wiring.py b/messagefoundry/config/wiring.py index 4dcd1a60b3..019294cc75 100644 --- a/messagefoundry/config/wiring.py +++ b/messagefoundry/config/wiring.py @@ -37,6 +37,7 @@ def handle(msg): import inspect import ipaddress import logging +import math import os import re import sys @@ -875,6 +876,23 @@ def DatabaseRef( 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", { @@ -2494,10 +2512,29 @@ def _validate_sync_reply(settings: Mapping[str, Any]) -> None: 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"): diff --git a/messagefoundry/pipeline/reference_sync.py b/messagefoundry/pipeline/reference_sync.py index d4fc51ab18..79cabcc226 100644 --- a/messagefoundry/pipeline/reference_sync.py +++ b/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: diff --git a/messagefoundry/pipeline/wiring_runner.py b/messagefoundry/pipeline/wiring_runner.py index d9d7a4523e..22d913a672 100644 --- a/messagefoundry/pipeline/wiring_runner.py +++ b/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) diff --git a/messagefoundry/transports/base.py b/messagefoundry/transports/base.py index 1535fcb74d..b57663e3f4 100644 --- a/messagefoundry/transports/base.py +++ b/messagefoundry/transports/base.py @@ -77,6 +77,8 @@ "DEFAULT_MAX_ITEMS_PER_POLL", "cap_setting", "count_setting", + "float_setting", + "positive_seconds", "positive_cap", "resolve_poll_ceiling", ] @@ -110,9 +112,11 @@ DEFAULT_MAX_ITEMS_PER_POLL = 500 -#: Text that spells zero in plain digits: ``"0"``, ``"00"``, ``"0.0"``, ``"-0.0"``, ``".0"``. An -#: ``int`` cap reads this as "off" without ``float()``, so text that ``float()`` merely rounds to -#: zero, such as ``"1e-400"``, reaches :func:`count_setting` and is refused there (BACKLOG #3431). +#: ASCII text that spells zero: ``"0"``, ``"00"``, ``"0.0"``, ``"-0.0"``, ``".0"``. A cap reads this +#: as "off" without ``float()``, so text that ``float()`` merely rounds to zero, such as +#: ``"1e-400"``, reaches :func:`count_setting` or :func:`float_setting` and is refused there +#: (BACKLOG #3431, #3432). It is a shortcut, not the whole rule: ``int()`` and :func:`_spells_zero` +#: each read a few more exact zeros, such as ``"0_0"``. _ZERO_TEXT = re.compile(r"[+-]?(?:0+(?:\.0*)?|\.0+)") @@ -142,8 +146,8 @@ def count_setting(value: Any, *, knob: str, transport: str) -> int: here already ``env()``-resolved, and a resolved value stays out of an error, as it does in ``_lookup_max_rows`` in ``transports/database.py`` (BACKLOG #1183). Every raise below sits outside the handler, so ``int()``'s own message, which quotes the value, is not left on ``__context__`` - (BACKLOG #1796). This covers the type refusals here and in :func:`cap_setting` only. At least the - range refusals in :func:`positive_cap` and :func:`resolve_poll_ceiling` still quote the value.""" + (BACKLOG #1796). The range refusals in :func:`positive_cap`, :func:`resolve_poll_ceiling` and + :func:`poll_interval` withhold it too (BACKLOG #3432).""" reason: str if isinstance(value, bool): reason = "a count is not a bool" @@ -171,6 +175,88 @@ def count_setting(value: Any, *, knob: str, transport: str) -> int: raise ValueError(f"{transport} {knob} is not a valid int value (value withheld): {reason}") +def float_setting(value: Any, *, knob: str, transport: str) -> float: + """Read one float setting, such as a timeout or a rate, as a finite number, or refuse it naming + the key (BACKLOG #3432). + + **This docstring holds the rule; the readers that use it point here.** They are at least every + ``float`` cap read through :func:`cap_setting`, :func:`poll_interval`, and the timeout reads that + call this directly. A bare ``float()`` turns ``True`` into 1.0, takes ``"nan"`` and ``"inf"``, + and rounds ``"1e-400"`` to ``0.0``. So a bool built a one-second timeout, a NaN built a timeout + that fires at once, and text that only rounds to zero could turn a guard off. So: + + - an ``int`` or a ``float`` passes, and so does another real number, such as a ``Decimal`` or a + ``Fraction``; + - a bool is refused, ``False`` as well as ``True``: a bool is not a number of seconds; + - a NaN or an infinity is refused, and so is a number too large for a float, such as ``1e400``; + - a value that ``float()`` only rounds to zero is refused, such as the text ``"1e-400"`` or + ``Decimal("1e-400")``. Text is zero only when every digit in it is a zero and it has no + exponent. So ``"0"``, ``"0.0"``, ``"-0"``, ``"0_0"`` and the full-width ``"0"`` are zero, + as :func:`count_setting`'s ``int()`` reads the whole-number ones, and ``"0e0"`` is refused; + - other text is read by ``float()``, so ``" 2.5 "`` and ``"+2.5"`` pass; + - anything else is refused, whatever ``float()`` would have made of it. + + Zero and negatives come back as they are. What a zero means, and whether a negative is a typo, is + each caller's own question, answered with a message naming its own key. + + **A TOML literal is parsed before this rule sees it.** ``tomllib`` reads ``x = 1e-400`` as + ``0.0``, so in ``connections.toml`` that literal reads exactly as a written ``0.0`` does, which is + zero. That stays: the spelling is gone before any reader runs, and the alternative is a + ``parse_float`` hook on the whole file. Text from an uncast ``env()`` reference keeps its + spelling, so the rule above applies to it. + + The refusal names the transport, the key and the reason, and withholds the value, by the rule in + :func:`count_setting` (BACKLOG #1183, #1796).""" + reason: str + if isinstance(value, bool): + reason = "it is a bool" + elif isinstance(value, str | numbers.Number): + # numbers.Number promises no conversion to float; one without it (a complex) raises TypeError. + number: Any = value + try: + seconds = float(number) + except OverflowError: + reason = "it is too large for a float" + except (TypeError, ValueError): + reason = "it is not a real number" + else: + if not math.isfinite(seconds): + reason = "it is not a finite number" + elif seconds == 0 and not _spells_zero(value): + reason = "float() reads it as zero, but it is not an exact zero in plain digits" + else: + return seconds + else: + reason = f"it is a {type(value).__name__}, not a number or text" + raise ValueError(f"{transport} {knob} is not a valid float value (value withheld): {reason}") + + +def positive_seconds(value: Any, *, knob: str, transport: str) -> float: + """:func:`float_setting`, then refuse a value that is not above zero, withholding it. + + For a number of seconds with no "off", where zero or a negative would make every wait end at + once: at least ``poll_seconds``, the HTTP reply timeouts and the Timer's ``interval_seconds``.""" + seconds = float_setting(value, knob=knob, transport=transport) + if seconds <= 0: + raise ValueError( + f"{transport} {knob} must be a number of seconds above zero (value withheld)" + ) + return seconds + + +def _spells_zero(value: Any) -> bool: + """Whether ``value``, which ``float()`` read as zero, is an exact zero: a number equal to zero, or + text whose digits are all zeros and which has no exponent. + + ``float()`` has already parsed the text, so without an exponent its value is its digits; when + every digit is a zero, the value is exactly zero. That takes ``"0_0"`` and the full-width + ``"0"``, which ``int()`` reads as ``0`` on an ``int`` cap too. An exponent is refused even on + zero digits, as ``"0e0"`` is on an ``int`` cap.""" + if isinstance(value, str): + return not any(c in "eE" or (c.isdecimal() and int(c) != 0) for c in value) + return bool(value == 0) + + def cap_setting[NumT: (int, float)]( value: Any, kind: type[NumT], *, knob: str, transport: str ) -> NumT | None: @@ -197,40 +283,29 @@ def cap_setting[NumT: (int, float)]( ``"0.0"`` or ``"-0.0"``, or when ``int()`` reads it as ``0``. Text is never tested with ``float()`` there, which would round ``"1e-400"`` to ``0.0`` and read it as off; that text goes to :func:`count_setting`, which refuses it, as it refuses ``"0e0"``. A number that equals zero, such - as ``0.0`` or ``-0.0``, is off too. On a ``float`` cap, any text ``float()`` reads as zero is off. - A ``float`` cap refuses a bool too, ``False`` included. + as ``0.0`` or ``-0.0``, is off too. + + **A ``float`` cap is read by :func:`float_setting`, and only an exact zero is its "off" (BACKLOG + #3432).** Its exact zeros are a little wider than an ``int`` cap's: text whose digits are all + zeros with no exponent, so ``"0_0.0"`` is off on a ``float`` cap and refused on an ``int`` cap, + whose ``int()`` cannot read it. ``"1e-400"`` and ``"0e0"`` are refused on both, and a bool, a NaN + and an infinity are refused too. That function's docstring holds the rule, including why a + ``connections.toml`` literal ``1e-400`` still reads as zero. A negative value is returned as-is, for the callers that refuse one at build with a message naming their own key; :func:`positive_cap` is that caller for most sites. It is a different mistake with a different answer, and folding it into "off" here would swallow the typo. A value that is not a - number is refused here, naming the key and withholding the value, by the rule in - :func:`count_setting`. + number is refused here, naming the key and withholding the value. """ if value is None or value == "": return None + if isinstance(value, str) and _ZERO_TEXT.fullmatch(value.strip()): + return None if kind is int: - if isinstance(value, str) and _ZERO_TEXT.fullmatch(value.strip()): - return None count = count_setting(value, knob=knob, transport=transport) return None if count == 0 else count - if isinstance(value, str): - with contextlib.suppress(ValueError): - if float(value) == 0: # at least "0", " 0 ", "0.0" and "-0.0" - return None - if isinstance(value, bool): - # float(True) is 1.0, so a bool would build a one-second timeout or a rate of one per second. - raise ValueError( - f"{transport} {knob} is not a valid float value (value withheld): it is a bool" - ) - cap: NumT | None - try: - cap = kind(value) - except (TypeError, ValueError, OverflowError): - cap = None - if cap is None: - # Raised after the handler ends: float()'s own message quotes the value (BACKLOG #1796). - raise ValueError(f"{transport} {knob} is not a valid float value (value withheld)") - # A NaN is not zero, so it comes back for the caller's range check to refuse. + # kind is float here, so kind() converts nothing; it gives the result the NumT type. + cap = kind(float_setting(value, knob=knob, transport=transport)) return None if cap == 0 else cap @@ -244,20 +319,18 @@ def positive_cap[NumT: (int, float)]( ) -> NumT | None: """:func:`cap_setting`, then refuse at build whatever is left that is not above zero (BACKLOG #1872). - After zero has been read as "off", what reaches the check is a negative or a NaN. Each one is a - cap that refuses all traffic, or, for a timeout, closes every peer the moment it connects, and - nobody writes one on purpose. Refused here, the typo surfaces at wiring and in ``messagefoundry - check``, not as a connection that reports running and accepts nothing. Written ``not cap > 0`` - rather than ``cap <= 0`` so a NaN float cap is refused too: every comparison with NaN is false, so - ``cap_setting`` cannot read it as off either. An ``int`` cap is a count, so it is held to the rule - in :func:`count_setting` first. + After zero has been read as "off", what reaches the check is a negative. It is a cap that refuses + all traffic, or, for a timeout, closes every peer the moment it connects, and nobody writes one on + purpose. Refused here, the typo surfaces at wiring and in ``messagefoundry check``, not as a + connection that reports running and accepts nothing. :func:`cap_setting` has already refused a + NaN (BACKLOG #3432). The refusal withholds the value, by the rule in :func:`count_setting`. ``off_hint`` ends the not-above-zero refusal. The default tells the operator how to turn the cap off. A caller where ``None``/``0`` does NOT turn it off passes its own text, so the refusal does not point at an off switch that does not exist (the DICOM SCP's ``max_object_bytes``, BACKLOG #2103).""" cap = cap_setting(value, kind, knob=knob, transport=transport) - if cap is not None and not cap > 0: - raise ValueError(f"{transport} {knob}={value!r} must be above zero ({off_hint})") + if cap is not None and cap <= 0: + raise ValueError(f"{transport} {knob} must be above zero (value withheld; {off_hint})") return cap @@ -273,7 +346,7 @@ def resolve_poll_ceiling(value: Any, *, knob: str, transport: str) -> int | None Clamping it to 1 would instead silently ingest at a rate nobody asked for. The connector already raises a :class:`ValueError` for a bad ``after_read``, so a bad number surfaces the same way: at wiring / ``messagefoundry check``, before the connection ever starts. The count itself is read by - :func:`count_setting`.""" + :func:`count_setting`, and the refusal withholds the value by its rule.""" # ``value`` is typed Any rather than object because every call site reads it out of an untyped # settings mapping; object would need a `type: ignore` on this line, and a suppression a reader # has to decide whether to trust is worse than the honest Any. @@ -282,8 +355,8 @@ def resolve_poll_ceiling(value: Any, *, knob: str, transport: str) -> int | None return None if ceiling < 1: raise ValueError( - f"{transport} {knob}={value!r} must be a positive number of items per poll " - f"(or 0 for unlimited)" + f"{transport} {knob} must be a positive number of items per poll " + "(value withheld; or 0 for unlimited)" ) return ceiling @@ -300,18 +373,10 @@ def poll_interval(settings: Mapping[str, Any], *, default: float, transport: str ``interval_seconds``. Deliberately not :func:`positive_cap`: that helper reads ``0`` as "off", and an interval has no - off. Written ``not interval > 0`` so a NaN, for which every comparison is false, is refused by - the same test.""" - value = settings.get("poll_seconds", default) - try: - interval = float(value) - except (TypeError, ValueError, OverflowError) as exc: - raise ValueError(f"{transport} poll_seconds={value!r} is not a number of seconds") from exc - if not (math.isfinite(interval) and interval > 0): - raise ValueError( - f"{transport} poll_seconds={value!r} must be a finite number of seconds above zero" - ) - return interval + off. Read by :func:`positive_seconds` (BACKLOG #3432).""" + return positive_seconds( + settings.get("poll_seconds", default), knob="poll_seconds", transport=transport + ) # A source hands each inbound message (raw bytes, MLLP framing already stripped) to this diff --git a/messagefoundry/transports/database.py b/messagefoundry/transports/database.py index cd709bae06..92653285d7 100644 --- a/messagefoundry/transports/database.py +++ b/messagefoundry/transports/database.py @@ -88,6 +88,7 @@ NegativeAckError, SourceConnector, count_setting, + float_setting, intake_open, poll_interval, register_destination, @@ -1304,7 +1305,11 @@ def __init__(self, config: Destination) -> None: self._pool_max = count_setting( s.get("pool_max", 5), knob="pool_max", transport="DATABASE destination" ) - self._acquire_timeout = float(s.get("acquire_timeout", _DEFAULT_DB_ACQUIRE_TIMEOUT)) + self._acquire_timeout = float_setting( + s.get("acquire_timeout", _DEFAULT_DB_ACQUIRE_TIMEOUT), + knob="acquire_timeout", + transport="DATABASE destination", + ) self._pool: Any = None self._pool_lock = asyncio.Lock() # ADR 0013: capture the statement's result-set (its RETURNING/OUTPUT rows). Default False → @@ -1619,7 +1624,11 @@ def __init__(self, config: Source) -> None: self._pool_max = count_setting( s.get("pool_max", 5), knob="pool_max", transport="DATABASE source" ) - self._acquire_timeout = float(s.get("acquire_timeout", _DEFAULT_DB_ACQUIRE_TIMEOUT)) + self._acquire_timeout = float_setting( + s.get("acquire_timeout", _DEFAULT_DB_ACQUIRE_TIMEOUT), + knob="acquire_timeout", + transport="DATABASE source", + ) self._pool: Any = None self._pool_lock = asyncio.Lock() self._handler: InboundHandler | None = None @@ -2049,8 +2058,10 @@ def __init__( self._login_timeout[cname] = _login_timeout( s, f"DatabaseLookup {cname!r}", dialect="sqlserver" ) - self._acquire_timeout[cname] = float( - s.get("acquire_timeout", _DEFAULT_DB_ACQUIRE_TIMEOUT) + self._acquire_timeout[cname] = float_setting( + s.get("acquire_timeout", _DEFAULT_DB_ACQUIRE_TIMEOUT), + knob="acquire_timeout", + transport=f"DatabaseLookup {cname!r}", ) self._max_rows[cname] = _lookup_max_rows(s.get("max_rows"), cname) self._pools: dict[str, Any] = {} diff --git a/messagefoundry/transports/dicom.py b/messagefoundry/transports/dicom.py index ed44c5189c..5541bb2687 100644 --- a/messagefoundry/transports/dicom.py +++ b/messagefoundry/transports/dicom.py @@ -119,6 +119,7 @@ NegativeAckError, SourceConnector, count_setting, + float_setting, peer_ip_allowed, positive_cap, register_destination, @@ -572,7 +573,9 @@ def __init__(self, config: Source) -> None: self._max_pdu_size = count_setting( s.get("max_pdu_size", 16384), knob="max_pdu_size", transport="DICOM-SCP source" ) - self._timeout = float(s.get("timeout_seconds", 30.0)) + self._timeout = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="DICOM-SCP source" + ) # Association-rate pacing (ASVS 2.4.1 / 15.2.2, BACKLOG #1114). The keys are NOT # max_messages_per_second/message_burst and are not read through mllp._pacing_settings, because # the UNIT here is an association, not a message — see _pace_association for why a message-rate @@ -1291,8 +1294,16 @@ def __init__(self, config: Destination) -> None: self._max_pdu_size = count_setting( s.get("max_pdu_size", 16384), knob="max_pdu_size", transport="DICOM-SCU destination" ) - self._timeout = float(s.get("timeout_seconds", 30.0)) - self._connect_timeout = float(s.get("connect_timeout", 10.0)) + self._timeout = float_setting( + s.get("timeout_seconds", 30.0), + knob="timeout_seconds", + transport="DICOM-SCU destination", + ) + self._connect_timeout = float_setting( + s.get("connect_timeout", 10.0), + knob="connect_timeout", + transport="DICOM-SCU destination", + ) # Build the client TLS context now so a bad cert/key fails at construction (check/dry-run), not # at the first delivery (like the SCP / MLLP / REST). #190 (ADR 0093): thread the instance [tls] # internal-CA trust-anchor policy so an internal hop that names no tls_ca_file of its own verifies diff --git a/messagefoundry/transports/dicomweb.py b/messagefoundry/transports/dicomweb.py index 24a8e7724c..d98b729d3b 100644 --- a/messagefoundry/transports/dicomweb.py +++ b/messagefoundry/transports/dicomweb.py @@ -52,6 +52,7 @@ DeliveryResponse, DestinationConnector, NegativeAckError, + float_setting, register_destination, ) from messagefoundry.transports.bounded_read import ( @@ -218,7 +219,9 @@ def __init__(self, config: Destination) -> None: # BACKLOG #1241: and the control-char screen alone is not enough for a value that becomes a # path SEGMENT — refuse at construction, where the operator sees it, not at the URL layer. _reject_non_uid(self.study_uid, "study_uid") - self.timeout: float = float(s.get("timeout_seconds", 30.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="DICOMweb destination" + ) self.encoding: str = s.get("encoding", "utf-8") # ADR 0013: capture the STOW-RS dicom+json response. Default False → returns None, byte-identical. self.capture_response: bool = bool(s.get("capture_response", False)) diff --git a/messagefoundry/transports/direct.py b/messagefoundry/transports/direct.py index 124c6c4ffe..63bbb2b3eb 100644 --- a/messagefoundry/transports/direct.py +++ b/messagefoundry/transports/direct.py @@ -81,6 +81,7 @@ DestinationConnector, NegativeAckError, encode_wire_body, + float_setting, register_destination, ) from messagefoundry.transports.email import checked_envelope @@ -264,7 +265,9 @@ def __init__(self, config: Destination) -> None: tls_ca_file = s.get("tls_ca_file") self.tls_ca_file: str | None = str(tls_ca_file) if tls_ca_file else None self.tls_check_hostname = bool(s.get("tls_check_hostname", True)) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="Direct destination" + ) self.encoding: str = str(s.get("encoding", "utf-8")) # S/MIME material — the whole point of Direct. Loaded and cross-checked at construction so a diff --git a/messagefoundry/transports/email.py b/messagefoundry/transports/email.py index b7153fb512..dd440e9767 100644 --- a/messagefoundry/transports/email.py +++ b/messagefoundry/transports/email.py @@ -80,6 +80,7 @@ DestinationConnector, NegativeAckError, encode_wire_body, + float_setting, register_destination, ) from messagefoundry.transports.mllp import InsecureHopGuard @@ -335,7 +336,9 @@ def __init__(self, config: Destination) -> None: tls_ca_file = s.get("tls_ca_file") self.tls_ca_file: str | None = str(tls_ca_file) if tls_ca_file else None self.tls_check_hostname = bool(s.get("tls_check_hostname", True)) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="Email destination" + ) self.encoding: str = str(s.get("encoding", "utf-8")) # STARTTLS-by-default posture, mirroring RestDestination's verify_tls / diff --git a/messagefoundry/transports/fhir.py b/messagefoundry/transports/fhir.py index d4a6061bb0..b068864a4b 100644 --- a/messagefoundry/transports/fhir.py +++ b/messagefoundry/transports/fhir.py @@ -71,6 +71,7 @@ DestinationConnector, NegativeAckError, encode_wire_body, + float_setting, register_destination, ) from messagefoundry.transports.bounded_read import ( @@ -560,7 +561,9 @@ def __init__(self, config: Destination) -> None: "(ASVS 14.2.1); see docs/SECURITY-LOOSENING.md", config.name, ) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="FHIR destination" + ) self.encoding: str = s.get("encoding", "utf-8") # ADR 0013: capture the FHIR server reply (assigned resource / ETag / OperationOutcome). Default # False → returns None, byte-identical. @@ -1309,7 +1312,11 @@ def __init__( ) refuse_url_credentials(url, f"FhirLookup {cname!r} 'url'") self._base[cname] = url - self._timeout[cname] = float(s.get("timeout_seconds", 30.0)) + self._timeout[cname] = float_setting( + s.get("timeout_seconds", 30.0), + knob="timeout_seconds", + transport=f"FhirLookup {cname!r}", + ) self._encoding[cname] = str(s.get("encoding", "utf-8")) # #200 (ADR 0092): the per-connection insecure-hop attestation keys the posture-keyed refusal. # Validated like the DB lookups': a flag written into this mutable dict without its diff --git a/messagefoundry/transports/file.py b/messagefoundry/transports/file.py index 726ee7092b..4589ef28bf 100644 --- a/messagefoundry/transports/file.py +++ b/messagefoundry/transports/file.py @@ -62,6 +62,7 @@ SourceConnector, SourceStartupError, encode_wire_body, + float_setting, intake_open, poll_interval, positive_cap, @@ -360,8 +361,8 @@ def _member_cap(value: object) -> int: ) if cap is None: # zero, in some spelling raise ValueError( - f"file source max_decompressed_members={value!r} must be above zero " - f"({_MEMBER_CAP_OFF_HINT})" + "file source max_decompressed_members must be above zero " + f"(value withheld; {_MEMBER_CAP_OFF_HINT})" ) return cap @@ -673,7 +674,9 @@ def __init__(self, config: Source) -> None: self._root_real = self.directory.resolve() self.pattern: str = s.get("pattern", "*") self.poll_seconds: float = poll_interval(s, default=1.0, transport="file source") - self.min_age_seconds: float = float(s.get("min_age_seconds", 0.0)) + self.min_age_seconds: float = float_setting( + s.get("min_age_seconds", 0.0), knob="min_age_seconds", transport="file source" + ) self.after_read: str = s.get("after_read", "move") # "move" | "delete" | "leave" (#142) if self.after_read not in ("move", "delete", "leave"): raise ValueError( diff --git a/messagefoundry/transports/http_auth.py b/messagefoundry/transports/http_auth.py index af8520c780..30258d5609 100644 --- a/messagefoundry/transports/http_auth.py +++ b/messagefoundry/transports/http_auth.py @@ -48,6 +48,7 @@ TrustAnchorPolicy, hop_name_prefix, ) +from messagefoundry.transports.base import float_setting from messagefoundry.transports.rest import ( HttpAuthError, ProxyConfig, @@ -265,8 +266,16 @@ def oauth2_cc_provider_from_settings( scope=(str(s["oauth2_scope"]) if s.get("oauth2_scope") else None), auth_style=str(s.get("oauth2_auth_style", "basic")), audience=(str(s["oauth2_audience"]) if s.get("oauth2_audience") else None), - expiry_skew_seconds=float(s.get("oauth2_expiry_skew_seconds", _DEFAULT_EXPIRY_SKEW)), - timeout_seconds=float(s.get("oauth2_timeout_seconds", _DEFAULT_TOKEN_TIMEOUT)), + expiry_skew_seconds=float_setting( + s.get("oauth2_expiry_skew_seconds", _DEFAULT_EXPIRY_SKEW), + knob="oauth2_expiry_skew_seconds", + transport="OAuth2", + ), + timeout_seconds=float_setting( + s.get("oauth2_timeout_seconds", _DEFAULT_TOKEN_TIMEOUT), + knob="oauth2_timeout_seconds", + transport="OAuth2", + ), # #200: the per-connection insecure-hop attestation keys the posture-keyed cleartext refusal in # __init__ (read from settings exactly as _dest_config / FhirLookup do). Default False → the hop # decides purely on posture. diff --git a/messagefoundry/transports/http_listener.py b/messagefoundry/transports/http_listener.py index 648765aebe..d767c580e8 100644 --- a/messagefoundry/transports/http_listener.py +++ b/messagefoundry/transports/http_listener.py @@ -56,6 +56,7 @@ ReplyOutcome, SourceConnector, positive_cap, + positive_seconds, register_source, wait_for_intake, ) @@ -75,6 +76,7 @@ "HttpRequest", "HttpRequestError", "HttpSource", + "reply_seconds", ] logger = logging.getLogger(__name__) @@ -627,11 +629,28 @@ def _header_cap(value: Any) -> int: ) if cap is None: # zero, in some spelling raise ValueError( - f"HTTP source max_header_bytes={value!r} must be above zero ({_HEADER_CAP_OFF_HINT})" + "HTTP source max_header_bytes must be above zero " + f"(value withheld; {_HEADER_CAP_OFF_HINT})" ) return cap +def reply_seconds(s: Mapping[str, Any], key: str) -> float: + """Read ``reply_timeout`` or ``reply_write_timeout``: seconds above zero, or 30 when unset. + + Unset, ``None`` and ``""`` take the 30-second default. Anything else is read by + :func:`~messagefoundry.transports.base.positive_seconds`, so it must be above zero, as + ``docs/CONNECTIONS.md`` says, because each bounds a blocked HTTP turn (BACKLOG #3432). The + ``float(s.get(key) or 30.0)`` this replaces read ``True`` as one second, took a NaN, and let + ``"0"`` through as a zero budget while a numeric ``0`` became the default. The runner reads + ``reply_timeout`` for its reply resolver through this function too, from the unresolved spec + settings; that agrees with this connector while the ``Http`` factory refuses a non-literal.""" + value = s.get(key) + if value is None or value == "": + return 30.0 + return positive_seconds(value, knob=key, transport="HTTP source") + + class HttpSource(SourceConnector): """Listen for inbound HTTP/1.1 requests, commit each POSTed body to the ingress stage via the pipeline handler, and return a ``202`` respond-with-receipt once it is durably committed (ADR 0023). @@ -729,11 +748,11 @@ def __init__(self, config: Source) -> None: # Synchronous captured-downstream reply (ADR 0154 D4). reply_from's presence is the mode # switch; absent, every path below is byte-identical to the shipped 202 slice (AC-8). self.reply_from: str | None = s.get("reply_from") or None - self.reply_timeout: float = float(s.get("reply_timeout") or 30.0) + self.reply_timeout: float = reply_seconds(s, "reply_timeout") self.reply_on_timeout: str = str(s.get("reply_on_timeout") or "504") self.reply_content_type: str = str(s.get("reply_content_type") or "passthrough") self.reply_on_empty: str = str(s.get("reply_on_empty") or "204") - self.reply_write_timeout: float = float(s.get("reply_write_timeout") or 30.0) + self.reply_write_timeout: float = reply_seconds(s, "reply_write_timeout") self._server: asyncio.Server | None = None self._handler: InboundHandler | None = None # vault BACKLOG #2606: the per-host cap SHIPS OFF here, unlike on the MLLP, raw-TCP and X12 @@ -1315,8 +1334,9 @@ def _drain_budget(self) -> float: defaults to 30 s against a 5 s shutdown grace, so ``stop()`` clamps this to its own sub-budget rather than letting one drain outlive the entire teardown by 6x. """ - if self.reply_from and self.reply_write_timeout: - return float(self.reply_write_timeout) + if self.reply_from: + # reply_seconds guarantees seconds above zero, so no falsy fallback is needed. + return self.reply_write_timeout return _CLIENT_SHUTDOWN_GRACE async def _write_safely(self, writer: asyncio.StreamWriter, data: bytes) -> None: diff --git a/messagefoundry/transports/mllp.py b/messagefoundry/transports/mllp.py index 72bcede590..737b28bb0f 100644 --- a/messagefoundry/transports/mllp.py +++ b/messagefoundry/transports/mllp.py @@ -87,6 +87,7 @@ InboundHandler, NegativeAckError, SourceConnector, + float_setting, intake_open, positive_cap, probe_tcp_reachable, @@ -750,8 +751,12 @@ def __init__(self, config: Destination) -> None: raise ValueError("MLLP destination requires a 'host' setting (the downstream peer)") self.host: str = host self.port: int = int(s["port"]) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) - self.connect_timeout: float = float(s.get("connect_timeout", 10.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="MLLP destination" + ) + self.connect_timeout: float = float_setting( + s.get("connect_timeout", 10.0), knob="connect_timeout", transport="MLLP destination" + ) self.encoding: str = s.get("encoding", "utf-8") # Per-outbound frame cap. This bounds ONLY the ACK-read decoder (the reply we read back); the # OUTGOING frame written by send() is deliberately UNCAPPED (frame() never truncates), so a @@ -1797,8 +1802,8 @@ def __init__(self, config: Source) -> None: # it: the unbounded table this control is not allowed to contain. Refuse at build, where # dry-run and `check` surface it, rather than at the first connection. raise ValueError( - "MLLP max_connections_per_host must be at least 1 (use None or 0 to disable the " - f"per-host cap), got {self.max_connections_per_host}" + "MLLP max_connections_per_host must be at least 1 (value withheld; use None or 0 to " + "disable the per-host cap)" ) self.receive_timeout: float | None = positive_cap( s.get("receive_timeout", DEFAULT_RECEIVE_TIMEOUT), @@ -1818,16 +1823,13 @@ def __init__(self, config: Source) -> None: knob="max_frame_seconds", transport="MLLP", ) - if self.max_frame_seconds is not None and not self.max_frame_seconds >= 0: - # Negative reaches here too, and would expire every frame the instant one opened — a + if self.max_frame_seconds is not None and self.max_frame_seconds < 0: + # Negative reaches here, and would expire every frame the instant one opened — a # listener that drops every sender, configured from a value the author meant as "off". - # Written `not >= 0` rather than `< 0` so NaN is refused as well: TOML can spell `nan`, - # `_cap_setting` cannot recognise it (`nan != 0`), and every comparison with it is false - # — so `min()` against the idle bound would silently drop the deadline and a bare - # `wait_for(..., nan)` would fire at once. + # `_cap_setting` has already refused a NaN (BACKLOG #3432). raise ValueError( - "MLLP max_frame_seconds must be a number of seconds, zero or more (use None or 0 to " - f"disable the frame deadline), got {self.max_frame_seconds}" + "MLLP max_frame_seconds must be a number of seconds, zero or more (value withheld; " + "use None or 0 to disable the frame deadline)" ) # BACKLOG #1725 act 3. Read through `positive_cap`, which also names the key when the value # is not a number; a negative one would let no frame be handled at all. diff --git a/messagefoundry/transports/remotefile.py b/messagefoundry/transports/remotefile.py index 6d8b7b1a8a..8bc5718bcf 100644 --- a/messagefoundry/transports/remotefile.py +++ b/messagefoundry/transports/remotefile.py @@ -114,6 +114,7 @@ SourceConnector, SourceStartupError, encode_wire_body, + float_setting, intake_open, poll_interval, positive_cap, @@ -935,7 +936,11 @@ def __init__( self._user = settings.get("username") self._password = settings.get("password") self._tls = tls - self._timeout = float(settings.get("connect_timeout", 30.0)) + self._timeout = float_setting( + settings.get("connect_timeout", 30.0), + knob="connect_timeout", + transport="REMOTEFILE ftp", + ) # Build the verifying TLS context once, fail-fast (build_check) — mirrors the SFTP host-key # posture: a verify-disabled ftps without the escape is refused here, not silently insecure. # #190 (ADR 0093): thread the instance [tls] internal-CA trust-anchor policy (verify path only). @@ -1604,7 +1609,11 @@ def __init__(self, settings: dict[str, Any]) -> None: # Read only to refuse it: a key_password never reaches paramiko (BACKLOG #2423). _refuse_sftp_key_wrap(self._private_key, settings.get("key_password")) self._known_hosts = settings.get("known_hosts") - self._timeout = float(settings.get("connect_timeout", 30.0)) + self._timeout = float_setting( + settings.get("connect_timeout", 30.0), + knob="connect_timeout", + transport="REMOTEFILE sftp", + ) # Fail fast at construction (build_check time): an unknown-host-key posture without the escape # must never silently weaken to auto-accept. #329: read the escape through the ADR-0092 clamp # (weakened_tls_escape_permitted_here consults the active construction posture, exactly like the diff --git a/messagefoundry/transports/rest.py b/messagefoundry/transports/rest.py index 910e07baf5..d96234da8b 100644 --- a/messagefoundry/transports/rest.py +++ b/messagefoundry/transports/rest.py @@ -76,6 +76,7 @@ DestinationConnector, NegativeAckError, encode_wire_body, + float_setting, register_destination, ) from messagefoundry.transports.bounded_read import ( @@ -1933,7 +1934,9 @@ def __init__(self, config: Destination) -> None: # BACKLOG #2060: names this hop in a bounded-read refusal, which never carries the URL. self._hop = hop_identity("REST", config.name) self.method: str = str(s.get("method", "POST")).upper() - self.timeout: float = float(s.get("timeout_seconds", 30.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="REST destination" + ) self.encoding: str = s.get("encoding", "utf-8") # ADR 0013: capture the HTTP response body. Default False → returns None, byte-identical. A 2xx # with a body → outcome='accepted'; a 2xx with an empty body → outcome='no_reply' (a successful diff --git a/messagefoundry/transports/smart.py b/messagefoundry/transports/smart.py index 37531faeb9..4b11377f3a 100644 --- a/messagefoundry/transports/smart.py +++ b/messagefoundry/transports/smart.py @@ -60,7 +60,7 @@ hop_name_prefix, ) from messagefoundry.redaction import json_loads_or_refusal -from messagefoundry.transports.base import DeliveryError +from messagefoundry.transports.base import DeliveryError, float_setting from messagefoundry.transports.bounded_read import ( MAX_TOKEN_RESPONSE_BYTES, EgressReplyError, @@ -689,8 +689,16 @@ def token_provider_from_settings( private_key_password=( str(s["smart_private_key_password"]) if s.get("smart_private_key_password") else None ), - expiry_skew_seconds=float(s.get("smart_expiry_skew_seconds", _DEFAULT_EXPIRY_SKEW)), - timeout_seconds=float(s.get("smart_timeout_seconds", _DEFAULT_TOKEN_TIMEOUT)), + expiry_skew_seconds=float_setting( + s.get("smart_expiry_skew_seconds", _DEFAULT_EXPIRY_SKEW), + knob="smart_expiry_skew_seconds", + transport="SMART", + ), + timeout_seconds=float_setting( + s.get("smart_timeout_seconds", _DEFAULT_TOKEN_TIMEOUT), + knob="smart_timeout_seconds", + transport="SMART", + ), # #200: the per-connection insecure-hop attestation keys the posture-keyed cleartext refusal in # __init__ (read from settings exactly as _dest_config / the OAuth2 provider do). attested=attested, diff --git a/messagefoundry/transports/soap.py b/messagefoundry/transports/soap.py index 9d5a9ae402..7c24f667a0 100644 --- a/messagefoundry/transports/soap.py +++ b/messagefoundry/transports/soap.py @@ -80,6 +80,7 @@ NegativeAckError, count_setting, encode_wire_body, + float_setting, register_destination, ) from messagefoundry.transports.bounded_read import ( @@ -331,7 +332,9 @@ def __init__(self, config: Destination) -> None: self.url = url # BACKLOG #2060: names this hop in a bounded-read refusal, which never carries the URL. self._hop = hop_identity("SOAP", config.name) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="SOAP destination" + ) self.encoding: str = s.get("encoding", "utf-8") self.version: str = str(s.get("soap_version", "1.1")) if self.version not in ("1.1", "1.2"): diff --git a/messagefoundry/transports/tcp.py b/messagefoundry/transports/tcp.py index 12b5c9dea6..f77d439bb7 100644 --- a/messagefoundry/transports/tcp.py +++ b/messagefoundry/transports/tcp.py @@ -38,6 +38,7 @@ DestinationConnector, InboundHandler, SourceConnector, + float_setting, positive_cap, probe_tcp_reachable, register_destination, @@ -152,8 +153,12 @@ def __init__(self, config: Destination) -> None: self.host: str = host self.port: int = int(s["port"]) self.codec = _codec_from_settings(s) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) - self.connect_timeout: float = float(s.get("connect_timeout", 10.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="TCP destination" + ) + self.connect_timeout: float = float_setting( + s.get("connect_timeout", 10.0), knob="connect_timeout", transport="TCP destination" + ) self.encoding: str = s.get("encoding", "utf-8") self.expect_reply: bool = bool(s.get("expect_reply", False)) self.max_frame_bytes: int | None = positive_cap( diff --git a/messagefoundry/transports/timer.py b/messagefoundry/transports/timer.py index f9b00e9894..435aa96cf2 100644 --- a/messagefoundry/transports/timer.py +++ b/messagefoundry/transports/timer.py @@ -29,6 +29,7 @@ InboundHandler, SourceConnector, intake_open, + positive_seconds, register_source, ) @@ -196,9 +197,11 @@ def __init__(self, config: Source) -> None: self._body_bytes = self.body.encode(self.encoding) self.run_once = bool(s.get("run_once", False)) iv = s.get("interval_seconds") - self.interval_seconds: float | None = float(iv) if iv is not None else None - if self.interval_seconds is not None and self.interval_seconds <= 0: - raise ValueError("timer source 'interval_seconds' must be > 0") + self.interval_seconds: float | None = ( + None + if iv is None + else positive_seconds(iv, knob="interval_seconds", transport="timer source") + ) # Cron / calendar schedule (ADR 0011 amendment, BACKLOG #160). Exclusive with interval/run_once; # DST is driven by the optional `timezone` (an IANA name → aware wall clock) or, by default, the # system-local wall clock (the OS handles local DST, no tzdata needed). diff --git a/messagefoundry/transports/x12.py b/messagefoundry/transports/x12.py index 687b48c13d..b3140747d2 100644 --- a/messagefoundry/transports/x12.py +++ b/messagefoundry/transports/x12.py @@ -49,6 +49,7 @@ NegativeAckError, SourceConnector, encode_wire_body, + float_setting, positive_cap, probe_tcp_reachable, register_destination, @@ -167,8 +168,12 @@ def __init__(self, config: Destination) -> None: self.host: str = host self.port: int = int(s["port"]) self.encoding: str = str(s.get("encoding", "utf-8")) - self.timeout: float = float(s.get("timeout_seconds", 30.0)) - self.connect_timeout: float = float(s.get("connect_timeout", 10.0)) + self.timeout: float = float_setting( + s.get("timeout_seconds", 30.0), knob="timeout_seconds", transport="X12 destination" + ) + self.connect_timeout: float = float_setting( + s.get("connect_timeout", 10.0), knob="connect_timeout", transport="X12 destination" + ) self.expect_reply: bool = bool(s.get("expect_reply", False)) # ADR 0016: capture the returned interchange (the 271/TA1) as a reply; ta1_required makes a # no-reply a retry. capture_response is forced True by reingress_to at wiring time (ADR 0013). diff --git a/tests/test_cap_setting_spellings.py b/tests/test_cap_setting_spellings.py index 5d58457ba7..ad3647a128 100644 --- a/tests/test_cap_setting_spellings.py +++ b/tests/test_cap_setting_spellings.py @@ -18,6 +18,7 @@ from __future__ import annotations import math +import operator from collections.abc import Callable from decimal import Decimal from fractions import Fraction @@ -34,21 +35,39 @@ ConnectionSpec, Database, DatabasePoll, + DatabaseRef, + Ftp, Http, Sftp, Soap, + WiringError, env, resolve_env_settings, ) from messagefoundry.transports import build_destination, build_source, database, mllp -from messagefoundry.transports.base import cap_setting, positive_cap, resolve_poll_ceiling +from messagefoundry.transports.base import ( + cap_setting, + count_setting, + float_setting, + poll_interval, + positive_cap, + resolve_poll_ceiling, +) from messagefoundry.transports.database import DatabaseLookupExecutor from messagefoundry.transports.dicom import DicomScpSource, DicomScuDestination +from messagefoundry.transports.dicomweb import DicomWebDestination +from messagefoundry.transports.direct import DirectDestination +from messagefoundry.transports.email import EmailDestination +from messagefoundry.transports.fhir import FhirDestination, FhirLookupExecutor from messagefoundry.transports.file import FileSource +from messagefoundry.transports.http_auth import oauth2_cc_provider_from_settings from messagefoundry.transports.http_listener import DEFAULT_MAX_HEADER_BYTES, HttpSource from messagefoundry.transports.mllp import MLLPDestination, MLLPSource from messagefoundry.transports.remotefile import RemoteFileSource +from messagefoundry.transports.rest import RestDestination +from messagefoundry.transports.smart import token_provider_from_settings from messagefoundry.transports.tcp import TcpDestination, TcpSource +from messagefoundry.transports.timer import TimerSource from messagefoundry.transports.x12 import X12Destination, X12Source Build = Callable[[dict[str, Any]], Any] @@ -212,7 +231,7 @@ def test_a_string_zero_burst_defaults_to_the_rate_like_a_number_zero( assert getattr(string_zero, burst_attr) == 100.0 assert getattr(build({rate_key: 100, key: "7"}), burst_attr) == 7.0 # the control # The refusal names the connector too, so a listener that stops passing its own name shows up. - with pytest.raises(ValueError, match=f"{transport} {key}="): + with pytest.raises(ValueError, match=rf"{transport} {key} must be above zero \(value withheld"): build({rate_key: 100, key: -1}) @@ -327,13 +346,16 @@ def test_positive_cap_refuses_what_is_left_after_zero_reads_as_off() -> None: for bad in (0.5, math.nan, "1e3", "lots", [1], math.inf): with pytest.raises(ValueError, match=r"^t k is not a valid int value \(value withheld\)"): positive_cap(bad, int, knob="k", transport="t") - # A whole negative is a range refusal, which still quotes the value. + # A whole negative is a range refusal, which withholds the value too (BACKLOG #3432). for negative in (-1, "-1"): - with pytest.raises(ValueError, match="t k="): + with pytest.raises(ValueError, match=r"^t k must be above zero \(value withheld"): positive_cap(negative, int, knob="k", transport="t") - for bad_seconds in (-0.5, "-0.5", math.nan): - with pytest.raises(ValueError, match="t k="): + for bad_seconds in (-0.5, "-0.5"): + with pytest.raises(ValueError, match=r"^t k must be above zero \(value withheld"): positive_cap(bad_seconds, float, knob="k", transport="t") + # A NaN is not a number of seconds at all, so it is refused before the range check. + with pytest.raises(ValueError, match=r"^t k is not a valid float value \(value withheld\)"): + positive_cap(math.nan, float, knob="k", transport="t") # === an integer cap is a whole count (BACKLOG #3431) ============================================== @@ -595,8 +617,9 @@ def test_an_integer_cap_reads_only_an_exact_zero_as_off() -> None: for not_zero in ("1e-400", "0e0", "0x0", "zero"): with pytest.raises(ValueError, match=r"^t k is not a valid int value"): cap_setting(not_zero, int, knob="k", transport="t") - # A float cap is not a count, so text float() reads as zero is still "off" there. - assert cap_setting("0e0", float, knob="k", transport="t") is None + # A float cap takes the same exact zeros as "off", and refuses the same exponent text (#3432). + with pytest.raises(ValueError, match=r"^t k is not a valid float value"): + cap_setting("0e0", float, knob="k", transport="t") @pytest.mark.parametrize("kind", [int, float]) @@ -614,3 +637,395 @@ def test_a_refusal_withholds_the_value(kind: type[int] | type[float]) -> None: assert sentinel not in str(caught.value) assert caught.value.__cause__ is None assert caught.value.__context__ is None + + +# === a float cap or timeout is a finite number (BACKLOG #3432) ===================================== + + +def _timer(over: dict[str, Any]) -> TimerSource: + return TimerSource(Source(type=ConnectorType.TIMER, settings={"body": "x", **over})) + + +def _ftp(over: dict[str, Any]) -> RemoteFileSource: + settings = {**Ftp(host="ftp.example.com", remote_dir="/in").settings, **over} + src = build_source( + Source(type=ConnectorType.REMOTEFILE, settings=settings), + egress=EgressSettings(deny_by_default=False), + ) + assert isinstance(src, RemoteFileSource) + return src + + +def _url_dest(cls: type[Any], ctype: ConnectorType, url: str) -> Build: + return lambda over: cls(Destination(name="OB_TEST", type=ctype, settings={"url": url, **over})) + + +_EMAIL_BASE = { + "host": "smtp.example.com", + "sender": "a@example.com", + "recipients": ["b@example.com"], +} + + +def _email(over: dict[str, Any]) -> EmailDestination: + return EmailDestination( + Destination(name="OB", type=ConnectorType.EMAIL, settings={**_EMAIL_BASE, **over}) + ) + + +def _fhir_lookup(over: dict[str, Any]) -> SimpleNamespace: + executor = FhirLookupExecutor( + {"epic": {"url": "https://fhir.example.com/r4", **over}}, + egress=EgressSettings(deny_by_default=False), + ) + return SimpleNamespace(timeout_seconds=executor._timeout["epic"]) + + +def _db_lookup(over: dict[str, Any]) -> SimpleNamespace: + executor = DatabaseLookupExecutor( + {"clarity": {"server": "db.example", "database": "d", **over}}, + egress=EgressSettings(deny_by_default=False), + ) + return SimpleNamespace(acquire_timeout=executor._acquire_timeout["clarity"]) + + +def _oauth2(over: dict[str, Any]) -> Any: + return oauth2_cc_provider_from_settings( + { + "oauth2_token_url": "https://auth.example.com/token", + "oauth2_client_id": "c", + "oauth2_client_secret": "s", + **over, + } + ) + + +#: Float caps where ``None``/``0`` turns the cap off: (builder, key, attribute). At least one per +#: connector that reads one. +_FLOAT_OFF_BY_ZERO = [ + pytest.param(_MLLP_IN, "receive_timeout", "receive_timeout", id="mllp-in-receive_timeout"), + pytest.param(_MLLP_IN, "max_frame_seconds", "max_frame_seconds", id="mllp-in-frame_seconds"), + pytest.param(_MLLP_IN, _RATE, _RATE, id="mllp-rate"), + pytest.param(_MLLP_OUT, "idle_timeout_seconds", "idle_timeout_seconds", id="mllp-out-idle"), + pytest.param(_TCP_IN, "receive_timeout", "receive_timeout", id="tcp-in-receive_timeout"), + pytest.param(_TCP_IN, "max_frame_seconds", "max_frame_seconds", id="tcp-in-frame_seconds"), + pytest.param(_TCP_OUT, "idle_timeout_seconds", "idle_timeout_seconds", id="tcp-out-idle"), + pytest.param(_X12_IN, "receive_timeout", "receive_timeout", id="x12-in-receive_timeout"), + pytest.param(_X12_IN, "max_frame_seconds", "max_frame_seconds", id="x12-in-frame_seconds"), + pytest.param( + _X12_OUT, "max_connection_age_seconds", "max_connection_age_seconds", id="x12-out-age" + ), + pytest.param(_HTTP_IN, "receive_timeout", "receive_timeout", id="http-in-receive_timeout"), + pytest.param(_HTTP_IN, _RATE, _RATE, id="http-rate"), + pytest.param( + _SCP, + "max_associations_per_second", + "max_associations_per_second", + id="scp-association-rate", + ), +] + +#: Timeouts read by ``float_setting`` alone, where zero is not "off": (builder, key, attribute). +#: Each was a bare ``float()`` before #3432. +_TIMEOUTS = [ + pytest.param(_MLLP_OUT, "timeout_seconds", "timeout", id="mllp-out-timeout"), + pytest.param(_MLLP_OUT, "connect_timeout", "connect_timeout", id="mllp-out-connect"), + pytest.param(_TCP_OUT, "timeout_seconds", "timeout", id="tcp-out-timeout"), + pytest.param(_TCP_OUT, "connect_timeout", "connect_timeout", id="tcp-out-connect"), + pytest.param(_X12_OUT, "timeout_seconds", "timeout", id="x12-out-timeout"), + pytest.param(_X12_OUT, "connect_timeout", "connect_timeout", id="x12-out-connect"), + pytest.param(_SCP, "timeout_seconds", "_timeout", id="scp-timeout"), + pytest.param(_SCU, "timeout_seconds", "_timeout", id="scu-timeout"), + pytest.param(_SCU, "connect_timeout", "_connect_timeout", id="scu-connect"), + pytest.param(_database_out, "acquire_timeout", "_acquire_timeout", id="database-out-acquire"), + pytest.param(_database, "acquire_timeout", "_acquire_timeout", id="database-in-acquire"), + pytest.param(_db_lookup, "acquire_timeout", "acquire_timeout", id="database-lookup-acquire"), + pytest.param( + _url_dest(FhirDestination, ConnectorType.FHIR, "https://fhir.example.com/r4"), + "timeout_seconds", + "timeout", + id="fhir-timeout", + ), + pytest.param(_fhir_lookup, "timeout_seconds", "timeout_seconds", id="fhir-lookup-timeout"), + pytest.param( + _url_dest(RestDestination, ConnectorType.REST, "https://api.example.com/x"), + "timeout_seconds", + "timeout", + id="rest-timeout", + ), + pytest.param(_soap, "timeout_seconds", "timeout", id="soap-timeout"), + pytest.param( + _url_dest(DicomWebDestination, ConnectorType.DICOMWEB, "https://pacs.example.com/studies"), + "timeout_seconds", + "timeout", + id="dicomweb-timeout", + ), + pytest.param(_email, "timeout_seconds", "timeout", id="email-timeout"), + pytest.param(_remote, "connect_timeout", "_client._timeout", id="sftp-connect"), + pytest.param(_ftp, "connect_timeout", "_client._timeout", id="ftp-connect"), + pytest.param(_file, "min_age_seconds", "min_age_seconds", id="file-min_age"), + pytest.param(_timer, "interval_seconds", "interval_seconds", id="timer-interval"), + pytest.param(_HTTP_IN, "reply_timeout", "reply_timeout", id="http-reply_timeout"), + pytest.param(_HTTP_IN, "reply_write_timeout", "reply_write_timeout", id="http-reply_write"), + pytest.param(_oauth2, "oauth2_timeout_seconds", "timeout_seconds", id="oauth2-timeout"), +] + +#: Each is refused by every float setting. ``"1e-400"`` is text ``float()`` only rounds to zero; +#: ``[2.5]`` is a type ``float()`` refuses anyway, refused here naming the key. +_NOT_SECONDS = [True, False, math.nan, math.inf, -math.inf, "nan", "inf", "1e400", "1e-400", [2.5]] + + +def _refusal(key: str) -> str: + return rf"{key} is not a valid float value \(value withheld\)" + + +@pytest.mark.parametrize(("build", "key", "attr"), _FLOAT_OFF_BY_ZERO + _TIMEOUTS) +@pytest.mark.parametrize("bad", _NOT_SECONDS) +def test_a_float_setting_refuses_a_bool_a_nan_an_infinity_or_a_rounded_zero( + build: Build, key: str, attr: str, bad: object +) -> None: + """``float(True)`` is 1.0, ``float("nan")`` is a timeout that fires at once, and + ``float("1e-400")`` is ``0.0``, which turned a float cap off. Each is refused, naming the key. + + Red mutation: put a bare ``float()`` back on any ``_TIMEOUTS`` site, or test a float cap's text + with ``float(value) == 0`` again. That site's cases build cleanly.""" + del attr + with pytest.raises(ValueError, match=_refusal(key)): + build({key: bad}) + + +@pytest.mark.parametrize(("build", "key", "attr"), _FLOAT_OFF_BY_ZERO + _TIMEOUTS) +def test_a_float_setting_accepts_a_number_or_its_text(build: Build, key: str, attr: str) -> None: + """The control for the refusal above: a number, its text from an uncast ``env()``, and a + ``Decimal`` all read as the number.""" + for good in (2.5, "2.5", " 2.5 ", Decimal("2.5")): + assert operator.attrgetter(attr)(build({key: good})) == 2.5, repr(good) + + +@pytest.mark.parametrize(("build", "key", "attr"), _FLOAT_OFF_BY_ZERO) +def test_a_float_cap_reads_only_an_exact_zero_as_off(build: Build, key: str, attr: str) -> None: + """``0``, ``"0"``, ``"0.0"`` and ``"-0"`` turn the cap off. ``"0e0"`` is exponent text, refused as + ``"1e-400"`` is, so no spelling that merely rounds to zero switches a guard off.""" + for zero in (0, 0.0, "0", " 0.0 ", "-0", None, ""): + assert getattr(build({key: zero}), attr) is None, repr(zero) + with pytest.raises(ValueError, match=_refusal(key)): + build({key: "0e0"}) + + +def test_the_direct_and_smart_timeouts_refuse_a_bool() -> None: + """The Direct destination and the SMART token provider load key material after reading the + timeout, so only the refusal is driven here; the reader is the one the tables above pin.""" + direct_base = { + **_EMAIL_BASE, + "signing_cert": "unused", + "signing_key": "unused", + "recipient_cert": "unused", + "trust_anchor": "unused", + } + for bad in (True, False, math.nan, "1e-400"): + with pytest.raises(ValueError, match=_refusal("timeout_seconds")): + DirectDestination( + Destination( + name="OB", + type=ConnectorType.DIRECT, + settings={**direct_base, "timeout_seconds": bad}, + ) + ) + with pytest.raises(ValueError, match=_refusal("smart_timeout_seconds")): + token_provider_from_settings( + { + "smart_token_url": "https://auth.example.com/token", + "smart_client_id": "c", + "smart_private_key": "unused", + "smart_timeout_seconds": bad, + } + ) + + +async def test_the_reference_sync_acquire_timeout_is_refused_before_a_pool_is_made( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Read after the pool was made, a refusal would have left that pool unclosed.""" + from messagefoundry.pipeline.reference_sync import _load_database_source + + async def _no_pool(*args: object, **kwargs: object) -> object: + raise AssertionError("the timeout should have been refused before a pool was made") + + monkeypatch.setattr(database, "_make_pool", _no_pool) + base = {"server": "db.example", "database": "d", "statement": "SELECT a", "key_column": "a"} + for bad in (True, False, math.nan, "inf", "1e-400"): + with pytest.raises(ValueError, match=_refusal("reference source acquire_timeout")): + await _load_database_source( + {**base, "acquire_timeout": bad}, EgressSettings(deny_by_default=False) + ) + + +def test_an_http_reply_timeout_must_be_above_zero() -> None: + """``docs/CONNECTIONS.md`` says each must be positive. A numeric ``0`` used to become the + default while ``"0"`` became a zero budget; both are refused now. Unset takes 30 seconds.""" + for key in ("reply_timeout", "reply_write_timeout"): + for bad in (0, "0", -1, "-0.5"): + with pytest.raises(ValueError, match=rf"HTTP source {key} must be a number of seconds"): + _HTTP_IN({key: bad}) + for unset in (None, ""): + assert getattr(_HTTP_IN({key: unset}), key) == 30.0 + assert getattr(_HTTP_IN({}), key) == 30.0 + + +_RANGE_REFUSALS = [ + pytest.param(lambda v: positive_cap(v, int, knob="k", transport="t"), -7, id="positive_cap"), + pytest.param(lambda v: positive_cap(v, float, knob="k", transport="t"), -7.5, id="float_cap"), + pytest.param(lambda v: resolve_poll_ceiling(v, knob="k", transport="t"), -7, id="poll_ceiling"), + pytest.param( + lambda v: poll_interval({"poll_seconds": v}, default=1.0, transport="t"), + -7.5, + id="poll_interval", + ), + pytest.param(lambda v: _HTTP_IN({"max_header_bytes": v}), "00", id="max_header_bytes"), + pytest.param(lambda v: _file({"max_decompressed_members": v}), "00", id="max_members"), + pytest.param(lambda v: _MLLP_IN({"max_connections_per_host": v}), -7, id="mllp-per_host"), + pytest.param(lambda v: _MLLP_IN({"max_frame_seconds": v}), -7.5, id="mllp-frame_seconds"), + pytest.param(lambda v: _HTTP_IN({"reply_timeout": v}), -7.5, id="http-reply_timeout"), +] + + +@pytest.mark.parametrize(("refuse", "value"), _RANGE_REFUSALS) +def test_a_range_refusal_withholds_the_value(refuse: Callable[[Any], Any], value: Any) -> None: + """A setting arrives ``env()``-resolved, so a range refusal keeps the value out of its message, + as the type refusals already did (BACKLOG #1183, #1796). + + Red mutation: put ``{value!r}`` back in any of these refusals.""" + with pytest.raises(ValueError) as caught: + refuse(value) + message = str(caught.value) + assert "value withheld" in message, message + assert str(value) not in message, message + + +def test_float_setting_states_one_rule() -> None: + """The reader itself, once: exact zeros come back as zero, and every value that is not a finite + real number is refused without its value.""" + for zero in (0, 0.0, -0.0, "0", "-0.0", Decimal(0)): + assert float_setting(zero, knob="k", transport="t") == 0.0, repr(zero) + assert float_setting(Fraction(5, 2), knob="k", transport="t") == 2.5 + assert float_setting(-3, knob="k", transport="t") == -3.0 + for bad in (Decimal("1e-400"), Decimal("NaN"), 10**400, complex(1, 0), b"2.5", None): + with pytest.raises(ValueError, match=r"^t k is not a valid float value \(value withheld\)"): + float_setting(bad, knob="k", transport="t") + with pytest.raises(ValueError) as caught: + float_setting("SENTINEL-3432", knob="k", transport="t") + assert "SENTINEL-3432" not in str(caught.value) + assert caught.value.__context__ is None + # A number too large for a float says so, rather than calling it not a number. + with pytest.raises(ValueError, match="too large for a float"): + float_setting(10**400, knob="k", transport="t") + + +@pytest.mark.parametrize("key", ["reply_timeout", "reply_write_timeout"]) +@pytest.mark.parametrize("bad", [True, math.nan, math.inf, -7.25, 0]) +def test_the_http_factory_refuses_what_the_connector_refuses(key: str, bad: object) -> None: + """The ``Http`` factory checks the two reply timeouts at wiring. ``isinstance(v, int | float)`` + let a bool through, and ``v <= 0`` let NaN and infinity through, so the factory and the connector + disagreed. The refusal also withholds the value now. + + Red mutation: drop the bool or the ``math.isfinite`` clause from the factory check.""" + overrides: dict[str, Any] = {key: bad} + with pytest.raises(WiringError) as caught: + Http(port=0, reply_from="OB_REPLY", **overrides) + message = str(caught.value) + assert f"Http {key} must be a finite positive number of seconds (value withheld)" in message + assert "-7.25" not in message + + +def test_the_http_factory_refuses_an_int_too_large_for_a_float_by_name() -> None: + """``math.isfinite(10**400)`` raises ``OverflowError``, so the factory raised that raw error, naming + no key. A long ``connections.toml`` integer reaches the same check through the desugar. + + Red mutation: drop the factory's ``float(value)`` overflow guard. ``OverflowError`` escapes.""" + for key in ("reply_timeout", "reply_write_timeout"): + overrides: dict[str, Any] = {key: 10**400} + with pytest.raises(WiringError, match=rf"^Http {key} .*too large for a float") as caught: + Http(port=0, reply_from="OB_REPLY", **overrides) + assert "0" * 20 not in str(caught.value) + + +#: Exact zeros that ``_ZERO_TEXT`` does not match. ``int()`` reads the whole-number ones as ``0``, so +#: an ``int`` cap already took them as off; a float setting called them "rounded to zero". +_ODD_ZERO_TEXT = ["0_0", "0", "-0_0.0_0", "0.0"] + + +def test_a_float_setting_reads_every_exact_zero_spelling_as_zero() -> None: + """Text whose digits are all zeros, with no exponent, is exactly zero, so it reads as zero and a + float cap reads it as off, as an ``int`` cap does. + + Red mutation: test text against ``_ZERO_TEXT`` alone in ``_spells_zero``. Each case is refused.""" + for zero in _ODD_ZERO_TEXT: + assert float_setting(zero, knob="k", transport="t") == 0.0, repr(zero) + assert cap_setting(zero, float, knob="k", transport="t") is None, repr(zero) + for zero in ("0_0", "0"): + assert count_setting(zero, knob="k", transport="t") == 0, repr(zero) + assert cap_setting(zero, int, knob="k", transport="t") is None, repr(zero) + + +def test_a_float_setting_refusal_of_a_zero_reading_states_a_true_reason() -> None: + """What ``float()`` reads as zero but is refused is either exponent text or not zero at all, such + as a plain decimal too small for a float. The reason says so, and claims no rounding of an exact + zero. + + Red mutation: restore "float() rounds it to zero, but it is not zero". The match fails.""" + tiny_decimal = "0." + "0" * 400 + "1" + for bad in ("0e0", "1e-400", tiny_decimal, Decimal("1e-400")): + with pytest.raises(ValueError, match="it is not an exact zero in plain digits$"): + float_setting(bad, knob="k", transport="t") + + +@pytest.mark.parametrize("bad", [True, False, math.nan, "inf", "1e-400", [2.5]]) +def test_database_ref_refuses_a_bad_acquire_timeout_when_declared(bad: object) -> None: + """A reference set is first synced after start, so a bool or a NaN refused only at sync passed + ``messagefoundry check``. ``DatabaseRef`` checks it when declared, as it checks ``connect_timeout``. + + Red mutation: drop the declaration check. ``DatabaseRef`` returns a spec.""" + overrides: dict[str, Any] = {"acquire_timeout": bad} + with pytest.raises(WiringError, match=_refusal("reference source acquire_timeout")): + DatabaseRef(server="s", database="d", statement="SELECT a", key_column="a", **overrides) + + +def test_database_ref_accepts_a_number_or_an_env_ref_for_acquire_timeout() -> None: + """The control: a number and its text are declared, and an ``env()`` ref is left for the sync.""" + for good in (12.5, "12.5", 0, env("MEFOR_TEST_3432_ACQUIRE")): + overrides: dict[str, Any] = {"acquire_timeout": good} + spec = DatabaseRef( + server="s", database="d", statement="SELECT a", key_column="a", **overrides + ) + assert spec.settings["acquire_timeout"] is good + + +def test_the_build_check_refuses_an_env_acquire_timeout_the_sync_would_refuse() -> None: + """``DatabaseRef`` cannot read an ``env()`` ref, so ``build_check_registry`` reads it once + resolved, by the sync's own reader. Before, ``messagefoundry check`` passed it. + + Red mutation: drop the ``reference_acquire_timeout`` call from ``build_check_registry``.""" + from messagefoundry.config.wiring import ReferenceSpec, Registry + from messagefoundry.pipeline.wiring_runner import build_check_registry + + registry = Registry() + source = DatabaseRef( + server="db.example.org", + database="d", + statement="SELECT a", + key_column="a", + acquire_timeout=env("MEFOR_TEST_3432_ACQUIRE"), # type: ignore[arg-type] + ) + registry.references["codes"] = ReferenceSpec(name="codes", source=source) + + def check(value: str) -> None: + build_check_registry( + registry, + inbound_bind_host="127.0.0.1", + env_values={"mefor_test_3432_acquire": value}, + egress=EgressSettings(deny_by_default=False), + ) + + with pytest.raises(ValueError, match=_refusal("reference source acquire_timeout")): + check("nan") + check("12.5") # the control: a number passes diff --git a/tests/test_connection_event_emit.py b/tests/test_connection_event_emit.py index 99b5c4bc9b..b2eba010ca 100644 --- a/tests/test_connection_event_emit.py +++ b/tests/test_connection_event_emit.py @@ -671,9 +671,8 @@ def test_a_negative_cap_is_refused_at_build_not_discovered_at_the_first_connecti ): _mllp(max_frame_seconds=-1.0) # NaN is truthy and fails every comparison, so a `< 0` test let it through as a live deadline. - with pytest.raises( - ValueError, match="max_frame_seconds must be a number of seconds, zero or more" - ): + # The cap reader now refuses it before that guard runs (BACKLOG #3432). + with pytest.raises(ValueError, match=r"max_frame_seconds is not a valid float value"): _mllp(max_frame_seconds=float("nan")) # 0 stays the documented "off" spelling for both, and is NOT caught by the refusal above. assert _mllp(max_connections_per_host=0).max_connections_per_host is None diff --git a/tests/test_dicom_scp_disposition.py b/tests/test_dicom_scp_disposition.py index e574794b4b..5a43b9dafd 100644 --- a/tests/test_dicom_scp_disposition.py +++ b/tests/test_dicom_scp_disposition.py @@ -251,7 +251,7 @@ def test_a_negative_cap_is_refused_with_the_scps_own_text(bad: object) -> None: with pytest.raises(ValueError) as caught: _scp(bad) # type: ignore[arg-type] message = str(caught.value) - assert "DICOM-SCP source max_object_bytes=" in message + assert "DICOM-SCP source max_object_bytes must be above zero (value withheld" in message assert "disable" not in message.replace("does not disable", "") assert "does not disable" in message assert str(ingress_guards.INGRESS_MAX_BYTES) in message diff --git a/tests/test_poll_seconds_validation.py b/tests/test_poll_seconds_validation.py index 760c5bf568..d1ce22a39b 100644 --- a/tests/test_poll_seconds_validation.py +++ b/tests/test_poll_seconds_validation.py @@ -68,7 +68,7 @@ def _database(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, value: Any) -> An @pytest.mark.parametrize("source", list(_SOURCES)) @pytest.mark.parametrize( "value", - [0, 0.0, -1, -0.5, math.nan, math.inf, "0", "nan", "soon"], + [0, 0.0, -1, -0.5, math.nan, math.inf, "0", "nan", "soon", True, False, "1e-400"], ids=[ "zero", "zero-float", @@ -79,13 +79,16 @@ def _database(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, value: Any) -> An "zero-text", "nan-text", "not-a-number", + "true", + "false", + "rounds-to-zero", ], ) def test_a_poll_interval_that_is_not_a_finite_positive_number_is_refused( source: str, value: Any, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: build, _, _ = _SOURCES[source] - with pytest.raises(ValueError, match="poll_seconds="): + with pytest.raises(ValueError, match="poll_seconds "): build(tmp_path, monkeypatch, value) @@ -104,5 +107,5 @@ def test_the_refusal_names_the_source_and_says_what_is_wanted() -> None: with pytest.raises(ValueError) as caught: poll_interval({"poll_seconds": 0}, default=1.0, transport="file source") assert str(caught.value) == ( - "file source poll_seconds=0 must be a finite number of seconds above zero" + "file source poll_seconds must be a number of seconds above zero (value withheld)" ) diff --git a/tests/test_timer_source.py b/tests/test_timer_source.py index 6c7a4e140d..b3ef824062 100644 --- a/tests/test_timer_source.py +++ b/tests/test_timer_source.py @@ -254,7 +254,7 @@ def test_timer_requires_a_schedule() -> None: def test_timer_rejects_nonpositive_interval() -> None: - with pytest.raises(ValueError, match="must be > 0"): + with pytest.raises(ValueError, match="interval_seconds must be a number of seconds above zero"): TimerSource(Source(type=ConnectorType.TIMER, settings={"body": "X", "interval_seconds": 0}))