diff --git a/javascript/selenium-webdriver/project_bidi_schema.mjs b/javascript/selenium-webdriver/project_bidi_schema.mjs index 8c97b4548f87b..4fd850e2ea0a9 100644 --- a/javascript/selenium-webdriver/project_bidi_schema.mjs +++ b/javascript/selenium-webdriver/project_bidi_schema.mjs @@ -29,7 +29,7 @@ * | { ordered: [{ ref, requires: [key] }] } // structural, spec order * | { correlated: true } // resolved by request id, not the payload * field: { name, wire, required, type } - * type ref: { primitive } | { const } | { ref } | { enum, primitive? } | { list } | { map, extensible? } | { union, scalar? } + * type ref: { primitive } | { const } | { ref } | { enum, primitive? } | { list } | { map, extensible? } | { union, scalar?, scalarValues? } * any ref may also carry `nullable: true` (a `/ null` alternative). On a * record node, `map` is the value type of `* key => value` entries and * `extensible: true` marks an open `* text => any` record. @@ -53,6 +53,9 @@ * `RemoteValue / text`) and carries that arm's primitive: a binding collapsing it onto its * object_only ref arm passes a non-object payload (the string keys) through, but only when * it matches the primitive — a wrong-typed scalar is still rejected. + * `scalarValues` on a `union` ref pins the exact literals its `{ const }` scalar arms admit + * (input.Origin's "viewport" / "pointer"), so a binding can reject a wrong string, not just a + * wrong primitive — the tightest check the schema affords for a bare-scalar union arm. * * Types the normalizer synthesized for anonymous CDDL constructs additionally * carry `{ synthetic: true, owner, label }`: `owner` is the type the construct @@ -143,12 +146,16 @@ function scalarArmPrimitive(arm) { // and carries that arm's primitive (or the array of primitives when the scalar arms differ). // A binding that collapses such a union onto its object (object_only) ref arm must still let // a non-object payload through here, but only when it matches this primitive — a wrong-typed -// scalar is still a wire error. Derived once, in the schema, rather than re-detected per binding. +// scalar is still a wire error. `scalarValues` additionally pins the exact literals a `{ const }` +// scalar arm admits (input.Origin's "viewport" / "pointer"), so a binding can reject a wrong +// string too, not just a wrong primitive. Derived once, in the schema, not re-detected per binding. function unionNode(arms) { const node = { union: arms } const primitives = [...new Set(arms.map(scalarArmPrimitive).filter((p) => p !== undefined))] if (primitives.length === 1) node.scalar = primitives[0] else if (primitives.length > 1) node.scalar = primitives + const values = arms.filter((a) => a.const !== undefined).map((a) => a.const) + if (values.length) node.scalarValues = values return node } diff --git a/javascript/selenium-webdriver/project_bidi_schema_test.mjs b/javascript/selenium-webdriver/project_bidi_schema_test.mjs index dd7d57d5b3933..efda52c7a73ed 100644 --- a/javascript/selenium-webdriver/project_bidi_schema_test.mjs +++ b/javascript/selenium-webdriver/project_bidi_schema_test.mjs @@ -437,6 +437,9 @@ describe('schema signals (objectOnly / extensible / enum primitive)', () => { const s = projectSchema([origin, group('x.Element', [field('type', [lit('element')]), field('id', ['text'])])], {}) assert.equal(s.types['x.Origin'].kind, 'alias') assert.equal(s.types['x.Origin'].objectOnly, undefined) + // The const arms' literals are pinned so a binding can reject a wrong string, not just a wrong primitive. + assert.equal(s.types['x.Origin'].type.scalar, 'string') + assert.deepEqual(s.types['x.Origin'].type.scalarValues, ['viewport', 'pointer']) }) it('marks every extensible type extensible, regardless of send/receive reachability', () => { diff --git a/rb/lib/selenium/webdriver/bidi/protocol/input.rb b/rb/lib/selenium/webdriver/bidi/protocol/input.rb index 4e3e0c1d69bf4..cb2fdee876fe2 100644 --- a/rb/lib/selenium/webdriver/bidi/protocol/input.rb +++ b/rb/lib/selenium/webdriver/bidi/protocol/input.rb @@ -256,6 +256,7 @@ class Origin < Serialization::Union variants( element: 'Input::ElementOrigin' ) + scalar_values 'viewport', 'pointer' end # @api private diff --git a/rb/lib/selenium/webdriver/bidi/serialization/record.rb b/rb/lib/selenium/webdriver/bidi/serialization/record.rb index 2c07af77f2f57..679357dab7183 100644 --- a/rb/lib/selenium/webdriver/bidi/serialization/record.rb +++ b/rb/lib/selenium/webdriver/bidi/serialization/record.rb @@ -122,14 +122,64 @@ def validate_values(attributes) # Checks a field that carries an actual value (neither omitted nor nil): a nullable-const # field against its literal, list/scalar shape, primitive type (lists excepted, as inbound - # does), and enum membership (resolved lazily so a cross-domain enum need not load first). + # does), ref type, and enum membership (resolved lazily so a cross-domain enum need not load first). def validate_present(field, value) validate_const(field, value) check_outbound_shape(field, value) check_outbound_primitive(field, value) unless field.list + validate_ref(field, value) if field.ref Serialization.validate!("#{name}##{field.name}", value, Protocol.const_get(field.enum)) if field.enum end + # Outbound mirror of read_ref: a ref-typed value must be the type it declares, so a wrong + # record or a value no union variant accepts is a caller error caught here, not a browser + # round-trip. Shape is already checked, so a list is an Array. + def validate_ref(field, value) + klass = (@refs ||= {})[field.name] ||= Protocol.const_get(field.ref) + field.list ? validate_ref_list(field, klass, value) : validate_ref_value(field, klass, value) + end + + # Mirrors read_list: a scalar field is a [key, value] map, a nested list recurses, otherwise + # each element is checked against the ref. + def validate_ref_list(field, klass, list) + list.each do |element| + if field.scalar + validate_ref_entry(field, klass, element) + elsif element.is_a?(::Array) + validate_ref_list(field, klass, element) + else + validate_ref_value(field, klass, element) + end + end + end + + # A [key, value] map entry: the key may be a variant or a bare scalar, the value is a variant. + def validate_ref_entry(field, klass, element) + unless element.is_a?(::Array) && element.size == 2 + raise ::ArgumentError, "#{name}##{field.name} expected a [key, value] pair, got #{element.inspect}" + end + + key, value = element + key.is_a?(Serializable) ? validate_ref_value(field, klass, key) : check_outbound_scalar(field, key) + validate_ref_value(field, klass, value) + end + + # A record ref must be an instance of that record; a union ref must be one the union accepts. + def validate_ref_value(field, klass, value) + return if klass < Union ? klass.valid_outbound?(value) : value.is_a?(klass) + + raise ::ArgumentError, "#{name}##{field.name} expected #{field.ref}, got #{value.inspect}" + end + + # Outbound mirror of scalar_value: a bare map key must match one of the arm's primitives. + def check_outbound_scalar(field, value) + expected = Array(field.scalar).flat_map { |primitive| PRIMITIVE_TYPES[primitive] || [] } + return if expected.empty? || expected.any? { |type| value.is_a?(type) } + + raise ::ArgumentError, + "#{name}##{field.name} expected #{Array(field.scalar).join(' or ')}, got #{value.inspect}" + end + # A nullable constant (`literal / null`) is caller-settable but its only non-null value is # the literal, so a value that is neither the literal nor nil (nil is handled above) is a # local error rather than a wire round-trip. A non-const field carries UNSET here and passes. diff --git a/rb/lib/selenium/webdriver/bidi/serialization/union.rb b/rb/lib/selenium/webdriver/bidi/serialization/union.rb index b0e9ff29b3455..551ff2addc699 100644 --- a/rb/lib/selenium/webdriver/bidi/serialization/union.rb +++ b/rb/lib/selenium/webdriver/bidi/serialization/union.rb @@ -48,6 +48,11 @@ def fallback(path) = @fallback = path # object, so a non-Hash payload is a schema violation rather than a scalar arm. def object_only = @object_only = true + # Declared (via the schema's `scalarValues` signal) on a non-object_only union whose + # bare-scalar arms are a fixed set of literals (input.Origin's "viewport" / "pointer"). + # An outbound scalar outside that set matches no arm, so it is a caller error. + def scalar_values(*values) = @scalar_values = values + # A non-Hash payload is a bare scalar arm (e.g. input.Origin's "viewport") with no # object to dispatch on, so it is returned unchanged — unless every arm is an object # (object_only), where a non-Hash cannot match any variant and is a wire error. @@ -84,8 +89,36 @@ def build(**kwargs) raise ::ArgumentError, "invalid combination for #{name}: #{invalid.join(', ')}" end + # Outbound mirror of from_json: is +value+ one this union accepts? Any variant is accepted, + # and a variant that is itself a union recurses (e.g. LocalValue's RemoteReference fallback). + # A non-object_only union (e.g. input.Origin) also admits one of its pinned bare-scalar + # literals; an object (a Hash or another union's record) that matched no variant does not. + def valid_outbound?(value) + return true if variant_refs.any? { |ref| variant_accepts?(ref, value) } + + !@object_only && scalar_arm?(value) + end + private + # A bare-scalar arm must be one of the literals the schema pinned for this union + # (scalar_values, e.g. input.Origin's "viewport" / "pointer"). The generator guarantees a + # non-object_only union declares them, so no runtime guard is needed here. + def scalar_arm?(value) + @scalar_values.include?(value) + end + + # Every variant's class name: the discriminated table, the presence paths, and the fallback. + def variant_refs + @variant_refs ||= [*@variants&.values, *@presence&.keys, @fallback].compact + end + + # A variant that is itself a union recurses; a record variant is matched by instance. + def variant_accepts?(ref, value) + klass = (@variant_classes ||= {})[ref] ||= Protocol.const_get(ref) + klass < Union ? klass.valid_outbound?(value) : value.is_a?(klass) + end + # The discriminator value may legitimately be null (e.g. script.NullValue's # "null" tag), so it is matched by key presence. def select(json_payload) diff --git a/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb b/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb index a5df6e59cc5df..5cd71cfdd9b01 100644 --- a/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb +++ b/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb @@ -456,8 +456,10 @@ def discriminator_pair # the union's definition in the live spec (nil when the schema has none). object_only # mirrors the schema's `objectOnly` signal: when true, a non-Hash payload is rejected # rather than passed through (every arm is an object, so it can match no variant). + # scalar_values mirrors the schema's `scalarValues` signal: the exact literals a bare-scalar + # arm admits (input.Origin's "viewport" / "pointer"), so outbound rejects any other scalar. UnionClass = Struct.new(:ruby_name, :discriminator_wire, :variants, :schema_name, :nested, :spec_href, :object_only, - keyword_init: true) do + :scalar_values, keyword_init: true) do def union? = true def value_variants = variants.select { |v| v.mode == :value } def presence_variants = variants.select { |v| v.mode == :presence } @@ -473,6 +475,13 @@ def discriminator_decl(indent) BiDiGenerate.wrap_call("#{head}, ", pairs, indent, open: '{', close: '}') end + + def scalar_values? = !(scalar_values.nil? || scalar_values.empty?) + + # `scalar_values 'viewport', 'pointer'` — the literals a bare-scalar arm admits. + def scalar_values_decl + "scalar_values #{scalar_values.map { |v| BiDiGenerate.ruby_literal(v) }.join(', ')}" + end end # spec_href links the domain's module section in the live spec (nil when unknown). @@ -854,7 +863,15 @@ def union_class(name) # order); consume it rather than re-deriving and silently depending on emit # order. An alias-to-union (only input.Origin) has no selector — its const-string # arms aren't first-class types — so it keeps the structural re-derivation. - type['kind'] == 'union' ? union_from_selector(name, type['selector']) : union_from_alias(name) + klass = type['kind'] == 'union' ? union_from_selector(name, type['selector']) : union_from_alias(name) + # A non-object_only union has a bare-scalar arm; only const-literal arms (scalar_values) are + # modeled, so the runtime can validate an outbound scalar. A non-object_only union without them + # is a shape the generator doesn't yet handle — fail here, at generation, not at a caller's runtime. + if !klass.object_only && !klass.scalar_values? + raise "non-object_only union #{name} has no scalar_values to validate its bare-scalar arm" + end + + klass end # Map a union `selector` to dispatch variants the template renders: @@ -902,7 +919,8 @@ def ordered_variants(selector) # discriminator; the bare-string arms need no dispatch (Union.from_json returns a # non-Hash payload unchanged). So dispatch the ref arms by their const tag. def union_from_alias(name) - consts = @types[name]['type']['union'].filter_map { |arm| arm['ref'] }.to_h do |ref| + spec = @types[name] + consts = spec['type']['union'].filter_map { |arm| arm['ref'] }.to_h do |ref| const = @types[ref]['fields'].find { |f| f['type'].key?('const') } const || raise("alias-union #{name} arm #{ref} has no const discriminator to dispatch on") [ref, const] @@ -911,10 +929,12 @@ def union_from_alias(name) VariantIR.new(mode: :value, value: const['type']['const'], ref: ruby_path(ref), requires: nil) end # An alias-union carries bare-scalar arms (input.Origin's "viewport"/"pointer"), so it - # is never object_only — those arms must still pass a non-Hash payload through. + # is never object_only — those arms must still pass a non-Hash payload through, but only + # a value the schema pins in scalarValues (so a stray "banana" is still rejected outbound). UnionClass.new(ruby_name: BiDiGenerate.type_class_name(name), discriminator_wire: consts.values.first['wire'], variants: variants, schema_name: name, - spec_href: @types[name]['specHref'], object_only: @types[name]['objectOnly'] ? true : false) + spec_href: spec['specHref'], object_only: spec['objectOnly'] ? true : false, + scalar_values: spec['type']['scalarValues']) end def record_params(fields) diff --git a/rb/lib/selenium/webdriver/bidi/support/templates/module.rb.erb b/rb/lib/selenium/webdriver/bidi/support/templates/module.rb.erb index 687d44169a015..ccf75ab8c4e69 100644 --- a/rb/lib/selenium/webdriver/bidi/support/templates/module.rb.erb +++ b/rb/lib/selenium/webdriver/bidi/support/templates/module.rb.erb @@ -69,6 +69,9 @@ module Selenium <%- if type.object_only -%> object_only <%- end -%> +<%- if type.scalar_values? -%> + <%= type.scalar_values_decl %> +<%- end -%> <%- type.nested_types.each do |nested| -%> # @api private diff --git a/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs b/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs index 85cea3f491db9..fb9c5ecb99422 100644 --- a/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs +++ b/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs @@ -64,6 +64,16 @@ module Selenium def validate_present: (untyped field, untyped value) -> void + def validate_ref: (untyped field, untyped value) -> void + + def validate_ref_list: (untyped field, untyped klass, untyped list) -> void + + def validate_ref_entry: (untyped field, untyped klass, untyped element) -> void + + def validate_ref_value: (untyped field, untyped klass, untyped value) -> void + + def check_outbound_scalar: (untyped field, untyped value) -> void + def validate_const: (untyped field, untyped value) -> void def check_outbound_shape: (untyped field, untyped value) -> void @@ -125,12 +135,22 @@ module Selenium def self.object_only: () -> bool + def self.scalar_values: (*untyped values) -> Array[untyped] + def self.from_json: (untyped json_payload) -> untyped def self.build: (**untyped kwargs) -> untyped + def self.valid_outbound?: (untyped value) -> bool + private + def self.scalar_arm?: (untyped value) -> bool + + def self.variant_refs: () -> Array[String] + + def self.variant_accepts?: (String ref, untyped value) -> bool + def self.select: (Hash[untyped, untyped] json_payload) -> untyped def self.outbound_variant: (Hash[untyped, untyped] kwargs) -> untyped diff --git a/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb b/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb index c220fec5eb808..2ad1d196182f2 100644 --- a/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb +++ b/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb @@ -417,6 +417,118 @@ def moz_install(**kwargs) end end + describe 'outbound ref validation' do + # A record-typed ref: the value must be an instance of that exact record. A different + # record (even a sibling reference type) is a caller error caught before the wire. + it 'accepts the declared record for a record-typed ref' do + params = Input::SetFilesParameters.new( + context: 'c', element: Script::SharedReference.new(shared_id: 's1'), files: [] + ) + + expect(params.element).to be_a(Script::SharedReference) + end + + it 'rejects a wrong record for a record-typed ref' do + wrong = Script::RemoteObjectReference.new(handle: 'h') + + expect { Input::SetFilesParameters.new(context: 'c', element: wrong, files: []) } + .to raise_error(ArgumentError, /SetFilesParameters#element expected Script::SharedReference/) + end + + # A union-typed ref accepts any of the union's declared variants (decision 1), so a Cookie + # value may be either BytesValue arm. + it 'accepts any declared variant for a union-typed ref' do + string_cookie = Network::Cookie.new(**valid_cookie_attrs, value: Network::StringValue.new(value: 'YQ==')) + base64_cookie = Network::Cookie.new(**valid_cookie_attrs, value: Network::Base64Value.new(value: 'YQ==')) + + expect(string_cookie.value).to be_a(Network::StringValue) + expect(base64_cookie.value).to be_a(Network::Base64Value) + end + + # A record from a different union with the same wire shape is still not a BytesValue + # variant, so it is rejected rather than duck-typed onto the wire. + it 'rejects a variant from a different union for a union-typed ref' do + expect { Network::Cookie.new(**valid_cookie_attrs, value: Script::StringValue.new(value: 'x')) } + .to raise_error(ArgumentError, /Cookie#value expected Network::BytesValue/) + end + + # BytesValue is object_only, so a bare scalar cannot match any arm — the outbound mirror of + # rejecting a non-object where a typed object is expected (decision 4). + it 'rejects a bare scalar for an object-only union ref' do + expect { Network::Cookie.new(**valid_cookie_attrs, value: 'plain') } + .to raise_error(ArgumentError, /Cookie#value expected Network::BytesValue/) + end + + # A union with a bare-scalar arm (input.Origin's "viewport"/"pointer") admits each of its + # declared literals, but a record from another union remains a cross-union mismatch. + it 'accepts a declared bare-scalar arm for a non-object-only union ref' do + expect(Input::PointerMoveAction.new(x: 0, y: 0, origin: 'viewport').origin).to eq('viewport') + expect(Input::PointerMoveAction.new(x: 0, y: 0, origin: 'pointer').origin).to eq('pointer') + end + + # The object arm is still accepted alongside the scalar arms. + it 'accepts the object arm for a scalar-tolerant union ref' do + origin = Input::ElementOrigin.new(element: Script::SharedReference.new(shared_id: 's1')) + + expect(Input::PointerMoveAction.new(x: 0, y: 0, origin: origin).origin).to be_a(Input::ElementOrigin) + end + + # scalar_values pins the arm's literals ("viewport"/"pointer"), so a string outside that set + # matches no arm and is a caller error rather than a value the browser rejects a round-trip later. + it 'rejects a bare string that is not one of the union scalar arms' do + expect { Input::PointerMoveAction.new(x: 0, y: 0, origin: 'banana') } + .to raise_error(ArgumentError, /PointerMoveAction#origin expected Input::Origin/) + end + + # A wrong-typed scalar (a number or boolean where the arm is a string literal) is likewise + # not one of the declared arms. + it 'rejects a wrong-typed scalar for a union ref whose arms are string literals' do + expect { Input::PointerMoveAction.new(x: 0, y: 0, origin: 1) } + .to raise_error(ArgumentError, /PointerMoveAction#origin expected Input::Origin/) + expect { Input::PointerMoveAction.new(x: 0, y: 0, origin: true) } + .to raise_error(ArgumentError, /PointerMoveAction#origin expected Input::Origin/) + end + + it 'rejects a cross-union variant even where a scalar arm exists' do + expect { Input::PointerMoveAction.new(x: 0, y: 0, origin: Script::StringValue.new(value: 'x')) } + .to raise_error(ArgumentError, /PointerMoveAction#origin expected Input::Origin/) + end + + # A raw Hash is an object that matched no variant, not a bare-scalar arm, so it is rejected + # rather than passed through untyped. + it 'rejects a raw Hash for a union ref with a scalar arm' do + expect { Input::PointerMoveAction.new(x: 0, y: 0, origin: {type: 'element'}) } + .to raise_error(ArgumentError, /PointerMoveAction#origin expected Input::Origin/) + end + + # A ref list validates every element against the ref, so one bad element is rejected even + # when its siblings are valid variants. + it 'accepts a list whose every element is a declared variant' do + array = Script::ArrayLocalValue.new(value: [Script::StringValue.new(value: 'x'), + Script::NumberValue.new(value: 1)]) + + expect(array.value.size).to eq(2) + end + + it 'validates each element of a ref list, rejecting a non-variant element' do + expect { Script::ArrayLocalValue.new(value: [Script::NumberValue.new(value: 1), 42]) } + .to raise_error(ArgumentError, /ArrayLocalValue#value expected Script::LocalValue, got 42/) + end + + # A scalar-arm map keeps its bare-string key while still typing the value: a wrong-typed + # value (not a LocalValue variant) at the value position is rejected before the wire. + it 'rejects a non-variant value at a scalar-arm map position' do + expect { Script::ObjectLocalValue.new(value: [['k', 'not-a-value']]) } + .to raise_error(ArgumentError, /ObjectLocalValue#value expected Script::LocalValue, got "not-a-value"/) + end + + # The bare key at a scalar-arm map position must still match the arm's primitive. + it 'rejects a wrong-typed bare key at a scalar-arm map position' do + expect { Script::ObjectLocalValue.new(value: [[42, Script::StringValue.new(value: 'x')]]) } + .to raise_error(ArgumentError, /ObjectLocalValue#value expected string, got 42/) + end + end + describe 'enum symbol coercion' do it 'takes an idiomatic symbol and serializes the wire token (kebab included)' do params = Bluetooth::SimulateAdapterParameters.new(context: 'c', state: :powered_off)