Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion rb/lib/selenium/webdriver/bidi/serialization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
35 changes: 30 additions & 5 deletions rb/lib/selenium/webdriver/bidi/serialization/record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions rb/sig/lib/selenium/webdriver/bidi/serialization.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
43 changes: 33 additions & 10 deletions rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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')
Expand All @@ -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

Expand Down Expand Up @@ -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/)
Expand Down Expand Up @@ -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
Expand Down