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
42 changes: 24 additions & 18 deletions rb/lib/selenium/webdriver/bidi/serialization/record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ def validate_ref_value(field, klass, value)

# 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) }
checks = Array(field.scalar).filter_map { |primitive| PRIMITIVE_CHECKS[primitive] }
return if checks.empty? || checks.any? { |check| check.call(value) }

raise ::ArgumentError,
"#{name}##{field.name} expected #{Array(field.scalar).join(' or ')}, got #{value.inspect}"
Expand Down Expand Up @@ -204,8 +204,8 @@ def check_outbound_shape(field, value)
# ArgumentError here rather than a rejection the browser reports a round-trip later. A field
# with no primitive descriptor (enum, ref, opaque) passes; lists are skipped, as inbound does.
def check_outbound_primitive(field, value)
expected = PRIMITIVE_TYPES[field.primitive]
return if expected.nil? || expected.any? { |type| value.is_a?(type) }
check = PRIMITIVE_CHECKS[field.primitive]
return if check.nil? || check.call(value)

raise ::ArgumentError, "#{name}##{field.name} expected #{field.primitive}, got #{value.inspect}"
end
Expand Down Expand Up @@ -235,8 +235,11 @@ def read(field, raw)
return Serialization.to_symbol("#{name}##{field.name}", raw, enum_hash(field)) if field.enum

if field.ref.nil?
check_primitive(field, raw) unless field.list
return raw
return raw if field.list

check_primitive(field, raw)
# A whole number is exact in both types, so the declared type is held with nothing lost.
return field.primitive == 'integer' && raw.is_a?(::Float) ? raw.to_i : raw
end

read_ref(field, raw)
Expand All @@ -263,18 +266,21 @@ def check_shape(field, raw)
"#{name}##{field.name} expected #{field.list ? 'a list' : 'a single value'}, got #{raw.inspect}"
end

# Ruby classes a checkable primitive admits. `number` is any Numeric (JSON has one
# number type); `integer` requires an Integer — a browser emits `5`, not `5.0`, for an
# integer (JS has no int/float split), so this rarely false-positives yet still rejects
# a genuine non-integer like 1.5. A field with no primitive descriptor is left unchecked.
PRIMITIVE_TYPES = {
'string' => [::String], 'boolean' => [::TrueClass, ::FalseClass],
'number' => [::Numeric], 'integer' => [::Integer]
# The check a schema primitive admits, by JSON kind rather than Ruby class: `number` is
# any Numeric (JSON has one number type), and `integer` is any whole one — a browser is
# free to send `5` or `5.0` (JS has no int/float split), while a fractional value like
# 1.5 is a real mismatch. A field with no primitive descriptor is left unchecked.
WHOLE_FLOAT = ->(value) { value.is_a?(::Float) && value.finite? && (value % 1).zero? }
PRIMITIVE_CHECKS = {
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
'string' => ->(value) { value.is_a?(::String) },
'boolean' => ->(value) { value.is_a?(::TrueClass) || value.is_a?(::FalseClass) },
'number' => ->(value) { value.is_a?(::Numeric) },
'integer' => ->(value) { value.is_a?(::Integer) || WHOLE_FLOAT.call(value) }
}.freeze

def check_primitive(field, raw)
expected = PRIMITIVE_TYPES[field.primitive]
return if expected.nil? || expected.any? { |type| raw.is_a?(type) }
check = PRIMITIVE_CHECKS[field.primitive]
return if check.nil? || check.call(raw)

raise Error::SerializationError, "#{name}##{field.name} expected #{field.primitive}, got #{raw.inspect}"
end
Expand Down Expand Up @@ -317,11 +323,11 @@ def read_map_entry(field, element, klass)
# A bare scalar at a scalar-tolerant union position must match one of the union's
# scalar-arm primitives (+scalar+ is a primitive name or an array of them); a
# wrong-typed scalar (a number where a string is expected) is a wire error, not
# something to pass through. An unrecognized primitive (none in PRIMITIVE_TYPES) is
# something to pass through. An unrecognized primitive (none in PRIMITIVE_CHECKS) is
# left unchecked, matching the lenient default elsewhere.
def scalar_value(field, value)
expected = Array(field.scalar).flat_map { |primitive| PRIMITIVE_TYPES[primitive] || [] }
return value if expected.empty? || expected.any? { |type| value.is_a?(type) }
checks = Array(field.scalar).filter_map { |primitive| PRIMITIVE_CHECKS[primitive] }
return value if checks.empty? || checks.any? { |check| check.call(value) }

raise Error::SerializationError,
"#{name}##{field.name} expected #{Array(field.scalar).join(' or ')}, got #{value.inspect}"
Expand Down
14 changes: 12 additions & 2 deletions rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -404,11 +404,15 @@ def moz_install(**kwargs)
.to raise_error(ArgumentError, /NavigateParameters#url expected string/)
end

it 'rejects a float for an integer field, mirroring the wire integer/number split' do
expect { Emulation::ScreenArea.new(width: 5.0, height: 5) }
it 'rejects a fractional value for an integer field, mirroring the wire integer/number split' do
expect { Emulation::ScreenArea.new(width: 5.5, height: 5) }
.to raise_error(ArgumentError, /ScreenArea#width expected integer/)
end

it 'accepts a whole-valued float for an integer field, as the wire may spell it either way' do
expect(Emulation::ScreenArea.new(width: 5.0, height: 5).as_json).to eq('width' => 5.0, 'height' => 5)
end

it 'accepts either an integer or a float for a number field' do
klass = Emulation::GeolocationCoordinates

Expand Down Expand Up @@ -621,6 +625,12 @@ def moz_install(**kwargs)
expect(parsed.key).to eq(5)
end

it 'accepts a whole-valued float for an integer-typed field and holds it as an Integer' do
parsed = Bluetooth::BluetoothManufacturerData.from_json('key' => 5.0, 'data' => 'x')

expect(parsed.key).to be_an(::Integer).and eq(5)
end

# Signal 3: a scalar hidden behind an alias (size -> js-uint -> integer) now carries
# its leaf primitive, so a wrong-typed value is rejected instead of passing opaque.
it 'raises when an alias-typed integer field (js-uint) arrives as a string' do
Expand Down
Loading