diff --git a/rb/lib/selenium/webdriver/bidi/serialization.rb b/rb/lib/selenium/webdriver/bidi/serialization.rb index 4b36591c96c49..9e3a05ab3ea65 100644 --- a/rb/lib/selenium/webdriver/bidi/serialization.rb +++ b/rb/lib/selenium/webdriver/bidi/serialization.rb @@ -21,7 +21,8 @@ module Selenium module WebDriver class BiDi # Wire round-trip runtime for the generated protocol layer: the value-type bases - # (Record, Union), the omit sentinel (UNSET), and outbound enum validation. + # (Record, Union), the omit sentinel (UNSET), outbound enum validation, and the + # strict-inbound toggle. # # @api private module Serialization @@ -33,6 +34,17 @@ module Serialization def UNSET.inspect = 'UNSET' UNSET.freeze + # Strict inbound mode. Off by default: a required field missing from a response is + # tolerated as omitted and warned, so a schema ahead of the browser does not block the + # caller. When SE_BIDI_STRICT is set to anything but 0/false, that same case escalates + # to an error for callers who want it. + # + # @api private + def self.strict? + value = ENV.fetch('SE_BIDI_STRICT', '').strip.downcase + !value.empty? && value != '0' && value != 'false' + end + # Validates an outbound enum argument: +value+ is a symbol (or list of symbols) that # must be a key of the enum hash (+{symbol => wire_token}+), so a bad value fails # locally with a clear error instead of a round-trip. Outbound only; inbound wire diff --git a/rb/lib/selenium/webdriver/bidi/serialization/record.rb b/rb/lib/selenium/webdriver/bidi/serialization/record.rb index 05450bc311158..7285c8b5c44a9 100644 --- a/rb/lib/selenium/webdriver/bidi/serialization/record.rb +++ b/rb/lib/selenium/webdriver/bidi/serialization/record.rb @@ -79,9 +79,10 @@ def new(**kwargs) construct(**attributes) end - # Inbound: builds from the wire. A missing required field raises (in +wire_value+), - # enum tokens are mapped back to symbols and an unrecognized one raises (in +read+), and - # extra keys are captured (extensible) or ignored (closed) — strict on shape, lenient on extras. + # Inbound: builds from the wire. A missing required field is omitted and warned (or + # raised in strict mode, in +wire_value+); enum tokens are mapped back to symbols and an + # unrecognized one raises (in +read+); an undeclared property is warned, then captured + # (extensible) or dropped (closed) — strict on shape, lenient on extras. def from_json(json_payload) unless json_payload.is_a?(::Hash) raise Error::WebDriverError, "#{name} expected an object on the wire, got #{json_payload.inspect}" @@ -90,7 +91,9 @@ def from_json(json_payload) attributes = fields.to_h do |f| [f.name, wire_value(f, json_payload)] end - attributes[:extensions] = extra(json_payload) if extensible? + undeclared = extra(json_payload) + warn_undeclared(undeclared) unless undeclared.empty? + attributes[:extensions] = undeclared if extensible? construct(**attributes) end @@ -143,7 +146,19 @@ def wire_value(field, json_payload) return read(field, json_payload[field.wire_key]) if json_payload.key?(field.wire_key) return UNSET unless field.required - raise Error::WebDriverError, "#{name}##{field.name} is required but was missing from the response" + missing_required(field) + end + + # A required field absent from the response is tolerated as omitted (UNSET) and warned, so a + # schema ahead of the browser does not block the caller; strict mode (SE_BIDI_STRICT) escalates + # to an error for callers who want it. Omitted (UNSET) stays distinct from an explicit null (nil), + # which matters for the required-and-nullable fields the schema flags. + def missing_required(field) + message = "#{name}##{field.name} is required but was missing from the response" + raise Error::WebDriverError, message if Serialization.strict? + + WebDriver.logger.warn(message, id: :bidi_missing_required) + UNSET end def read(field, raw) @@ -252,6 +267,16 @@ def extra(json_payload) known = (@wire_keys ||= fields.map(&:wire_key)) json_payload.except(*known) end + + # Forward-compat signal: a property the type does not model is tolerated (retained on an + # extensible type, dropped on a closed one) and warned so schema drift is visible. Tagged + # +:bidi_undeclared_property+ so a caller can silence it via +logger.ignore+. + def warn_undeclared(undeclared) + undeclared.each_key do |key| + WebDriver.logger.warn("#{name} received an undeclared property: #{key.inspect}", + id: :bidi_undeclared_property) + end + end end # @api private diff --git a/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs b/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs index 214a9958dd4a6..40837e6740134 100644 --- a/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs +++ b/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs @@ -28,6 +28,8 @@ module Selenium def self.to_symbol: (String name, untyped value, untyped enum) -> untyped + def self.strict?: () -> bool + class Record < ::Data def self.define: (**untyped spec) -> singleton(Record) @@ -68,6 +70,8 @@ module Selenium def wire_value: (untyped field, Hash[untyped, untyped] json_payload) -> untyped + def missing_required: (untyped field) -> untyped + def read: (untyped field, untyped raw) -> untyped def read_ref: (untyped field, untyped raw) -> untyped @@ -85,6 +89,8 @@ module Selenium def scalar_value: (untyped field, untyped value) -> untyped def extra: (Hash[untyped, untyped] json_payload) -> Hash[untyped, untyped] + + def warn_undeclared: (Hash[untyped, untyped] undeclared) -> void end interface _Serializable diff --git a/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb b/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb index 3cbb110fe79ae..d65229b8fdc31 100644 --- a/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb +++ b/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb @@ -210,7 +210,9 @@ def valid_cookie_attrs describe 'extensible records' do it 'captures unknown keys and merges them back on serialization' do - parsed = Script::SharedReference.from_json('sharedId' => 's1', 'webdriverValue' => 42) + parsed = nil + expect { parsed = Script::SharedReference.from_json('sharedId' => 's1', 'webdriverValue' => 42) } + .to have_warning(:bidi_undeclared_property) expect(parsed.shared_id).to eq('s1') expect(parsed.extensions).to eq('webdriverValue' => 42) @@ -220,7 +222,9 @@ def valid_cookie_attrs # A re-sendable type (reachable from a command's params, e.g. a cookie filter) keeps # unknown properties so a received-then-resent payload round-trips them. it 'preserves an unknown key on a re-sendable type across a receive/re-send round trip' do - parsed = Storage::CookieFilter.from_json('name' => 'sid', 'x-vendor' => 'keep-me') + parsed = nil + expect { parsed = Storage::CookieFilter.from_json('name' => 'sid', 'x-vendor' => 'keep-me') } + .to have_warning(:bidi_undeclared_property) expect(parsed.extensions).to eq('x-vendor' => 'keep-me') expect(parsed.as_json).to eq('name' => 'sid', 'x-vendor' => 'keep-me') @@ -230,16 +234,19 @@ def valid_cookie_attrs # params), so preserveExtras is false: unknown keys are ignored, not stored/echoed. it 'drops an unknown key on an extensible-but-received-only type on re-serialize' do wire = Network::Cookie.new(**valid_cookie_attrs).as_json.merge('x-vendor' => 'drop-me') - parsed = Network::Cookie.from_json(wire) + parsed = nil + expect { parsed = Network::Cookie.from_json(wire) }.to have_warning(:bidi_undeclared_property) expect(parsed).not_to respond_to(:extensions) expect(parsed.as_json).not_to include('x-vendor') end - it 'ignores an unknown key without raising on a non-extensible type' do + it 'warns on and drops an unknown key on a non-extensible type' do wire = {'type' => 'password', 'username' => 'u', 'password' => 'p', 'x-vendor' => 'v'} + parsed = nil + expect { parsed = Network::AuthCredentials.from_json(wire) }.to have_warning(:bidi_undeclared_property) - expect { Network::AuthCredentials.from_json(wire) }.not_to raise_error + expect(parsed).not_to respond_to(:extensions) end end @@ -356,11 +363,6 @@ def valid_cookie_attrs # tripping the required-presence check on the others. let(:cookie_wire) { Network::Cookie.new(**valid_cookie_attrs).as_json } - it 'raises when a required field is missing from the response' do - expect { Network::Cookie.from_json('name' => 'sid') } - .to raise_error(Error::WebDriverError, /Cookie#value is required but was missing/) - end - it 'raises when a non-nullable field arrives as explicit null' do expect { Network::Cookie.from_json(cookie_wire.merge('name' => nil)) } .to raise_error(Error::WebDriverError, /Cookie#name received null but is not nullable/) @@ -421,6 +423,27 @@ def valid_cookie_attrs .to raise_error(Error::WebDriverError, /size expected integer/) end end + + # RequestDeviceInfo is a minimal record: a required `id` and a required-and-nullable `name`. + describe 'inbound required-field tolerance' do + it 'tolerates a missing required-nullable field as omitted (UNSET, not null) and warns' do + parsed = nil + expect { parsed = Bluetooth::RequestDeviceInfo.from_json('id' => 'dev-1') } + .to have_warning(:bidi_missing_required) + explicit = Bluetooth::RequestDeviceInfo.from_json('id' => 'dev-1', 'name' => nil) + + expect(parsed.name).to equal(Serialization::UNSET) + expect(explicit.name).to be_nil + end + + it 'escalates a missing required field to an error in strict mode (SE_BIDI_STRICT)' do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with('SE_BIDI_STRICT', '').and_return('true') + + expect { Bluetooth::RequestDeviceInfo.from_json('id' => 'dev-1') } + .to raise_error(Error::WebDriverError, /RequestDeviceInfo#name is required but was missing/) + end + end end end # Protocol end # BiDi