diff --git a/javascript/selenium-webdriver/project_bidi_schema.mjs b/javascript/selenium-webdriver/project_bidi_schema.mjs index 53b99dd205776..36c8cb38c29e4 100644 --- a/javascript/selenium-webdriver/project_bidi_schema.mjs +++ b/javascript/selenium-webdriver/project_bidi_schema.mjs @@ -21,7 +21,7 @@ * The normalizer has already removed the awkward CDDL shapes, so this is a * straight mapping into a small vocabulary: * - * type node: { kind: 'record', fields: [field], map?, extensible?, preserveExtras?, specHref? } + * type node: { kind: 'record', fields: [field], map?, extensible?, specHref? } * | { kind: 'enum', values: [string], specHref? } * | { kind: 'union', variants: [ref], selector, objectOnly?, specHref? } * | { kind: 'alias', type, specHref? } @@ -43,13 +43,10 @@ * points at the editor's draft, so for an older generated artifact the target drifts * from the pinned source; synthetic types (and anything neither source covers) omit it. * - * Three derived signals let a binding validate the wire boundary without re-deriving + * Two derived signals let a binding validate the wire boundary without re-deriving * anything itself: * `objectOnly: true` — a union all of whose arms are object (record) types, so a * non-object payload is a schema violation, not a scalar arm. - * `preserveExtras: true` — an `extensible` type that can also be *sent* (reachable - * from a command's params), so unknown properties received on - * the wire must be stored and echoed back rather than dropped. * an inline `enum` ref carries the `primitive` its literals share, so even a scalar * the normalizer did not hoist to a named enum is typed rather than opaque. * `scalar` on an inline `union` ref marks a union with a bare-scalar arm (a map entry's @@ -335,47 +332,6 @@ function variantIsObject(ref, types, seen = new Set()) { return false // enum } -// The type-name refs a projected ref node points at, recursing through list / map / -// inline union / inline record. (checkSchema has an equivalent local walk for its own -// referential checks; this module-level one feeds the reachability closure below.) -function refNames(node) { - if (!node) return [] - if (node.ref) return [node.ref] - if (node.list) return refNames(node.list) - if (node.map) return refNames(node.map) - if (node.union) return node.union.flatMap(refNames) - if (node.record) return node.record.flatMap((f) => refNames(f.type)) - return [] -} - -// The type-name refs a type *node* (record / union / alias) points at: a record's -// field and map value types, a union's variants, an alias's target. -function typeRefNames(node) { - if (node.kind === 'record') { - const refs = node.fields.flatMap((f) => refNames(f.type)) - if (node.map) refs.push(...refNames(node.map)) - return refs - } - if (node.kind === 'union') return node.variants - if (node.kind === 'alias') return refNames(node.type) - return [] -} - -// The set of types that can be *sent*: reachable from some command's params, through -// fields, lists, unions, maps, and nested records/aliases. Results and events are not -// roots — a type reached only through them is received-only. `preserveExtras` gates the -// extras store on this, so only a type you can hand back keeps unknown wire properties. -function reSendableTypes(commands, types) { - const reachable = new Set() - const visit = (name) => { - if (!name || reachable.has(name) || !types[name]) return - reachable.add(name) - for (const r of typeRefNames(types[name])) visit(r) - } - for (const c of commands) if (c.params?.ref) visit(c.params.ref) - return reachable -} - // The constant value a record pins on wire key `k`, as `{ value }` (a string or // `null`), or `{ open: true }` when the field exists but is not constant (a base // type acting as the catch-all, e.g. log.GenericLogEntry.type), or null when the @@ -635,13 +591,6 @@ export function projectSchema(ast, model, links = {}) { } } - // An extensible type keeps unknown wire properties only when it is also re-sendable - // (reachable from a command's params) — a type you receive and can hand back, so its - // extras must round-trip. A received-only extensible type drops them. - const reSendable = reSendableTypes(commands, types) - for (const [name, node] of Object.entries(types)) - if (node.extensible && reSendable.has(name)) node.preserveExtras = true - // Per-domain module links, for a binding that emits one class/namespace per domain. const domains = {} for (const domain of Object.keys(model)) { diff --git a/javascript/selenium-webdriver/project_bidi_schema_test.mjs b/javascript/selenium-webdriver/project_bidi_schema_test.mjs index 6420031d65fa8..02c1e36a8af3a 100644 --- a/javascript/selenium-webdriver/project_bidi_schema_test.mjs +++ b/javascript/selenium-webdriver/project_bidi_schema_test.mjs @@ -386,7 +386,7 @@ describe('unionSelector', () => { }) }) -describe('schema signals (objectOnly / preserveExtras / enum primitive)', () => { +describe('schema signals (objectOnly / extensible / enum primitive)', () => { const rec = (name, typeConst) => group(name, [field('type', [lit(typeConst)])]) const union = (name, refs) => ({ Type: 'variable', @@ -430,7 +430,10 @@ describe('schema signals (objectOnly / preserveExtras / enum primitive)', () => assert.equal(s.types['x.Origin'].objectOnly, undefined) }) - it('marks an extensible type reachable from command params as preserveExtras, but not a result-only one', () => { + it('marks every extensible type extensible, regardless of send/receive reachability', () => { + // Extensibility is the whole signal: a type reachable only through a command's result + // keeps its extras store just as one reachable through params does. Send-reachability + // ("retain extras only where they can be sent back") is deliberately not a factor. const ast = [ group('x.SetParams', [field('cfg', [ref('x.Config')])]), group('x.Config', [field('text', ['any'], { n: 0, m: null })]), @@ -439,10 +442,8 @@ describe('schema signals (objectOnly / preserveExtras / enum primitive)', () => ] const model = { x: { commands: [{ method: 'x.set', name: 'set', params: 'x.SetParams', result: 'x.GetResult' }] } } const s = projectSchema(ast, model) - assert.equal(s.types['x.Config'].extensible, true) - assert.equal(s.types['x.Config'].preserveExtras, true) // reachable through the command's params - assert.equal(s.types['x.Info'].extensible, true) - assert.equal(s.types['x.Info'].preserveExtras, undefined) // reachable only through the result + assert.equal(s.types['x.Config'].extensible, true) // reachable through the command's params + assert.equal(s.types['x.Info'].extensible, true) // reachable only through the result assert.deepEqual(checkSchema(s), []) }) diff --git a/rb/lib/selenium/webdriver/bidi/protocol/network.rb b/rb/lib/selenium/webdriver/bidi/protocol/network.rb index 8e6dfa0578526..07d6ba14c8abe 100644 --- a/rb/lib/selenium/webdriver/bidi/protocol/network.rb +++ b/rb/lib/selenium/webdriver/bidi/protocol/network.rb @@ -150,7 +150,8 @@ class BytesValue < Serialization::Union http_only: {wire_key: 'httpOnly', primitive: 'boolean'}, secure: {wire_key: 'secure', primitive: 'boolean'}, same_site: {wire_key: 'sameSite', enum: 'Network::SAME_SITE'}, - expiry: {wire_key: 'expiry', required: false, primitive: 'integer'} + expiry: {wire_key: 'expiry', required: false, primitive: 'integer'}, + extensible: true ) # @api private diff --git a/rb/lib/selenium/webdriver/bidi/protocol/session.rb b/rb/lib/selenium/webdriver/bidi/protocol/session.rb index d6f8110a423b8..ac6021349168a 100644 --- a/rb/lib/selenium/webdriver/bidi/protocol/session.rb +++ b/rb/lib/selenium/webdriver/bidi/protocol/session.rb @@ -204,7 +204,8 @@ class ProxyConfiguration < Serialization::Union required: false, ref: 'Session::UserPromptHandler' }, - web_socket_url: {wire_key: 'webSocketUrl', required: false, primitive: 'string'} + web_socket_url: {wire_key: 'webSocketUrl', required: false, primitive: 'string'}, + extensible: true ) # @api private diff --git a/rb/lib/selenium/webdriver/bidi/protocol/storage.rb b/rb/lib/selenium/webdriver/bidi/protocol/storage.rb index 0b86396a3be29..35ecb16a384b1 100644 --- a/rb/lib/selenium/webdriver/bidi/protocol/storage.rb +++ b/rb/lib/selenium/webdriver/bidi/protocol/storage.rb @@ -33,7 +33,8 @@ class Storage < Domain # @see https://w3c.github.io/webdriver-bidi/#type-storage-PartitionKey PartitionKey = Serialization::Record.define( user_context: {wire_key: 'userContext', required: false, primitive: 'string'}, - source_origin: {wire_key: 'sourceOrigin', required: false, primitive: 'string'} + source_origin: {wire_key: 'sourceOrigin', required: false, primitive: 'string'}, + extensible: true ) # @api private diff --git a/rb/lib/selenium/webdriver/bidi/serialization/record.rb b/rb/lib/selenium/webdriver/bidi/serialization/record.rb index 7285c8b5c44a9..21c7215514b9f 100644 --- a/rb/lib/selenium/webdriver/bidi/serialization/record.rb +++ b/rb/lib/selenium/webdriver/bidi/serialization/record.rb @@ -81,8 +81,8 @@ def new(**kwargs) # 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. + # unrecognized one raises (in +read+); an undeclared property is captured silently + # (extensible) or warned and 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}" @@ -92,8 +92,11 @@ def from_json(json_payload) [f.name, wire_value(f, json_payload)] end undeclared = extra(json_payload) - warn_undeclared(undeclared) unless undeclared.empty? - attributes[:extensions] = undeclared if extensible? + if extensible? + attributes[:extensions] = undeclared # the spec sanctions these extras; preserve them silently + else + warn_undeclared(undeclared) unless undeclared.empty? + end construct(**attributes) end @@ -268,9 +271,9 @@ def extra(json_payload) 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+. + # Forward-compat signal: a property a closed type does not model is dropped and warned so + # schema drift is visible (an extensible type keeps its extras silently — the spec sanctions + # them). 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}", @@ -301,9 +304,26 @@ def as_json(*) value = Serialization.to_wire(value, Protocol.const_get(f.enum)) if f.enum payload[f.wire_key] = Serializable.as_json(value) end - payload.merge!(extensions) if self.class.extensible? && !extensions.empty? + merge_extensions!(payload) if self.class.extensible? && !extensions.empty? payload end + + private + + # Merge the passthrough extras onto the wire, erroring rather than letting an extra whose key + # is a declared field's wire key silently clobber that typed value; an extra is by definition + # a field the spec does not declare. Keys are stringified first so a symbol key (e.g. `name:`) + # cannot slip past the guard and then reappear as a duplicate wire key once serialized. The + # single gate every outbound path funnels through: +new+, +with+, and in-place mutation. + def merge_extensions!(payload) + extras = extensions.transform_keys(&:to_s) + collisions = extras.keys & self.class.fields.map(&:wire_key) + unless collisions.empty? + raise ::ArgumentError, "#{self.class.name} extensions shadow declared fields: #{collisions.join(', ')}" + end + + payload.merge!(extras) + end end end end diff --git a/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb b/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb index 402152f2a0e55..85f9a75e89a06 100644 --- a/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb +++ b/rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb @@ -775,11 +775,11 @@ def record_class(name, type) wire: const['wire'], value: const['type']['const'], rbs: rbs_const(const['type']['const'])} fields = type['fields'].reject { |f| baked_discriminator?(f) }.map { |f| field_ir(f) } - # Gate the extensions store on `preserveExtras` (extensible AND re-sendable), not raw - # `extensible`: only a type you receive and can hand back keeps unknown wire keys. A - # received-only extensible type gets no store, so its unknown keys are silently ignored. + # Every extensible type gets the extensions store: an undeclared wire key is preserved + # and echoed back on any type the spec marks extensible, whether or not it is re-sendable. + # Extensibility alone is the signal; send-reachability does not enter into it. TypeClass.new(ruby_name: BiDiGenerate.type_class_name(name), fields: fields, - discriminator: discriminator, extensible: type['preserveExtras'] ? true : false, + discriminator: discriminator, extensible: type['extensible'] ? true : false, schema_name: name, synthetic: type['synthetic'] ? true : false, owner: type['owner'], label: type['label'], spec_href: type['specHref']) end diff --git a/rb/sig/lib/selenium/webdriver/bidi/protocol/network.rbs b/rb/sig/lib/selenium/webdriver/bidi/protocol/network.rbs index ea56d1a8ca2c2..ce3035c4807bd 100644 --- a/rb/sig/lib/selenium/webdriver/bidi/protocol/network.rbs +++ b/rb/sig/lib/selenium/webdriver/bidi/protocol/network.rbs @@ -89,7 +89,8 @@ module Selenium attr_reader secure: bool attr_reader same_site: Symbol attr_reader expiry: untyped - def self.new: (name: String, value: ::Selenium::WebDriver::BiDi::Protocol::Network::BytesValue, domain: String, path: String, size: Integer, http_only: bool, secure: bool, same_site: Symbol, ?expiry: Integer) -> instance + attr_reader extensions: Hash[String, untyped] + def self.new: (name: String, value: ::Selenium::WebDriver::BiDi::Protocol::Network::BytesValue, domain: String, path: String, size: Integer, http_only: bool, secure: bool, same_site: Symbol, ?expiry: Integer, ?extensions: Hash[String, untyped]) -> instance end class CookieHeader < ::Selenium::WebDriver::BiDi::Serialization::Record diff --git a/rb/sig/lib/selenium/webdriver/bidi/protocol/session.rbs b/rb/sig/lib/selenium/webdriver/bidi/protocol/session.rbs index dbe53a85f68b3..a0ec23ebdc033 100644 --- a/rb/sig/lib/selenium/webdriver/bidi/protocol/session.rbs +++ b/rb/sig/lib/selenium/webdriver/bidi/protocol/session.rbs @@ -140,7 +140,8 @@ module Selenium attr_reader proxy: untyped attr_reader unhandled_prompt_behavior: untyped attr_reader web_socket_url: untyped - def self.new: (accept_insecure_certs: bool, browser_name: String, browser_version: String, platform_name: String, set_window_rect: bool, user_agent: String, ?proxy: ::Selenium::WebDriver::BiDi::Protocol::Session::ProxyConfiguration, ?unhandled_prompt_behavior: ::Selenium::WebDriver::BiDi::Protocol::Session::UserPromptHandler, ?web_socket_url: String) -> instance + attr_reader extensions: Hash[String, untyped] + def self.new: (accept_insecure_certs: bool, browser_name: String, browser_version: String, platform_name: String, set_window_rect: bool, user_agent: String, ?proxy: ::Selenium::WebDriver::BiDi::Protocol::Session::ProxyConfiguration, ?unhandled_prompt_behavior: ::Selenium::WebDriver::BiDi::Protocol::Session::UserPromptHandler, ?web_socket_url: String, ?extensions: Hash[String, untyped]) -> instance end end diff --git a/rb/sig/lib/selenium/webdriver/bidi/protocol/storage.rbs b/rb/sig/lib/selenium/webdriver/bidi/protocol/storage.rbs index a81b5c58e216d..a0270089446d4 100644 --- a/rb/sig/lib/selenium/webdriver/bidi/protocol/storage.rbs +++ b/rb/sig/lib/selenium/webdriver/bidi/protocol/storage.rbs @@ -26,7 +26,8 @@ module Selenium class PartitionKey < ::Selenium::WebDriver::BiDi::Serialization::Record attr_reader user_context: untyped attr_reader source_origin: untyped - def self.new: (?user_context: String, ?source_origin: String) -> instance + attr_reader extensions: Hash[String, untyped] + def self.new: (?user_context: String, ?source_origin: String, ?extensions: Hash[String, untyped]) -> instance end class CookieFilter < ::Selenium::WebDriver::BiDi::Serialization::Record diff --git a/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs b/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs index 40837e6740134..51849c9a0a4bf 100644 --- a/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs +++ b/rb/sig/lib/selenium/webdriver/bidi/serialization.rbs @@ -103,6 +103,10 @@ module Selenium def self.as_json: (untyped value) -> untyped def as_json: (*untyped) -> Hash[String, untyped] + + private + + def merge_extensions!: (Hash[String, untyped] payload) -> void end end diff --git a/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb b/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb index 2b0525b33ec7e..a01278d25d8cf 100644 --- a/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb +++ b/rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb @@ -209,36 +209,56 @@ def valid_cookie_attrs end describe 'extensible records' do - it 'captures unknown keys and merges them back on serialization' do + # The spec sanctions extras on an extensible type, so they are captured silently — no + # undeclared-property warning, unlike a closed type. + it 'captures unknown keys silently and merges them back on serialization' do parsed = nil expect { parsed = Script::SharedReference.from_json('sharedId' => 's1', 'webdriverValue' => 42) } - .to have_warning(:bidi_undeclared_property) + .not_to have_warning(:bidi_undeclared_property) expect(parsed.shared_id).to eq('s1') expect(parsed.extensions).to eq('webdriverValue' => 42) expect(parsed.as_json).to eq('sharedId' => 's1', 'webdriverValue' => 42) end - # 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 + # An extensible type keeps unknown properties so a received-then-resent payload + # round-trips them. Extensibility alone is the trigger. + it 'preserves an unknown key on an extensible type across a receive/re-send round trip' do parsed = nil expect { parsed = Storage::CookieFilter.from_json('name' => 'sid', 'x-vendor' => 'keep-me') } - .to have_warning(:bidi_undeclared_property) + .not_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') end # network.Cookie is extensible but received-only (not reachable from any command's - # 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') + # params); it still preserves and echoes an unknown key, because extensibility — not + # send-reachability — is what sanctions the extra field. + it 'preserves an unknown key on an extensible received-only type across re-serialize' do + wire = Network::Cookie.new(**valid_cookie_attrs).as_json.merge('x-vendor' => 'keep-me') parsed = nil - expect { parsed = Network::Cookie.from_json(wire) }.to have_warning(:bidi_undeclared_property) + expect { parsed = Network::Cookie.from_json(wire) }.not_to have_warning(:bidi_undeclared_property) - expect(parsed).not_to respond_to(:extensions) - expect(parsed.as_json).not_to include('x-vendor') + expect(parsed.extensions).to eq('x-vendor' => 'keep-me') + expect(parsed.as_json).to include('x-vendor' => 'keep-me') + end + + # An extra is by definition a field the spec does not declare, so an extensions key that + # collides with a declared wire key would silently clobber a typed, validated value on the + # wire. Reject it at the merge instead — the single gate every outbound path funnels through. + it 'rejects an extension that shadows a declared field on serialize' do + cookie = Network::Cookie.new(**valid_cookie_attrs, extensions: {'name' => 'clobber'}) + + expect { cookie.as_json }.to raise_error(ArgumentError, /extensions shadow declared fields: name/) + end + + # A symbol key stringifies to a declared wire key on serialization, so it must trip the same + # guard rather than ride onto the wire as a duplicate of the typed field. + it 'rejects a symbol-keyed extension that shadows a declared field on serialize' do + cookie = Network::Cookie.new(**valid_cookie_attrs, extensions: {name: 'clobber'}) + + expect { cookie.as_json }.to raise_error(ArgumentError, /extensions shadow declared fields: name/) end it 'warns on and drops an unknown key on a non-extensible type' do