From 5d346d9523d26a340b2922bf17c65c8e7fce908f Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Fri, 21 Aug 2026 16:49:45 -0500 Subject: [PATCH 1/3] [py] align generated BiDi layer with the low-level contract and drop ADR cross-references --- py/generate_bidi_protocol.py | 22 ++++++--- .../webdriver/common/_bidi/serialization.py | 48 ++++++++++++------- .../webdriver/common/bidi/protocol_tests.py | 4 +- .../common/bidi_protocol_command_tests.py | 4 +- .../common/bidi_serialization_tests.py | 34 ++++++++----- 5 files changed, 74 insertions(+), 38 deletions(-) diff --git a/py/generate_bidi_protocol.py b/py/generate_bidi_protocol.py index 2ed2b25191c46..7579891da0d02 100644 --- a/py/generate_bidi_protocol.py +++ b/py/generate_bidi_protocol.py @@ -235,6 +235,7 @@ class UnionIR: object_only: bool = False spec_href: str | None = None scalar_type: str | None = None # Literal[...] for an alias-union's bare-scalar arms + scalar_values: list[Any] | None = None # the literals a bare-scalar arm admits @dataclass @@ -345,8 +346,8 @@ def _record_ir(self, name: str, type_: dict) -> RecordIR: fields=fields, discriminator=discriminator, # A type the spec marks extensible carries an untyped map for the fields the spec does - # not declare (ADR decision 1); every extensible type keeps them, received-only ones - # included (ADR decision 9). A non-extensible type gets no store. + # not declare; every extensible type keeps them, received-only ones + # included. A non-extensible type gets no store. extensible=bool(type_.get("extensible")), spec_href=type_.get("specHref"), ) @@ -358,7 +359,7 @@ def _field_ir(self, field_: dict) -> FieldIR: resolved = self._resolve(field_["type"]) # Carry a const literal whether or not it is nullable. A non-nullable const is a baked # discriminator (forced, init=False); a nullable const (e.g. ``bypass: true / null``) is a - # settable field held to its literal-or-null by the runtime (ADR decision 4, by vocabulary). + # settable field held to its literal-or-null by the runtime. const = field_["type"].get("const", _NO_FIXED) py, refs = self.py_type(field_["type"]) self._pending_refs |= refs @@ -392,6 +393,11 @@ def _union_ir(self, name: str) -> UnionIR: ir = self._union_from_selector(name, type_["selector"]) else: ir = self._union_from_alias(name) + # A non-object_only union has a bare-scalar arm; only const-literal arms (scalarValues) + # are modeled, so the runtime can validate an outbound scalar. Without them the runtime + # would accept any scalar, so fail here, at generation, not at a caller's runtime. + if not ir.object_only and not ir.scalar_values: + raise ValueError(f"non-object_only union {name} has no scalarValues to validate its bare-scalar arm") for v in ir.variants: py = self._py_ref(v.ref, self._pending_refs) ir.variant_types.append(py) @@ -460,10 +466,8 @@ def _union_from_alias(self, name: str) -> UnionIR: # projector leaves it unflagged and a non-object payload still passes through. Those arms # have no ref, so they are absent from the record variants above; surface them in the value # alias as a Literal[...] so a caller sees the scalar options, not only the record variant. - scalar_consts = [ - arm["const"] for arm in self.types[name]["type"]["union"] if "const" in arm and "ref" not in arm - ] - scalar_type = f"Literal[{', '.join(repr(c) for c in scalar_consts)}]" if scalar_consts else None + scalar_values = self.types[name]["type"].get("scalarValues") or [] + scalar_type = f"Literal[{', '.join(repr(c) for c in scalar_values)}]" if scalar_values else None object_only = bool(self.types[name].get("objectOnly")) return UnionIR( type_class_name(name), @@ -475,6 +479,7 @@ def _union_from_alias(self, name: str) -> UnionIR: object_only=object_only, spec_href=self.types[name].get("specHref"), scalar_type=scalar_type, + scalar_values=scalar_values, ) def params_for(self, params_ref: dict | None) -> list[ParamIR] | None: @@ -885,6 +890,9 @@ def _emit_union(u: UnionIR) -> str: lines.append(f" _DISCRIMINATOR_VALUES = {frozenset_lit(values, 4)}") if u.object_only: lines.append(" _OBJECT_ONLY = True") + if u.scalar_values: + values = [lit(v) for v in u.scalar_values] + lines.append(f" _SCALAR_VALUES = {frozenset_lit(values, 4)}") alias = _emit_type_alias(value_alias(u.schema_name), u.variant_types) return "\n".join(lines) + "\n\n\n" + alias diff --git a/py/selenium/webdriver/common/_bidi/serialization.py b/py/selenium/webdriver/common/_bidi/serialization.py index ebbe6b21eb205..f010168e8ec32 100644 --- a/py/selenium/webdriver/common/_bidi/serialization.py +++ b/py/selenium/webdriver/common/_bidi/serialization.py @@ -257,7 +257,7 @@ def __post_init__(self) -> None: value = getattr(self, f.name) if w.fixed is not UNSET: # A baked discriminator (non-nullable) is forced to its const. A nullable constant - # is settable and must be its literal or null (ADR decision 4, by vocabulary). + # is settable and must be its literal or null. if w.nullable and value is not UNSET and value is not None and value != w.fixed: raise BiDiSerializationError( f"{type(self).__name__}.{f.name}: {value!r} must be {w.fixed!r} or None" @@ -294,7 +294,7 @@ def as_json(self) -> dict: declared.add(w.wire) value = getattr(self, f.name) if value is UNSET: - # Outbound requires every required field (ADR decision 1). Enforced here at + # Outbound requires every required field. Enforced here at # the boundary, not in the constructor, so the object stays permissive and # inbound can tolerate the same field being absent (see from_json). if w.required: @@ -306,7 +306,7 @@ def as_json(self) -> dict: payload[w.wire] = _as_json(value) if self._EXTENSIBLE: extras = getattr(self, "extensions", None) or {} - # A key the type declares must never appear in the extras map (ADR decision 1), so an + # A key the type declares must never appear in the extras map, so an # extra can never shadow a declared field on the wire — whether or not that field is set. shadowed = [k for k in extras if k in declared] if shadowed: @@ -338,9 +338,9 @@ def from_json(cls, payload: dict) -> Any: continue kwargs[f.name] = _read_field(cls, f.name, w, payload) undeclared = [k for k in payload if k not in known] - # A missing required field is tolerated and left unset (ADR decision 8). An undeclared field + # A missing required field is tolerated and left unset. An undeclared field # on an extensible type is spec-sanctioned, not a deviation: it is preserved in the extras - # map silently (ADR decision 1/9). On a non-extensible type it is a deviation: warn and drop. + # map silently. On a non-extensible type it is a deviation: warn and drop. # Each tolerated kind warns at most once per record — never once per key, so a verbose payload # cannot flood the log; strict_inbound escalates a genuine deviation to an error. if missing_required: @@ -401,15 +401,23 @@ def _check_scalar(cls: type, name: str, w: _Wire, value: Any) -> Any: raise BiDiSerializationError(f"{cls.__name__}.{name}: map key expected {' or '.join(scalars)}, got {got} {value!r}") -# JSON value -> the Python types a schema primitive accepts. ``integer`` accepts only an -# int (a non-integer float like 1.5 is a real mismatch, and even 5.0 is rejected under -# strict-first — relax reactively if a browser is ever seen sending it). ``number`` accepts -# an int or a float. ``bool`` is excluded from the numeric checks: it is an ``int`` subclass -# but is not a number. +def _is_whole(value: Any) -> bool: + # A browser is free to encode a whole number either way (JS has no int/float split), so + # ``5`` and ``5.0`` are both integers on the wire; only a fractional value is a mismatch. + if isinstance(value, bool): + return False + if isinstance(value, int): + return True + return isinstance(value, float) and value.is_integer() + + +# JSON value -> the Python types a schema primitive accepts. A primitive matches by JSON kind, +# not Python type: ``integer`` admits any whole number, ``number`` an int or a float. ``bool`` +# is excluded from the numeric checks: it is an ``int`` subclass but is not a number. _PRIMITIVE_CHECKS = { "str": lambda v: isinstance(v, str), "bool": lambda v: isinstance(v, bool), - "int": lambda v: isinstance(v, int) and not isinstance(v, bool), + "int": _is_whole, "float": lambda v: isinstance(v, (int, float)) and not isinstance(v, bool), } @@ -417,7 +425,7 @@ def _check_scalar(cls: type, name: str, w: _Wire, value: Any) -> Any: def _read_scalar(cls: type, name: str, w: _Wire, raw: Any) -> Any: if w.fixed is not UNSET: # A nullable constant (a non-nullable one is baked and never read): the wire value must be - # the literal, else it is invalid (ADR decision 4, by vocabulary). A null took the nullable + # the literal, else it is invalid. A null took the nullable # path in _read_field before reaching here. if raw != w.fixed: raise BiDiSerializationError(f"{cls.__name__}.{name}: {raw!r} is not the constant {w.fixed!r}") @@ -441,11 +449,14 @@ def _read_scalar(cls: type, name: str, w: _Wire, raw: Any) -> Any: if check and not check(raw): got = type(raw).__name__ raise BiDiSerializationError(f"{cls.__name__}.{name}: expected {w.primitive}, got {got} {raw!r}") + if w.primitive == "int" and isinstance(raw, float): + # A whole number is exact in both types, so the declared type is held with nothing lost. + return int(raw) return raw def _validate_outbound(owner: str, name: str, w: _Wire, value: Any) -> None: - """Reject an outbound value that violates its wire type before it is sent (ADR decision 1). + """Reject an outbound value that violates its wire type before it is sent. A caller mistake — a wrong primitive, a scalar where a list is expected, a raw dict where a typed record belongs — surfaces here as a local error rather than a remote protocol error. @@ -535,6 +546,7 @@ class Union: _FALLBACK: str | None = None _DISCRIMINATOR_VALUES: frozenset[Any] | None = None _OBJECT_ONLY: bool = False + _SCALAR_VALUES: frozenset = frozenset() _VARIANT_CLASSES: tuple[type, ...] | None = None # per-subclass cache; see _variant_classes @classmethod @@ -576,10 +588,11 @@ def collect(union: type[Union]) -> None: @classmethod def validate_outbound(cls, owner: str, name: str, value: Any) -> None: - """Reject an outbound value that is not one of this union's variants (ADR decisions 4-5). + """Reject an outbound value that is not one of this union's variants. - A variant instance passes. A bare scalar passes only for a union that has a scalar arm, - never an object-only one. This mirrors inbound dispatch, which errors on the same values. + A variant instance passes. A bare scalar passes only for a union that has a scalar arm, and + only as one of the literals that arm declares, so a stray string is a caller error rather + than a wire round-trip. This mirrors inbound dispatch, which errors on the same values. """ if isinstance(value, Record): if not isinstance(value, cls._variant_classes()): @@ -592,6 +605,9 @@ def validate_outbound(cls, owner: str, name: str, value: Any) -> None: raise BiDiSerializationError( f"{owner}.{name}: expected an object variant of {cls.__name__}, got {got} {value!r}" ) + if value not in cls._SCALAR_VALUES: + expected = ", ".join(repr(v) for v in sorted(cls._SCALAR_VALUES)) + raise BiDiSerializationError(f"{owner}.{name}: {value!r} is not one of {cls.__name__}'s arms ({expected})") @classmethod def from_json(cls, payload: Any) -> Any: diff --git a/py/test/selenium/webdriver/common/bidi/protocol_tests.py b/py/test/selenium/webdriver/common/bidi/protocol_tests.py index f0bcd265442d6..a632dd5a96dc4 100644 --- a/py/test/selenium/webdriver/common/bidi/protocol_tests.py +++ b/py/test/selenium/webdriver/common/bidi/protocol_tests.py @@ -26,8 +26,8 @@ is uniform): a plain command result and a deeply nested union/record result. Event delivery is not covered here: the layer speaks commands (request/response) -only. Routing pushed events into their generated types is the facade piece ADR -17701 keeps out of scope, so there is nothing in `_bidi` to exercise yet. +only. Routing pushed events into their generated types is the facade piece that +stays out of scope, so there is nothing in `_bidi` to exercise yet. """ from selenium.webdriver.common._bidi.browsing_context import ( diff --git a/py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py b/py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py index 11b208787add1..7e581a8ac1da4 100644 --- a/py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py +++ b/py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py @@ -146,7 +146,7 @@ def test_a_union_discriminator_selects_and_sends_its_variant(): assert connection.sent == {"method": "network.continueWithAuth", "params": {"request": "r", "action": "cancel"}} -# --- extras preserved only for a re-sendable extensible type (ADR item 8) --- +# --- extras preserved only for a re-sendable extensible type --- _STRING_VALUE = {"type": "string", "value": "v"} @@ -160,7 +160,7 @@ def test_a_re_sendable_extensible_type_round_trips_unknown_wire_keys(): def test_a_received_only_extensible_type_retains_unknown_wire_keys(): # network.Cookie is extensible per spec, so it retains unknown keys even though it is only ever - # received, never sent back — every extensible type keeps its extras (ADR decision 9). + # received, never sent back — every extensible type keeps its extras. cookie = Cookie.from_json( { "name": "n", diff --git a/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py b/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py index 7083c78c65f1d..3716785588cba 100644 --- a/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py +++ b/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py @@ -163,6 +163,7 @@ class Rect(Record): @register("test.Shape") class Shape(Union): _PRESENCE = (("test.Circle", ("radius",)), ("test.Rect", ("width", "height"))) + _SCALAR_VALUES = frozenset({"blob"}) @register("test.BareOrObject") @@ -285,7 +286,7 @@ def test_round_trips_through_the_wire(): assert Point.from_json(Point(x=1, y=2).as_json()) == Point(x=1, y=2) -# --- outbound value validation (as_json, ADR decision 1) --- +# --- outbound value validation (as_json) --- def test_as_json_accepts_valid_outbound_values(): @@ -298,6 +299,11 @@ def test_as_json_rejects_a_wrong_typed_primitive(): Point(x="nope", y=2).as_json() +def test_as_json_rejects_a_fractional_value_on_an_integer_field(): + with pytest.raises(BiDiSerializationError, match=r"Point.x: expected int, got float"): + Point(x=1.5, y=2).as_json() + + def test_as_json_rejects_a_scalar_where_a_list_is_expected(): with pytest.raises(BiDiSerializationError, match=r"Tags.tags: expected a list"): Tags(tags="a").as_json() @@ -337,7 +343,7 @@ def test_as_json_rejects_a_non_variant_object_map_value(): StringMap(value=[["k", Circle(radius=1)]]).as_json() -# --- outbound union fields (ADR decisions 4-5) --- +# --- outbound union fields --- def test_as_json_serializes_a_valid_union_variant(): @@ -355,9 +361,14 @@ def test_as_json_rejects_a_scalar_on_an_object_only_union_field(): UnionField(obj="cat").as_json() -def test_as_json_allows_a_bare_scalar_on_a_non_object_only_union_field(): - # Shape has scalar arms (not object-only), so a bare scalar passes as inbound would return it. - assert UnionField(shape="whatever").as_json() == {"shape": "whatever"} +def test_as_json_allows_a_pinned_bare_scalar_on_a_non_object_only_union_field(): + # Shape has a scalar arm (not object-only), so its pinned literal passes as inbound would return it. + assert UnionField(shape="blob").as_json() == {"shape": "blob"} + + +def test_as_json_rejects_an_unpinned_bare_scalar_on_a_non_object_only_union_field(): + with pytest.raises(BiDiSerializationError, match=r"UnionField.shape: 'whatever' is not one of Shape's arms"): + UnionField(shape="whatever").as_json() def test_as_json_accepts_a_transitively_nested_union_variant(): @@ -463,9 +474,10 @@ def test_integer_rejects_a_fractional_float(): Scalars.from_json({"count": 1.5, "ratio": 1.0, "flag": True, "name": "n"}) -def test_integer_rejects_even_a_whole_valued_float(): - with pytest.raises(BiDiSerializationError, match=r"expected int"): - Scalars.from_json({"count": 5.0, "ratio": 1.0, "flag": True, "name": "n"}) +def test_integer_accepts_a_whole_valued_float_and_holds_it_as_an_int(): + parsed = Scalars.from_json({"count": 5.0, "ratio": 1.0, "flag": True, "name": "n"}) + assert parsed.count == 5 + assert isinstance(parsed.count, int) def test_integer_rejects_a_bool(): @@ -547,7 +559,7 @@ def test_an_inbound_enum_value_outside_the_schema_raises(): def test_an_extensible_record_keeps_undeclared_properties_silently(caplog): # An undeclared field on an extensible type is spec-sanctioned (preserved), not a deviation, - # so it is kept without a warning (ADR decision 1/9). + # so it is kept without a warning. with caplog.at_level(logging.WARNING): result = Extensible.from_json({"known": "k", "extra": "e", "more": 1}) assert result.extensions == {"extra": "e", "more": 1} @@ -566,13 +578,13 @@ def test_an_extensible_record_merges_captured_keys_back_on_serialization(): def test_an_extension_may_not_shadow_a_declared_field_on_serialization(): - # A key the type declares must never appear in the extras map (ADR decision 1), so an extra + # A key the type declares must never appear in the extras map, so an extra # cannot overwrite a declared field on the wire. with pytest.raises(BiDiSerializationError, match=r"shadows declared field 'known'"): Extensible(known="k", extensions={"known": "evil"}).as_json() -# --- nullable constants (ADR decision 4, by vocabulary) --- +# --- nullable constants --- def test_a_nullable_constant_accepts_its_literal_and_null_outbound(): From 3c886a0698e92eca04f8ffcf0d470c2338aa961c Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Mon, 24 Aug 2026 13:51:38 -0500 Subject: [PATCH 2/3] [py] always reject a missing required inbound BiDi field --- .../webdriver/common/_bidi/serialization.py | 66 ++++++------------- .../common/bidi_serialization_tests.py | 50 +++----------- 2 files changed, 27 insertions(+), 89 deletions(-) diff --git a/py/selenium/webdriver/common/_bidi/serialization.py b/py/selenium/webdriver/common/_bidi/serialization.py index f010168e8ec32..b75f5b0d38139 100644 --- a/py/selenium/webdriver/common/_bidi/serialization.py +++ b/py/selenium/webdriver/common/_bidi/serialization.py @@ -35,9 +35,7 @@ import dataclasses import logging import re -from collections.abc import Callable, Iterator -from contextlib import contextmanager -from contextvars import ContextVar +from collections.abc import Callable from dataclasses import dataclass from enum import Enum from importlib import import_module @@ -52,41 +50,14 @@ class BiDiSerializationError(WebDriverException): """A payload could not be (de)serialized against this Selenium's BiDi schema.""" -_strict_inbound: ContextVar[bool] = ContextVar("bidi_strict_inbound", default=False) - - -@contextmanager -def strict_inbound() -> Iterator[None]: - """Escalate the inbound tolerations to errors for the duration of the block. - - By default the layer tolerates a payload that lags or runs ahead of this Selenium's - schema — a missing required field or an undeclared property — by warning and carrying - on (the wire boundary is not ours to control). Wrap a command call in - ``with strict_inbound():`` to make those deviations raise :class:`BiDiSerializationError` - instead, for a caller that wants strict conformance. A corrupt value always errors. - """ - token = _strict_inbound.set(True) - try: - yield - finally: - _strict_inbound.reset(token) - - -def _tolerate(message: str) -> None: - """A tolerated inbound deviation: raise it in strict mode, otherwise warn and continue.""" - if _strict_inbound.get(): - raise BiDiSerializationError(message) - logger.warning(message) - - -# Cap how many key names one tolerated-inbound warning spells out, so a payload with many -# unknown/absent keys yields a bounded log line / exception message instead of one built from -# every remote-supplied key; the remainder is summarized as a count. +# Cap how many key names one message spells out, so a payload with many unknown/absent keys +# yields a bounded log line / exception message instead of one built from every remote-supplied +# key; the remainder is summarized as a count. _MAX_KEYS_SHOWN = 10 def _summarize(owner: str, kind: str, keys: list[str], suffix: str) -> str: - """A bounded ``owner: kind 'a', 'b', … (+N more) (suffix)`` message for tolerated keys.""" + """A bounded ``owner: kind 'a', 'b', … (+N more) (suffix)`` message.""" shown = ", ".join(repr(k) for k in keys[:_MAX_KEYS_SHOWN]) if len(keys) > _MAX_KEYS_SHOWN: shown += f", … (+{len(keys) - _MAX_KEYS_SHOWN} more)" @@ -242,9 +213,8 @@ class Record: :func:`meta`. The object itself is permissive; validation lives at the boundaries. Outbound (:meth:`as_json`) omits ``UNSET``, emits ``null`` only for nullable fields, and errors if a required field is unset. Inbound (:meth:`from_json`) errors on a - corrupt value but tolerates a missing required field or an undeclared property — - it warns and carries on (``strict_inbound`` escalates to an error) — so a client - generated from one spec revision keeps working against a browser on another. + corrupt value or a missing required field, and tolerates only an undeclared property, + so a browser that adds a field does not break a client generated from an older schema. """ _EXTENSIBLE: bool = False @@ -294,9 +264,9 @@ def as_json(self) -> dict: declared.add(w.wire) value = getattr(self, f.name) if value is UNSET: - # Outbound requires every required field. Enforced here at - # the boundary, not in the constructor, so the object stays permissive and - # inbound can tolerate the same field being absent (see from_json). + # The constructor already requires every required field; this backstops a + # caller that passed the UNSET sentinel explicitly, so an unset required + # field can never reach the wire as an omission. if w.required: raise BiDiSerializationError(f"{type(self).__name__}.{f.name}: required {w.wire!r} is not set") continue @@ -338,23 +308,25 @@ def from_json(cls, payload: dict) -> Any: continue kwargs[f.name] = _read_field(cls, f.name, w, payload) undeclared = [k for k in payload if k not in known] - # A missing required field is tolerated and left unset. An undeclared field + # A required field the remote omitted cannot yield a valid object, so it errors rather + # than leaving a hole a caller cannot distinguish from a real value. An undeclared field # on an extensible type is spec-sanctioned, not a deviation: it is preserved in the extras - # map silently. On a non-extensible type it is a deviation: warn and drop. - # Each tolerated kind warns at most once per record — never once per key, so a verbose payload - # cannot flood the log; strict_inbound escalates a genuine deviation to an error. + # map silently. On a non-extensible type it is a deviation: warn and drop. Both messages + # name at most a bounded number of keys, so a verbose payload cannot flood the log. if missing_required: - _tolerate(_summarize(cls.__name__, "missing required", missing_required, "left unset")) + raise BiDiSerializationError( + _summarize(cls.__name__, "missing required", missing_required, "absent from the response") + ) if cls._EXTENSIBLE: kwargs["extensions"] = {k: payload[k] for k in undeclared} elif undeclared: - _tolerate(_summarize(cls.__name__, "undeclared", undeclared, "dropped")) + logger.warning(_summarize(cls.__name__, "undeclared", undeclared, "dropped")) return cls(**kwargs) def _read_field(cls: type, name: str, w: _Wire, payload: dict) -> Any: # Called only for a field present on the wire; from_json handles an absent field (an absent - # required one is tolerated and warned there, in one bounded message per record). + # required one errors there, in one bounded message naming every field that was missing). raw = payload[w.wire] if raw is None: if w.nullable: diff --git a/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py b/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py index 3716785588cba..f2b3e73fad009 100644 --- a/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py +++ b/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py @@ -41,7 +41,6 @@ meta, register, resolve, - strict_inbound, ) # --- fixtures: value types declared the way the generator emits them --- @@ -392,41 +391,20 @@ def test_a_unions_variant_classes_are_computed_once_and_cached(): # --- inbound: required / optional / null --- -def test_a_missing_required_field_inbound_is_tolerated_warned_and_left_unset(caplog): - with caplog.at_level(logging.WARNING): - result = Point.from_json({"x": 1}) - assert result.x == 1 - assert result.y is UNSET - assert "missing required" in caplog.text - assert "'y'" in caplog.text - - -def test_multiple_missing_required_fields_warn_once_for_the_record(caplog): - with caplog.at_level(logging.WARNING): - result = Point.from_json({}) - assert result.x is UNSET - assert result.y is UNSET - missing_warnings = [r for r in caplog.records if "missing required" in r.getMessage()] - assert len(missing_warnings) == 1 - assert "'x'" in caplog.text - assert "'y'" in caplog.text - - -def test_strict_inbound_escalates_a_missing_required_field_to_an_error(): - with strict_inbound(), pytest.raises(BiDiSerializationError, match=r"missing required 'y'"): +def test_a_missing_required_field_inbound_is_an_error(): + with pytest.raises(BiDiSerializationError, match=r"missing required 'y'"): Point.from_json({"x": 1}) -def test_strict_inbound_scope_is_restored_after_the_block(): - with strict_inbound(): - pass - assert Point.from_json({"x": 1}).y is UNSET # tolerant again +def test_every_missing_required_field_is_named_in_one_error(): + with pytest.raises(BiDiSerializationError, match=r"missing required 'x', 'y'"): + Point.from_json({}) def test_as_json_errors_when_a_required_field_is_unset(): - # A field the wire omitted is tolerated inbound (left UNSET) but must not go back out: - # outbound requires every required field, so re-serializing it errors. - incomplete = Point.from_json({"x": 1}) + # Nothing inbound can leave a required field unset any more, but a caller can pass the + # sentinel, and outbound requires every required field, so serializing one errors. + incomplete = Point(x=1, y=UNSET) with pytest.raises(BiDiSerializationError, match=r"Point.y: required 'y' is not set"): incomplete.as_json() @@ -566,13 +544,6 @@ def test_an_extensible_record_keeps_undeclared_properties_silently(caplog): assert caplog.records == [] -def test_strict_inbound_does_not_reject_undeclared_properties_on_an_extensible_record(): - # Extras are valid on an extensible type, so strict mode must not escalate them to an error. - with strict_inbound(): - result = Extensible.from_json({"known": "k", "extra": "e"}) - assert result.extensions == {"extra": "e"} - - def test_an_extensible_record_merges_captured_keys_back_on_serialization(): assert Extensible(known="k", extensions={"extra": "e"}).as_json() == {"known": "k", "extra": "e"} @@ -614,11 +585,6 @@ def test_a_closed_record_drops_and_warns_on_undeclared_properties(caplog): assert "'z'" in caplog.text -def test_strict_inbound_escalates_an_undeclared_property_to_an_error(): - with strict_inbound(), pytest.raises(BiDiSerializationError, match=r"undeclared 'z'"): - Point.from_json({"x": 1, "y": 2, "z": 3}) - - def test_many_undeclared_properties_warn_once_for_the_record_not_once_per_key(caplog): with caplog.at_level(logging.WARNING): Point.from_json({"x": 1, "y": 2, "a": 1, "b": 2, "c": 3}) From 955708c93b7e75de81e7d4b7e5880bf5277c66f7 Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Mon, 24 Aug 2026 15:11:22 -0500 Subject: [PATCH 3/3] [py] reject an inbound BiDi scalar outside its union's declared arms --- .../webdriver/common/_bidi/serialization.py | 27 +++++++++++++++---- .../common/bidi_serialization_tests.py | 11 ++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/py/selenium/webdriver/common/_bidi/serialization.py b/py/selenium/webdriver/common/_bidi/serialization.py index b75f5b0d38139..38fbcf62cd859 100644 --- a/py/selenium/webdriver/common/_bidi/serialization.py +++ b/py/selenium/webdriver/common/_bidi/serialization.py @@ -577,9 +577,22 @@ def validate_outbound(cls, owner: str, name: str, value: Any) -> None: raise BiDiSerializationError( f"{owner}.{name}: expected an object variant of {cls.__name__}, got {got} {value!r}" ) - if value not in cls._SCALAR_VALUES: - expected = ", ".join(repr(v) for v in sorted(cls._SCALAR_VALUES)) - raise BiDiSerializationError(f"{owner}.{name}: {value!r} is not one of {cls.__name__}'s arms ({expected})") + if not cls._scalar_arm(value): + raise BiDiSerializationError(f"{owner}.{name}: {value!r} is not one of {cls._arms()}") + + @classmethod + def _scalar_arm(cls, value: Any) -> bool: + """Whether a bare scalar is one of the literals this union's scalar arm declares. + + Compared by equality rather than set membership so an unhashable payload (a list where + a scalar belongs) answers False instead of raising and masking the real error. + """ + return any(value == arm for arm in cls._SCALAR_VALUES) + + @classmethod + def _arms(cls) -> str: + # Sorted by repr, not value: the arms of one union need not share a primitive type. + return f"{cls.__name__}'s arms ({', '.join(repr(v) for v in sorted(cls._SCALAR_VALUES, key=repr))})" @classmethod def from_json(cls, payload: Any) -> Any: @@ -589,8 +602,12 @@ def from_json(cls, payload: Any) -> Any: if cls._OBJECT_ONLY: got = type(payload).__name__ raise BiDiSerializationError(f"{cls.__name__} expected an object on the wire, got {got} {payload!r}") - # A bare scalar arm (e.g. input.Origin's "viewport") has no object to - # dispatch on, so it is returned unchanged. + # A bare scalar arm (e.g. input.Origin's "viewport") has no object to dispatch + # on, so it stands for itself — but only as a literal the schema pins. + if not cls._scalar_arm(payload): + raise BiDiSerializationError( + f"{cls.__name__} received a scalar not in this Selenium's BiDi schema: {payload!r}" + ) return payload variant = cls._select(payload) if variant is None: diff --git a/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py b/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py index f2b3e73fad009..61418b43f386c 100644 --- a/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py +++ b/py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py @@ -169,6 +169,7 @@ class Shape(Union): class BareOrObject(Union): _DISCRIMINATOR = "type" _VARIANTS = {} + _SCALAR_VALUES = frozenset({"viewport"}) @register("test.ObjectOnly") @@ -624,6 +625,16 @@ def test_a_bare_scalar_arm_is_returned_unchanged(): assert BareOrObject.from_json("viewport") == "viewport" +def test_an_inbound_scalar_outside_the_pinned_arms_raises(): + with pytest.raises(BiDiSerializationError, match=r"received a scalar not in this Selenium's BiDi schema"): + BareOrObject.from_json("banana") + + +def test_an_inbound_unhashable_payload_on_a_scalar_arm_raises_rather_than_crashing(): + with pytest.raises(BiDiSerializationError, match=r"received a scalar not in this Selenium's BiDi schema"): + BareOrObject.from_json(["viewport"]) + + def test_a_variant_outside_the_schema_raises_instead_of_passing_through(): with pytest.raises(BiDiSerializationError, match=r"not in this Selenium's BiDi schema"): Animal.from_json({"kind": "fish"})