From ca5110c7bdd97c8e5fc7205f06ff8f7b8d79e9d0 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Tue, 18 Aug 2026 16:34:18 +0530 Subject: [PATCH 1/5] [js] Add serialization and domain layer --- javascript/selenium-webdriver/BUILD.bazel | 5 + .../selenium-webdriver/bidi/domain.d.ts | 37 +++ javascript/selenium-webdriver/bidi/domain.js | 76 +++++ .../bidi/serialization/enum.d.ts | 23 ++ .../bidi/serialization/enum.js | 31 ++ .../bidi/serialization/record.d.ts | 58 ++++ .../bidi/serialization/record.js | 258 +++++++++++++++++ .../bidi/serialization/registry.js | 34 +++ .../bidi/serialization/union.d.ts | 27 ++ .../bidi/serialization/union.js | 87 ++++++ .../test/bidi/domain_test.js | 98 +++++++ .../test/bidi/serialization/record_test.js | 191 +++++++++++++ .../test/bidi/serialization/union_test.js | 117 ++++++++ .../bidi/serialization/wire_contract_test.js | 267 ++++++++++++++++++ 14 files changed, 1309 insertions(+) create mode 100644 javascript/selenium-webdriver/bidi/domain.d.ts create mode 100644 javascript/selenium-webdriver/bidi/domain.js create mode 100644 javascript/selenium-webdriver/bidi/serialization/enum.d.ts create mode 100644 javascript/selenium-webdriver/bidi/serialization/enum.js create mode 100644 javascript/selenium-webdriver/bidi/serialization/record.d.ts create mode 100644 javascript/selenium-webdriver/bidi/serialization/record.js create mode 100644 javascript/selenium-webdriver/bidi/serialization/registry.js create mode 100644 javascript/selenium-webdriver/bidi/serialization/union.d.ts create mode 100644 javascript/selenium-webdriver/bidi/serialization/union.js create mode 100644 javascript/selenium-webdriver/test/bidi/domain_test.js create mode 100644 javascript/selenium-webdriver/test/bidi/serialization/record_test.js create mode 100644 javascript/selenium-webdriver/test/bidi/serialization/union_test.js create mode 100644 javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js diff --git a/javascript/selenium-webdriver/BUILD.bazel b/javascript/selenium-webdriver/BUILD.bazel index 008d909e2aa1a..873626a93f8ce 100644 --- a/javascript/selenium-webdriver/BUILD.bazel +++ b/javascript/selenium-webdriver/BUILD.bazel @@ -150,6 +150,7 @@ js_library( "common/*.js", "bidi/*.js", "bidi/external/*.js", + "bidi/serialization/*.js", ]), deps = [ ":node_modules/@bazel/runfiles", @@ -197,7 +198,11 @@ pkg_tar( ) SMALL_TESTS = [ + "test/bidi/domain_test.js", "test/bidi/index_test.js", + "test/bidi/serialization/record_test.js", + "test/bidi/serialization/union_test.js", + "test/bidi/serialization/wire_contract_test.js", "test/io/io_test.js", "test/io/zip_test.js", "test/lib/bidi_connection_test.js", diff --git a/javascript/selenium-webdriver/bidi/domain.d.ts b/javascript/selenium-webdriver/bidi/domain.d.ts new file mode 100644 index 0000000000000..8f5a0be49d1cd --- /dev/null +++ b/javascript/selenium-webdriver/bidi/domain.d.ts @@ -0,0 +1,37 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +export interface EventDescriptor { + readonly method: string + readonly type?: { fromWire(payload: unknown): T } +} + +export function event(method: string, type?: { fromWire(payload: unknown): T }): EventDescriptor + +/** Internal construction guard — only a generated `Class.create(driver)` passes this. Never use directly. */ +export const DOMAIN_TOKEN: unique symbol + +export declare class Domain { + protected constructor(bidi: unknown, token: typeof DOMAIN_TOKEN) + protected static connect(driver: unknown): Promise + protected send(method: string, params: Record): Promise + addCallback( + descriptor: EventDescriptor, + handler: (params: T) => void, + ): Promise<{ id: string; unsubscribe(): Promise }> + removeCallback(subscriptionId: string): Promise +} diff --git a/javascript/selenium-webdriver/bidi/domain.js b/javascript/selenium-webdriver/bidi/domain.js new file mode 100644 index 0000000000000..cfedab53ecb9a --- /dev/null +++ b/javascript/selenium-webdriver/bidi/domain.js @@ -0,0 +1,76 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +const { getBidiConnection } = require('../lib/bidi_connection') + +// Gates Domain's constructor so `new Network(someRandomThing)` fails loudly +// instead of silently producing a broken instance. A Symbol can't be forged +// or guessed, so this is real runtime enforcement, not just a TS annotation — +// only a generated `Class.create(driver)` (and this package's own tests) may +// pass it. It's exported deliberately, not hidden: the point is to stop +// accidental misuse of the normal `new Network(x)` shape, not to defend +// against someone who deliberately imports and passes this. +const DOMAIN_TOKEN = Symbol('Domain internal construction token — obtained only via Class.create(driver)') + +/** + * @param {string} method + * @param {{fromWire(payload: unknown): unknown}} [type] Runtime record/union + * class for the event's params, if the schema declares one. When present, + * addCallback() parses each delivered payload through it before the + * caller's handler runs — inbound wire payloads are validated against + * their resolved type; an event's params is such a payload just as much + * as a command's result is. + * @returns {{method: string, type: ({fromWire(payload: unknown): unknown}|undefined)}} + */ +function event(method, type) { + return { method, type } +} + +/** Shared base for every generated BiDi domain class. See domain.d.ts for the typed surface. */ +class Domain { + #bidi + + constructor(bidi, token) { + if (token !== DOMAIN_TOKEN) { + throw new TypeError(`${new.target.name} must be constructed via ${new.target.name}.create(driver), not \`new\``) + } + this.#bidi = bidi + } + + static async connect(driver) { + return getBidiConnection(driver) + } + + async send(method, params) { + const response = await this.#bidi.send({ method, params }) + if (response?.error !== undefined) { + throw new Error(`${response.error}: ${response.message}`) + } + return response?.result + } + + async addCallback(descriptor, handler) { + const dispatch = descriptor.type === undefined ? handler : (params) => handler(descriptor.type.fromWire(params)) + return this.#bidi.addCallback(descriptor.method, dispatch) + } + + async removeCallback(subscriptionId) { + return this.#bidi.removeCallback(subscriptionId) + } +} + +module.exports = { Domain, event, DOMAIN_TOKEN } diff --git a/javascript/selenium-webdriver/bidi/serialization/enum.d.ts b/javascript/selenium-webdriver/bidi/serialization/enum.d.ts new file mode 100644 index 0000000000000..b40ca8531c46e --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/enum.d.ts @@ -0,0 +1,23 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +export interface EnumEntry { + readonly values: readonly T[] + includes(value: unknown): value is T +} + +export function defineEnum(name: string, values: readonly T[]): EnumEntry diff --git a/javascript/selenium-webdriver/bidi/serialization/enum.js b/javascript/selenium-webdriver/bidi/serialization/enum.js new file mode 100644 index 0000000000000..db16f0ca243a5 --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/enum.js @@ -0,0 +1,31 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +const { register } = require('./registry') + +/** + * @param {string} name Schema type name, e.g. 'network.InterceptPhase'. + * @param {string[]} values + */ +function defineEnum(name, values) { + const allowed = new Set(values) + const entry = { kind: 'enum', values, includes: (value) => allowed.has(value) } + register(name, entry) + return entry +} + +module.exports = { defineEnum } diff --git a/javascript/selenium-webdriver/bidi/serialization/record.d.ts b/javascript/selenium-webdriver/bidi/serialization/record.d.ts new file mode 100644 index 0000000000000..843e5d3e5909d --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/record.d.ts @@ -0,0 +1,58 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Mirrors bidi_schema.json's type-ref vocabulary (see project_bidi_schema.mjs). +export interface TypeNode { + primitive?: string + const?: unknown + ref?: string + enum?: string[] + list?: TypeNode + map?: TypeNode + union?: TypeNode[] + nullable?: boolean + // Present on an inline union with a bare-scalar arm — the primitive(s) that + // arm accepts (see unionNode() in project_bidi_schema.mjs). Not consumed by + // validateValue yet; declared so embedding a real schema node type-checks. + scalar?: string | string[] + // The exact `const` literals a bare-scalar union arm admits (e.g. + // input.Origin's "viewport"/"pointer") — see unionNode(). Same status as + // `scalar`: not yet consumed by validateValue, declared for the embed. + scalarValues?: unknown[] +} + +export interface FieldSpec { + name: string + wire: string + required: boolean + type: TypeNode +} + +export interface RecordOptions { + extensible?: boolean +} + +export declare class ValidationError extends Error {} + +export interface RecordClass { + new (data: T): Readonly + fromWire(payload: unknown): Readonly +} + +export function defineRecord(name: string, fields: FieldSpec[], options?: RecordOptions): RecordClass + +export function defineAlias(name: string, type: TypeNode): void diff --git a/javascript/selenium-webdriver/bidi/serialization/record.js b/javascript/selenium-webdriver/bidi/serialization/record.js new file mode 100644 index 0000000000000..b39a8c29af7d3 --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/record.js @@ -0,0 +1,258 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +const { register, resolve } = require('./registry') + +class ValidationError extends Error {} + +// Validates a *present* value against a resolved type node. This check is +// identical for outbound and inbound — a structurally wrong value is always +// an error, whether it's being sent or received. Only presence (required) +// and extras (undeclared properties) differ by direction, handled separately +// in the constructor and fromWire() below. +// `direction` only affects how a nested ref-to-record/union is itself validated. +function validateValue(typeNode, value, path, direction) { + if (value === null) { + if (typeNode.nullable) return + throw new ValidationError(`${path}: null is not allowed`) + } + + if (typeNode.primitive !== undefined) { + const expected = { string: 'string', integer: 'number', number: 'number', boolean: 'boolean' }[typeNode.primitive] + if (expected && typeof value !== expected) { + throw new ValidationError(`${path}: expected ${typeNode.primitive}, got ${typeof value}`) + } + // `number` admits any JSON number; `integer` rejects a fractional value + // (5.7) while still accepting one written 5.0 (Number.isInteger(5.0) is true). + if (typeNode.primitive === 'integer' && !Number.isInteger(value)) { + throw new ValidationError(`${path}: expected an integer, got ${value}`) + } + return + } + + if (typeNode.const !== undefined) { + if (value !== typeNode.const) { + throw new ValidationError( + `${path}: expected constant ${JSON.stringify(typeNode.const)}, got ${JSON.stringify(value)}`, + ) + } + return + } + + if (typeNode.enum !== undefined) { + if (!typeNode.enum.includes(value)) { + throw new ValidationError( + `${path}: "${value}" is not a valid value; expected one of: ${typeNode.enum.join(', ')}`, + ) + } + return + } + + if (typeNode.list !== undefined) { + if (!Array.isArray(value)) { + throw new ValidationError(`${path}: expected a list, got ${typeof value}`) + } + value.forEach((item, i) => validateValue(typeNode.list, item, `${path}[${i}]`, direction)) + return + } + + if (typeNode.map !== undefined) { + if (typeof value !== 'object' || Array.isArray(value) || value === null) { + throw new ValidationError(`${path}: expected an object, got ${typeof value}`) + } + for (const [key, entry] of Object.entries(value)) { + validateValue(typeNode.map, entry, `${path}.${key}`, direction) + } + return + } + + if (typeNode.ref !== undefined) { + const referenced = resolve(typeNode.ref) + if (referenced === undefined) return // not yet registered — best-effort, skip deep validation + + if (referenced.kind === 'enum') { + if (!referenced.includes(value)) { + throw new ValidationError( + `${path}: "${value}" is not a valid ${typeNode.ref} value; expected one of: ${referenced.values.join(', ')}`, + ) + } + return + } + + if (referenced.kind === 'record') { + if (value instanceof referenced.RecordClass) return // already validated + if (typeof value !== 'object' || Array.isArray(value) || value === null) { + throw new ValidationError(`${path}: expected an object, got ${typeof value}`) + } + // Recurse through the same-direction path so a nested field gets the + // same tolerance (inbound) or strictness (outbound) as its parent. + if (direction === 'inbound') { + referenced.RecordClass.fromWire(value) + } else { + new referenced.RecordClass(value) + } + return + } + + if (referenced.kind === 'union') { + if (direction === 'inbound') { + referenced.fromWire(value) + } else { + referenced.build(value) + } + return + } + + if (referenced.kind === 'alias') { + validateValue(referenced.type, value, path, direction) + return + } + + return + } + + if (typeNode.union !== undefined) { + const errors = [] + for (const variant of typeNode.union) { + try { + validateValue(variant, value, path, direction) + return + } catch (err) { + errors.push(err.message) + } + } + throw new ValidationError(`${path}: value did not match any variant (${errors.join('; ')})`) + } +} + +/** + * @param {string} name Schema type name, e.g. 'network.AddInterceptParameters'. + * @param {Array<{name: string, wire: string, required: boolean, type: object}>} fields + * @param {{extensible?: boolean}} [options] + */ +function defineRecord(name, fields, options = {}) { + const { extensible = false } = options + const byWire = new Map(fields.map((f) => [f.wire, f])) + + class Record { + // Outbound: strict. Any value that doesn't match its declared shape is an error here. + constructor(data) { + if (typeof data !== 'object' || data === null || Array.isArray(data)) { + throw new ValidationError(`${name}: expected an object`) + } + + for (const field of fields) { + if (!Object.hasOwn(data, field.wire)) { + if (field.required) { + throw new ValidationError(`${name}.${field.wire}: required field is missing`) + } + continue + } + const value = data[field.wire] + validateValue(field.type, value, `${name}.${field.wire}`, 'outbound') + this[field.name] = value + } + + for (const wireKey of Object.keys(data)) { + if (byWire.has(wireKey)) continue + if (!extensible) { + throw new ValidationError(`${name}: unknown property "${wireKey}"`) + } + // Object.defineProperty, not `this[wireKey] = ...`: wireKey is caller-supplied + // and a literal "__proto__" key assigned via bracket notation hijacks this + // instance's actual prototype instead of becoming a field (CWE-1321). + // defineProperty always creates a genuine own data property, regardless of name. + Object.defineProperty(this, wireKey, { + value: data[wireKey], // vendor extras reach the wire on an extensible type + enumerable: true, + writable: true, + configurable: true, + }) + } + + Object.freeze(this) + } + + // Inbound: tolerant of undeclared properties, but a missing required field + // is rejected just like a structurally invalid value — omission used to be + // tolerated here, but that's no longer required. + // Bypasses the constructor above entirely — a single constructor enforcing + // both directions symmetrically would make tolerated undeclared-property + // retention impossible. + static fromWire(payload) { + if (typeof payload !== 'object' || payload === null || Array.isArray(payload)) { + throw new ValidationError(`${name}: expected an object on the wire, got ${typeof payload}`) + } + + const instance = Object.create(Record.prototype) + + for (const field of fields) { + if (!Object.hasOwn(payload, field.wire)) { + if (field.required) { + throw new ValidationError(`${name}.${field.wire}: required field is missing`) + } + continue // left genuinely absent, optional field + } + const value = payload[field.wire] + // A present value's shape is never tolerated, inbound or outbound. + validateValue(field.type, value, `${name}.${field.wire}`, 'inbound') + instance[field.name] = value + } + + for (const wireKey of Object.keys(payload)) { + if (byWire.has(wireKey)) continue + // An extensible type preserves an undeclared field silently — no narrower + // criterion than "extensible" itself (not, say, only fields that happen to be + // sendable back on some other type). A non-extensible type warns and drops it + // instead — the warning belongs only to the drop, not the retention. + if (extensible) { + // Object.defineProperty, not `instance[wireKey] = ...` — see the matching + // comment in the constructor above: a literal "__proto__" key from an + // untrusted wire payload must become a field, not swap the prototype (CWE-1321). + Object.defineProperty(instance, wireKey, { + value: payload[wireKey], + enumerable: true, + writable: true, + configurable: true, + }) + } else { + process.emitWarning(`${name}: undeclared property "${wireKey}"`, 'BiDiSchemaWarning') + } + } + + Object.freeze(instance) + return instance + } + } + + Object.defineProperty(Record, 'name', { value: name }) + register(name, { kind: 'record', RecordClass: Record }) + return Record +} + +/** + * Registers a schema `alias` — a name with no fields of its own, just a + * pointer to another type node (e.g. `network.Intercept` aliasing a plain + * string). A ref to an alias validates through the aliased type node. + * @param {string} name + * @param {object} type The schema's `type` node this name aliases. + */ +function defineAlias(name, type) { + register(name, { kind: 'alias', type }) +} + +module.exports = { defineRecord, defineAlias, ValidationError } diff --git a/javascript/selenium-webdriver/bidi/serialization/registry.js b/javascript/selenium-webdriver/bidi/serialization/registry.js new file mode 100644 index 0000000000000..0faaccbe4027d --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/registry.js @@ -0,0 +1,34 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Shared type registry: every generated type registers itself here by its +// exact schema name (e.g. 'network.InterceptPhase'), so a field whose type is +// a `ref` can look up what the referenced type actually is — without every +// domain file needing to import every other domain file directly, and without +// needing types defined in dependency order (resolution happens at validation +// time, not at define time, so forward and circular refs both work). +const types = new Map() + +function register(name, entry) { + types.set(name, entry) +} + +function resolve(name) { + return types.get(name) +} + +module.exports = { register, resolve } diff --git a/javascript/selenium-webdriver/bidi/serialization/union.d.ts b/javascript/selenium-webdriver/bidi/serialization/union.d.ts new file mode 100644 index 0000000000000..6f420168ce8af --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/union.d.ts @@ -0,0 +1,27 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +export interface UnionOptions { + objectOnly?: boolean +} + +export interface UnionClass { + build(data: unknown): Readonly + fromWire(payload: unknown): Readonly +} + +export function defineUnion(name: string, selector: unknown, options?: UnionOptions): UnionClass diff --git a/javascript/selenium-webdriver/bidi/serialization/union.js b/javascript/selenium-webdriver/bidi/serialization/union.js new file mode 100644 index 0000000000000..4c6c987484a9c --- /dev/null +++ b/javascript/selenium-webdriver/bidi/serialization/union.js @@ -0,0 +1,87 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +const { register, resolve } = require('./registry') +const { ValidationError } = require('./record') + +// Resolves the variant ref a value/payload matches, per the schema's selector shape: +// { by, variants: [{value, ref}], default? } - discriminated: match `data[by]` against +// each variant's value. +// { ordered: [{ref, requires}] } - structural: first variant whose `requires` +// keys are all present in `data`, in spec order. +function selectVariant(selector, data, hasKey) { + if (selector.by) { + const tag = hasKey(data, selector.by) ? data[selector.by] : undefined + const match = selector.variants.find((v) => v.value === tag) + if (match) return match.ref + return selector.default + } + if (selector.ordered) { + for (const variant of selector.ordered) { + if (variant.requires.every((key) => hasKey(data, key))) return variant.ref + } + return undefined + } + return undefined // correlated: resolved by request id elsewhere, not from the payload +} + +/** + * @param {string} name Schema type name, e.g. 'session.ProxyConfiguration'. + * @param {object} selector The schema's `selector` node for this union. + * @param {{objectOnly?: boolean}} [options] + */ +function defineUnion(name, selector, options = {}) { + const { objectOnly = false } = options + + const union = { + kind: 'union', + + // Outbound: resolve which variant `data` describes, then delegate to that + // variant's own (strict) constructor. + build(data) { + if (objectOnly && (typeof data !== 'object' || data === null || Array.isArray(data))) { + throw new ValidationError(`${name}: expected an object`) + } + const ref = selectVariant(selector, data, (d, key) => Object.hasOwn(d, key)) + if (ref === undefined) { + throw new ValidationError(`${name}: value does not match any known variant`) + } + const variant = resolve(ref) + return new variant.RecordClass(data) + }, + + // Inbound: resolve which variant `payload` matches. An unresolvable payload is a + // closed-vocabulary miss — always an error, never a warning, since there is no + // valid typed object to fall back to. + fromWire(payload) { + if (objectOnly && (typeof payload !== 'object' || payload === null || Array.isArray(payload))) { + throw new ValidationError(`${name}: expected an object on the wire, got ${typeof payload}`) + } + const ref = selectVariant(selector, payload, (d, key) => Object.hasOwn(d, key)) + if (ref === undefined) { + throw new ValidationError(`${name}: received a variant not in this binding's BiDi schema`) + } + const variant = resolve(ref) + return variant.RecordClass.fromWire(payload) + }, + } + + register(name, union) + return union +} + +module.exports = { defineUnion } diff --git a/javascript/selenium-webdriver/test/bidi/domain_test.js b/javascript/selenium-webdriver/test/bidi/domain_test.js new file mode 100644 index 0000000000000..5f605235220ee --- /dev/null +++ b/javascript/selenium-webdriver/test/bidi/domain_test.js @@ -0,0 +1,98 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +'use strict' + +const assert = require('node:assert') +const { Domain, event, DOMAIN_TOKEN } = require('selenium-webdriver/bidi/domain') +const { defineRecord } = require('selenium-webdriver/bidi/serialization/record') + +const EntryAdded = defineRecord('test.domain.EntryAdded', [ + { name: 'text', wire: 'text', required: true, type: { primitive: 'string' } }, +]) + +// Fake replacing the real BiDi transport — records what addCallback/removeCallback +// receive and lets the test drive delivery directly, without a socket. +function fakeBidi() { + return { + registered: undefined, + async addCallback(method, handler) { + this.registered = { method, handler } + return { id: 'sub-1', unsubscribe: async () => {} } + }, + async removeCallback(subscriptionId) { + this.removedId = subscriptionId + }, + } +} + +describe('Domain addCallback', function () { + it('parses delivered payloads through the descriptor type before the handler runs', async function () { + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.entryAdded', EntryAdded) + + const received = [] + await domain.addCallback(descriptor, (params) => received.push(params)) + + bidi.registered.handler({ text: 'hello' }) + + assert.strictEqual(received.length, 1) + assert.ok(received[0] instanceof EntryAdded) + assert.strictEqual(received[0].text, 'hello') + }) + + it('passes the raw payload through when the descriptor has no type', async function () { + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const received = [] + await domain.addCallback(descriptor, (params) => received.push(params)) + + bidi.registered.handler({ anything: 'goes' }) + + assert.strictEqual(received.length, 1) + assert.deepStrictEqual(received[0], { anything: 'goes' }) + assert.ok(!(received[0] instanceof EntryAdded)) + }) + + it('removeCallback forwards the subscription id to the underlying transport', async function () { + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + + await domain.removeCallback('sub-1') + + assert.strictEqual(bidi.removedId, 'sub-1') + }) +}) + +describe('Domain construction guard', function () { + it('rejects `new Domain(bidi)` with no token', function () { + assert.throws(() => new Domain(fakeBidi()), TypeError) + }) + + it('rejects a forged token', function () { + assert.throws(() => new Domain(fakeBidi(), Symbol('not the real token')), TypeError) + }) + + it('does not expose the wrapped transport as an enumerable/own property', function () { + const domain = new Domain(fakeBidi(), DOMAIN_TOKEN) + assert.deepStrictEqual(Object.keys(domain), []) + assert.strictEqual(JSON.stringify(domain), '{}') + }) +}) diff --git a/javascript/selenium-webdriver/test/bidi/serialization/record_test.js b/javascript/selenium-webdriver/test/bidi/serialization/record_test.js new file mode 100644 index 0000000000000..6c4e8c05bfdfd --- /dev/null +++ b/javascript/selenium-webdriver/test/bidi/serialization/record_test.js @@ -0,0 +1,191 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +'use strict' + +const assert = require('node:assert') +const { defineEnum } = require('selenium-webdriver/bidi/serialization/enum') +const { defineRecord, ValidationError } = require('selenium-webdriver/bidi/serialization/record') + +// Real fixtures from the WebDriver BiDi schema (network.InterceptPhase, +// network.AddInterceptParameters, network.BeforeRequestSentParameters), +// inlined so this test doesn't depend on generating bidi_schema.json. +defineEnum('test.record.InterceptPhase', ['beforeRequestSent', 'responseStarted', 'authRequired']) + +const AddInterceptParameters = defineRecord('test.record.AddInterceptParameters', [ + { name: 'phases', wire: 'phases', required: true, type: { list: { ref: 'test.record.InterceptPhase' } } }, + { name: 'urlPatterns', wire: 'urlPatterns', required: false, type: { list: { primitive: 'string' } } }, +]) + +const BeforeRequestSentParameters = defineRecord('test.record.BeforeRequestSentParameters', [ + { + name: 'context', + wire: 'context', + required: true, + type: { ref: 'browsingContext.BrowsingContext', nullable: true }, + }, + { name: 'isBlocked', wire: 'isBlocked', required: true, type: { primitive: 'boolean' } }, + { name: 'timestamp', wire: 'timestamp', required: true, type: { primitive: 'integer' } }, +]) + +describe('serialization/record', function () { + describe('outbound (constructor)', function () { + it('accepts a valid object', function () { + const params = new AddInterceptParameters({ phases: ['beforeRequestSent'] }) + assert.deepStrictEqual(params.phases, ['beforeRequestSent']) + }) + + it('rejects an undefined enum value', function () { + assert.throws(() => new AddInterceptParameters({ phases: ['notARealPhase'] }), ValidationError) + }) + + it('lists the valid values in the error message', function () { + assert.throws( + () => new AddInterceptParameters({ phases: ['notARealPhase'] }), + (err) => { + assert.ok(err.message.includes('beforeRequestSent'), err.message) + assert.ok(err.message.includes('responseStarted'), err.message) + assert.ok(err.message.includes('authRequired'), err.message) + return true + }, + ) + }) + + it('rejects a missing required field', function () { + assert.throws(() => new AddInterceptParameters({}), ValidationError) + }) + + it('rejects an unknown property on a non-extensible type', function () { + assert.throws(() => new AddInterceptParameters({ phases: ['beforeRequestSent'], bogus: 'x' }), ValidationError) + }) + + it('produces an immutable instance', function () { + const params = new AddInterceptParameters({ phases: ['beforeRequestSent'] }) + assert.throws(() => { + params.phases = [] + }) + }) + }) + + describe('inbound (fromWire)', function () { + let warnings + + beforeEach(function () { + warnings = [] + process.on('warning', onWarning) + }) + + afterEach(function () { + process.off('warning', onWarning) + }) + + function onWarning(w) { + warnings.push(w.message) + } + + it('accepts an explicit null for a required+nullable field', function () { + const parsed = BeforeRequestSentParameters.fromWire({ context: null, isBlocked: true, timestamp: 1 }) + assert.strictEqual(parsed.context, null) + }) + + it('throws when a required field is missing', function () { + assert.throws(() => BeforeRequestSentParameters.fromWire({ context: null, timestamp: 1 }), ValidationError) + }) + + it('throws when a required, non-nullable field is explicitly null (corruption, not absence)', function () { + assert.throws( + () => BeforeRequestSentParameters.fromWire({ context: null, isBlocked: null, timestamp: 1 }), + ValidationError, + ) + }) + + it('warns (not throws) on an undeclared property', async function () { + BeforeRequestSentParameters.fromWire({ context: null, isBlocked: true, timestamp: 1, vendorAttr: 'x' }) + await new Promise((resolve) => setTimeout(resolve, 20)) + assert.ok(warnings.some((m) => m.includes('vendorAttr'))) + }) + + it('rejects a non-object payload', function () { + assert.throws(() => BeforeRequestSentParameters.fromWire('not an object'), ValidationError) + }) + }) + + describe('integer validation', function () { + it('accepts a whole number outbound', function () { + const params = new BeforeRequestSentParameters({ context: null, isBlocked: true, timestamp: 2.0 }) + assert.strictEqual(params.timestamp, 2) + }) + + it('rejects a fractional value outbound', function () { + assert.throws( + () => new BeforeRequestSentParameters({ context: null, isBlocked: true, timestamp: 1.5 }), + ValidationError, + ) + }) + + it('rejects a fractional value inbound', function () { + assert.throws( + () => BeforeRequestSentParameters.fromWire({ context: null, isBlocked: true, timestamp: 1.5 }), + ValidationError, + ) + }) + }) + + describe('extensible types', function () { + const ExtensibleParams = defineRecord( + 'test.record.ExtensibleParams', + [{ name: 'proxyType', wire: 'proxyType', required: true, type: { const: 'autodetect' } }], + { extensible: true }, + ) + + it('lets outbound vendor extras reach the wire', function () { + const params = new ExtensibleParams({ proxyType: 'autodetect', 'vendor:flag': true }) + assert.strictEqual(params['vendor:flag'], true) + }) + + it('retains an inbound undeclared property silently — every extensible type does', async function () { + const warnings = [] + const onWarning = (w) => warnings.push(w.message) + process.on('warning', onWarning) + const parsed = ExtensibleParams.fromWire({ proxyType: 'autodetect', 'vendor:flag': 'x' }) + await new Promise((resolve) => setTimeout(resolve, 20)) + process.off('warning', onWarning) + assert.strictEqual(parsed['vendor:flag'], 'x') + assert.ok(warnings.every((m) => !m.includes('vendor:flag'))) + }) + + // CWE-1321: a literal "__proto__" key is a real, iterable own property once + // JSON.parse builds an object from wire text — but assigning through it with + // `instance[key] = value` invokes Object.prototype's __proto__ accessor and + // hijacks the instance's actual prototype instead of storing a field. + it('does not let a "__proto__" wire key hijack the parsed instance inbound', function () { + const raw = '{"proxyType":"autodetect","__proto__":{"pwned":true}}' + const parsed = ExtensibleParams.fromWire(JSON.parse(raw)) + assert.strictEqual(Object.getPrototypeOf(parsed), ExtensibleParams.prototype) + assert.ok(parsed instanceof ExtensibleParams) + assert.deepStrictEqual(parsed.__proto__, { pwned: true }) // preserved as data, not applied as a prototype + }) + + it('does not let a "__proto__" key hijack the constructed instance outbound', function () { + const raw = '{"proxyType":"autodetect","__proto__":{"pwned":true}}' + const built = new ExtensibleParams(JSON.parse(raw)) + assert.strictEqual(Object.getPrototypeOf(built), ExtensibleParams.prototype) + assert.ok(built instanceof ExtensibleParams) + assert.deepStrictEqual(built.__proto__, { pwned: true }) + }) + }) +}) diff --git a/javascript/selenium-webdriver/test/bidi/serialization/union_test.js b/javascript/selenium-webdriver/test/bidi/serialization/union_test.js new file mode 100644 index 0000000000000..c8bbf5b1fe9fe --- /dev/null +++ b/javascript/selenium-webdriver/test/bidi/serialization/union_test.js @@ -0,0 +1,117 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +'use strict' + +const assert = require('node:assert') +const { defineRecord, ValidationError } = require('selenium-webdriver/bidi/serialization/record') +const { defineUnion } = require('selenium-webdriver/bidi/serialization/union') + +// Real fixtures: session.ProxyConfiguration (discriminated by `proxyType`) and +// script.RemoteReference (structural, presence-based — no shared discriminator). +const AutodetectProxyConfiguration = defineRecord( + 'test.union.AutodetectProxyConfiguration', + [{ name: 'proxyType', wire: 'proxyType', required: true, type: { const: 'autodetect' } }], + { extensible: true }, +) +const ManualProxyConfiguration = defineRecord( + 'test.union.ManualProxyConfiguration', + [ + { name: 'proxyType', wire: 'proxyType', required: true, type: { const: 'manual' } }, + { name: 'socksProxy', wire: 'socksProxy', required: true, type: { primitive: 'string' } }, + ], + { extensible: true }, +) +const ProxyConfiguration = defineUnion( + 'test.union.ProxyConfiguration', + { + by: 'proxyType', + variants: [ + { value: 'autodetect', ref: 'test.union.AutodetectProxyConfiguration' }, + { value: 'manual', ref: 'test.union.ManualProxyConfiguration' }, + ], + }, + { objectOnly: true }, +) + +const SharedReference = defineRecord('test.union.SharedReference', [ + { name: 'sharedId', wire: 'sharedId', required: true, type: { primitive: 'string' } }, +]) +const RemoteObjectReference = defineRecord('test.union.RemoteObjectReference', [ + { name: 'handle', wire: 'handle', required: true, type: { primitive: 'string' } }, +]) +const RemoteReference = defineUnion( + 'test.union.RemoteReference', + { + ordered: [ + { ref: 'test.union.SharedReference', requires: ['sharedId'] }, + { ref: 'test.union.RemoteObjectReference', requires: ['handle'] }, + ], + }, + { objectOnly: true }, +) + +describe('serialization/union', function () { + describe('discriminated (selector.by)', function () { + it('dispatches outbound to the variant matching the discriminator', function () { + const manual = ProxyConfiguration.build({ proxyType: 'manual', socksProxy: 'localhost:9' }) + assert.ok(manual instanceof ManualProxyConfiguration) + assert.strictEqual(manual.socksProxy, 'localhost:9') + }) + + it('rejects an outbound value with an unresolvable discriminator', function () { + assert.throws(() => ProxyConfiguration.build({ proxyType: 'bogus' }), ValidationError) + }) + + it('dispatches inbound to the variant matching the discriminator', function () { + const parsed = ProxyConfiguration.fromWire({ proxyType: 'manual', socksProxy: 'localhost:9' }) + assert.ok(parsed instanceof ManualProxyConfiguration) + }) + + it('errors (not warns) on an inbound payload whose discriminator matches no known variant', function () { + assert.throws(() => ProxyConfiguration.fromWire({ proxyType: 'notARealType' }), ValidationError) + }) + + it('retains extras on an extensible variant silently', async function () { + const warnings = [] + const onWarning = (w) => warnings.push(w.message) + process.on('warning', onWarning) + const parsed = ProxyConfiguration.fromWire({ proxyType: 'autodetect', 'vendor:flag': 'x' }) + await new Promise((resolve) => setTimeout(resolve, 20)) + process.off('warning', onWarning) + assert.ok(parsed instanceof AutodetectProxyConfiguration) + assert.strictEqual(parsed['vendor:flag'], 'x') + assert.ok(warnings.every((m) => !m.includes('vendor:flag'))) + }) + }) + + describe('structural (selector.ordered)', function () { + it('dispatches to the first variant whose required keys are present', function () { + const shared = RemoteReference.fromWire({ sharedId: 'abc' }) + assert.ok(shared instanceof SharedReference) + }) + + it('dispatches to a later variant when its required keys are present instead', function () { + const remoteObj = RemoteReference.fromWire({ handle: 'h1' }) + assert.ok(remoteObj instanceof RemoteObjectReference) + }) + + it('errors when no variant matches', function () { + assert.throws(() => RemoteReference.fromWire({ somethingElse: true }), ValidationError) + }) + }) +}) diff --git a/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js b/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js new file mode 100644 index 0000000000000..f7583bbb6c369 --- /dev/null +++ b/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js @@ -0,0 +1,267 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +'use strict' + +// One describe block per behavioral guarantee this layer makes about a value +// crossing the wire — how it's represented, what's enforced sending it out, what's +// enforced receiving it back. record_test.js/union_test.js cover the serialization +// primitives more exhaustively; this file exists to walk each guarantee in +// isolation, one realistic fixture at a time, rather than to avoid overlap with them. + +const assert = require('node:assert') +const { defineRecord, ValidationError } = require('selenium-webdriver/bidi/serialization/record') +const { defineEnum } = require('selenium-webdriver/bidi/serialization/enum') +const { Domain, DOMAIN_TOKEN } = require('selenium-webdriver/bidi/domain') + +defineEnum('test.contract.InterceptPhase', ['beforeRequestSent', 'responseStarted']) + +// Mirrors network.BeforeRequestSentParameters' shape closely enough to exercise every +// structural/vocabulary case that must be rejected: a nullable ref, a non-nullable +// primitive, an integer, a list, an enum, and a nullable const. +const RecordFixture = defineRecord('test.contract.Record', [ + { name: 'context', wire: 'context', required: true, type: { ref: 'test.contract.BrowsingContext', nullable: true } }, + { name: 'isBlocking', wire: 'isBlocking', required: true, type: { primitive: 'boolean' } }, + { name: 'timestamp', wire: 'timestamp', required: true, type: { primitive: 'integer' } }, + { name: 'headers', wire: 'headers', required: false, type: { list: { primitive: 'string' } } }, + { name: 'phase', wire: 'phase', required: false, type: { ref: 'test.contract.InterceptPhase' } }, + { name: 'proxyType', wire: 'proxyType', required: false, type: { const: 'autodetect', nullable: true } }, +]) + +const ExtensibleFixture = defineRecord( + 'test.contract.Extensible', + [{ name: 'acceptInsecureCerts', wire: 'acceptInsecureCerts', required: false, type: { primitive: 'boolean' } }], + { extensible: true }, +) + +// Never constructed outbound anywhere in this file — stands in for a received-only +// extensible type (e.g. a result type no command ever takes as params). Undeclared-field +// retention applies to every extensible type, not just ones a caller can also send. +const ReceivedOnlyExtensibleFixture = defineRecord( + 'test.contract.ReceivedOnlyExtensible', + [{ name: 'realm', wire: 'realm', required: true, type: { primitive: 'string' } }], + { extensible: true }, +) + +async function captureWarnings(fn) { + const warnings = [] + const onWarning = (w) => warnings.push(w.message) + process.on('warning', onWarning) + try { + return { result: await fn(), warnings: await settle(warnings) } + } finally { + process.off('warning', onWarning) + } +} +function settle(warnings) { + return new Promise((resolve) => setTimeout(() => resolve(warnings), 20)) +} + +describe('wire contract — representation', function () { + describe('typed objects, not raw maps', function () { + it('a record is a real typed instance, not a plain object', function () { + const parsed = RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1 }) + assert.ok(parsed instanceof RecordFixture) + }) + + it('a non-extensible type has no map for undeclared fields — an extra key is rejected outbound', function () { + assert.throws( + () => new RecordFixture({ context: null, isBlocking: true, timestamp: 1, vendorFlag: true }), + ValidationError, + ) + }) + + it('an extensible type carries undeclared fields directly on the instance', function () { + const built = new ExtensibleFixture({ acceptInsecureCerts: true, 'vendor:flag': 'x' }) + assert.strictEqual(built['vendor:flag'], 'x') + }) + + it('a key the type declares can never be treated as an extra, even alongside it', function () { + const built = new ExtensibleFixture({ acceptInsecureCerts: true }) + assert.strictEqual(built.acceptInsecureCerts, true) + assert.strictEqual(Object.keys(built).includes('acceptInsecureCerts'), true) + }) + }) + + describe("mirror the spec's command and field names", function () { + // The wire key (what the spec/generator uses verbatim) and the JS-facing property + // name are deliberately separate slots in a FieldSpec — this is the mechanism that + // lets the generator mirror the spec's own key precisely, independent of whatever + // the language-idiomatic property name happens to be (for BiDi/JS these are the same + // string in practice, since BiDi's wire format is already camelCase). + const NameMirror = defineRecord('test.contract.NameMirror', [ + { name: 'jsPropertyName', wire: 'specWireKey', required: true, type: { primitive: 'string' } }, + ]) + + it('reads from the literal wire key, not the JS property name', function () { + const parsed = NameMirror.fromWire({ specWireKey: 'hello' }) + assert.strictEqual(parsed.jsPropertyName, 'hello') + assert.strictEqual(Object.hasOwn(parsed, 'specWireKey'), false) + }) + }) + + describe("preserve a numeric value's full range and precision", function () { + it('does not narrow or lose precision at the js-uint/js-int boundary', function () { + const parsed = RecordFixture.fromWire({ + context: null, + isBlocking: true, + timestamp: Number.MAX_SAFE_INTEGER, + }) + assert.strictEqual(parsed.timestamp, Number.MAX_SAFE_INTEGER) + }) + }) + + describe('hold a value strictly to its declared type', function () { + it('structural: null in a non-nullable field is invalid', function () { + assert.throws(() => RecordFixture.fromWire({ context: null, isBlocking: null, timestamp: 1 }), ValidationError) + }) + + it('structural: an incorrect primitive type is invalid', function () { + assert.throws(() => RecordFixture.fromWire({ context: null, isBlocking: 'yes', timestamp: 1 }), ValidationError) + }) + + it('structural: a fractional value where integer is declared is invalid', function () { + assert.throws(() => RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1.5 }), ValidationError) + }) + + it('structural: a cardinality mismatch (single value where a list is declared) is invalid', function () { + assert.throws( + () => RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, headers: 'not-a-list' }), + ValidationError, + ) + }) + + it('vocabulary: an enum value outside its defined set is invalid', function () { + assert.throws( + () => RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, phase: 'notARealPhase' }), + ValidationError, + ) + }) + + it('vocabulary: a nullable constant set to anything other than its literal or null is invalid', function () { + assert.throws( + () => RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, proxyType: 'manual' }), + ValidationError, + ) + // ...but the literal and null are both fine. + assert.doesNotThrow(() => + RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, proxyType: 'autodetect' }), + ) + assert.doesNotThrow(() => + RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, proxyType: null }), + ) + }) + }) +}) + +describe('wire contract — outbound', function () { + describe('reject an invalid or missing value', function () { + it('rejects a missing required field', function () { + assert.throws(() => new RecordFixture({ context: null, timestamp: 1 }), ValidationError) + }) + + it('rejects an invalid value the same way inbound does', function () { + assert.throws(() => new RecordFixture({ context: null, isBlocking: true, timestamp: 1.5 }), ValidationError) + }) + }) + + describe('send an extra field only on an extensible type', function () { + it('a non-extensible type cannot represent an extra field outbound', function () { + assert.throws( + () => new RecordFixture({ context: null, isBlocking: true, timestamp: 1, vendorFlag: true }), + ValidationError, + ) + }) + + it('an extensible type serializes the extra field', function () { + const built = new ExtensibleFixture({ 'vendor:flag': true }) + assert.strictEqual(built['vendor:flag'], true) + }) + }) +}) + +describe('wire contract — inbound', function () { + describe('reject an invalid value', function () { + it('a present-but-invalid value always errors, never falls back to a placeholder', function () { + assert.throws( + () => RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, headers: 'nope' }), + ValidationError, + ) + }) + }) + + describe('a missing required field', function () { + it('rejects a missing required field, same as a present-but-invalid one', function () { + assert.throws(() => RecordFixture.fromWire({ context: null, timestamp: 1 }), ValidationError) + }) + }) + + describe('tolerate an undeclared field', function () { + it('an extensible type retains it silently — no warning on the preserved path', async function () { + const { result: parsed, warnings } = await captureWarnings(() => + ExtensibleFixture.fromWire({ 'vendor:flag': 'x' }), + ) + assert.strictEqual(parsed['vendor:flag'], 'x') + assert.ok(warnings.every((m) => !m.includes('vendor:flag'))) + }) + + it('a non-extensible type drops it and warns — the warning belongs to the drop', async function () { + const { result: parsed, warnings } = await captureWarnings(() => + RecordFixture.fromWire({ context: null, isBlocking: true, timestamp: 1, vendorFlag: 'x' }), + ) + assert.strictEqual(parsed.vendorFlag, undefined) + assert.ok(warnings.some((m) => m.includes('vendorFlag'))) + }) + + // Previously a known gap: the generator only retained extras on an extensible type + // that was also "re-sendable" (reachable from a command's params) — an unnecessarily + // narrow criterion. Closed once project_bidi_schema.mjs (#17864) stopped computing + // that heuristic and started deriving clean inbound/outbound reachability instead — + // retention now follows `extensible` alone, with no narrower criterion, for every + // type including one never sent as a command's params. + it('a received-only extensible type retains it too — no narrower criterion than "extensible"', async function () { + const { result: parsed, warnings } = await captureWarnings(() => + ReceivedOnlyExtensibleFixture.fromWire({ realm: 'realm-1', 'vendor:flag': 'x' }), + ) + assert.strictEqual(parsed['vendor:flag'], 'x') + assert.ok(warnings.every((m) => !m.includes('vendor:flag'))) + }) + }) + + describe('preserve received values faithfully', function () { + it('does not truncate, round, or re-case a value', async function () { + const { result: parsed } = await captureWarnings(() => + RecordFixture.fromWire({ + context: null, + isBlocking: true, + timestamp: 1732000000123, + headers: ['X-Custom-Header', 'Another-One'], + }), + ) + assert.strictEqual(parsed.timestamp, 1732000000123) + assert.deepStrictEqual(parsed.headers, ['X-Custom-Header', 'Another-One']) + }) + }) +}) + +describe('wire contract — error responses are processed before payload validation', function () { + it('surfaces the remote error without ever reaching payload validation', async function () { + const fakeBidi = { send: async () => ({ error: 'unknown command', message: 'not implemented' }) } + const domain = new Domain(fakeBidi, DOMAIN_TOKEN) + await assert.rejects(domain.send('network.addIntercept', {}), /unknown command: not implemented/) + }) +}) From e8868ce1d605b029ef85f392cd31d9f71eb0a27d Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Mon, 24 Aug 2026 13:41:56 +0530 Subject: [PATCH 2/5] [js] Addressed comments --- .../selenium-webdriver/bidi/domain.d.ts | 17 +- javascript/selenium-webdriver/bidi/domain.js | 37 +++- .../bidi/serialization/enum.d.ts | 1 + .../bidi/serialization/enum.js | 6 +- .../bidi/serialization/record.d.ts | 7 + .../bidi/serialization/record.js | 142 +++++++++++---- .../bidi/serialization/union.d.ts | 4 + .../bidi/serialization/union.js | 5 + .../test/bidi/domain_test.js | 83 +++++++-- .../test/bidi/serialization/record_test.js | 164 +++++++++++++++++- .../bidi/serialization/wire_contract_test.js | 7 + 11 files changed, 412 insertions(+), 61 deletions(-) diff --git a/javascript/selenium-webdriver/bidi/domain.d.ts b/javascript/selenium-webdriver/bidi/domain.d.ts index 8f5a0be49d1cd..1444e69a51fb1 100644 --- a/javascript/selenium-webdriver/bidi/domain.d.ts +++ b/javascript/selenium-webdriver/bidi/domain.d.ts @@ -20,6 +20,7 @@ export interface EventDescriptor { readonly type?: { fromWire(payload: unknown): T } } +/** Describes one subscribable BiDi event, for use with Domain#addCallback(). */ export function event(method: string, type?: { fromWire(payload: unknown): T }): EventDescriptor /** Internal construction guard — only a generated `Class.create(driver)` passes this. Never use directly. */ @@ -29,9 +30,21 @@ export declare class Domain { protected constructor(bidi: unknown, token: typeof DOMAIN_TOKEN) protected static connect(driver: unknown): Promise protected send(method: string, params: Record): Promise + + /** + * Subscribes `handler` to a BiDi event — asks the remote end to start + * sending it (only if nothing else on this connection already has), then + * attaches `handler` as a local listener for it. Remote subscription is + * ref-counted against the connection's own listener count, so it's shared + * correctly across every Domain instance on that connection, not just this one. + * @param descriptor An event descriptor from event(). + * @param handler Invoked with the event's parsed params each time it fires. + * @returns A handle for this subscription; call `unsubscribe()` to stop + * receiving the event (and, if it was the last listener for it on this + * connection, to tell the remote end to stop sending it). + */ addCallback( descriptor: EventDescriptor, handler: (params: T) => void, - ): Promise<{ id: string; unsubscribe(): Promise }> - removeCallback(subscriptionId: string): Promise + ): Promise<{ unsubscribe(): Promise }> } diff --git a/javascript/selenium-webdriver/bidi/domain.js b/javascript/selenium-webdriver/bidi/domain.js index cfedab53ecb9a..1cc3ba012dae2 100644 --- a/javascript/selenium-webdriver/bidi/domain.js +++ b/javascript/selenium-webdriver/bidi/domain.js @@ -27,6 +27,7 @@ const { getBidiConnection } = require('../lib/bidi_connection') const DOMAIN_TOKEN = Symbol('Domain internal construction token — obtained only via Class.create(driver)') /** + * Describes one subscribable BiDi event, for use with Domain#addCallback(). * @param {string} method * @param {{fromWire(payload: unknown): unknown}} [type] Runtime record/union * class for the event's params, if the schema declares one. When present, @@ -35,6 +36,7 @@ const DOMAIN_TOKEN = Symbol('Domain internal construction token — obtained onl * their resolved type; an event's params is such a payload just as much * as a command's result is. * @returns {{method: string, type: ({fromWire(payload: unknown): unknown}|undefined)}} + * The event descriptor, ready to pass to Domain#addCallback(). */ function event(method, type) { return { method, type } @@ -63,13 +65,38 @@ class Domain { return response?.result } + /** + * Subscribes `handler` to a BiDi event. Two distinct things happen: remote + * subscription (telling the browser to start sending this event at all, + * via the underlying connection's `subscribe()`) and local listening + * (attaching `handler` to the connection so it runs when the event + * arrives) — a descriptor's event only ever reaches `handler` once both + * are in place. Remote subscription/unsubscription is ref-counted against + * the connection's own listener count — shared truth across every Domain + * instance on the same connection — so a second caller subscribing to the + * same event doesn't re-subscribe remotely, and unsubscribing doesn't cut + * off another caller still listening for it. + * @param {{method: string, type: ({fromWire(payload: unknown): unknown}|undefined)}} descriptor + * An event descriptor from event(). + * @param {function(unknown): void} handler Invoked with the event's params + * (parsed through descriptor.type first, if one was given) each time it fires. + * @returns {Promise<{unsubscribe: function(): Promise}>} + * A handle for this subscription — call `unsubscribe()` to stop receiving the event. + */ async addCallback(descriptor, handler) { const dispatch = descriptor.type === undefined ? handler : (params) => handler(descriptor.type.fromWire(params)) - return this.#bidi.addCallback(descriptor.method, dispatch) - } - - async removeCallback(subscriptionId) { - return this.#bidi.removeCallback(subscriptionId) + if (this.#bidi.listenerCount(descriptor.method) === 0) { + await this.#bidi.subscribe(descriptor.method) + } + this.#bidi.on(descriptor.method, dispatch) + return { + unsubscribe: async () => { + this.#bidi.off(descriptor.method, dispatch) + if (this.#bidi.listenerCount(descriptor.method) === 0) { + await this.#bidi.unsubscribe(descriptor.method) + } + }, + } } } diff --git a/javascript/selenium-webdriver/bidi/serialization/enum.d.ts b/javascript/selenium-webdriver/bidi/serialization/enum.d.ts index b40ca8531c46e..9a3334f6fbbae 100644 --- a/javascript/selenium-webdriver/bidi/serialization/enum.d.ts +++ b/javascript/selenium-webdriver/bidi/serialization/enum.d.ts @@ -20,4 +20,5 @@ export interface EnumEntry { includes(value: unknown): value is T } +/** Registers a schema `enum` — a closed set of string values a field may hold. */ export function defineEnum(name: string, values: readonly T[]): EnumEntry diff --git a/javascript/selenium-webdriver/bidi/serialization/enum.js b/javascript/selenium-webdriver/bidi/serialization/enum.js index db16f0ca243a5..339c2d06b6515 100644 --- a/javascript/selenium-webdriver/bidi/serialization/enum.js +++ b/javascript/selenium-webdriver/bidi/serialization/enum.js @@ -18,8 +18,12 @@ const { register } = require('./registry') /** + * Registers a schema `enum` — a closed set of string values a field may hold. * @param {string} name Schema type name, e.g. 'network.InterceptPhase'. - * @param {string[]} values + * @param {string[]} values The enum's valid values. + * @returns {{kind: 'enum', values: string[], includes: function(unknown): boolean}} + * The registered entry, used by validateValue() to check a ref'd value's + * membership; also returned so a generator can build a discoverable constant from it. */ function defineEnum(name, values) { const allowed = new Set(values) diff --git a/javascript/selenium-webdriver/bidi/serialization/record.d.ts b/javascript/selenium-webdriver/bidi/serialization/record.d.ts index 843e5d3e5909d..6e2fb91afadfb 100644 --- a/javascript/selenium-webdriver/bidi/serialization/record.d.ts +++ b/javascript/selenium-webdriver/bidi/serialization/record.d.ts @@ -24,6 +24,9 @@ export interface TypeNode { list?: TypeNode map?: TypeNode union?: TypeNode[] + // An inline (unnamed) record — project_bidi_schema.mjs's projectEntry() emits this + // for an anonymous CDDL group instead of hoisting it to a named, ref'able type. + record?: FieldSpec[] nullable?: boolean // Present on an inline union with a bare-scalar arm — the primitive(s) that // arm accepts (see unionNode() in project_bidi_schema.mjs). Not consumed by @@ -53,6 +56,10 @@ export interface RecordClass { fromWire(payload: unknown): Readonly } +/** + * Registers a schema `record` — a fixed set of named fields, each independently + * validated on the way out (constructor) and in (fromWire()). + */ export function defineRecord(name: string, fields: FieldSpec[], options?: RecordOptions): RecordClass export function defineAlias(name: string, type: TypeNode): void diff --git a/javascript/selenium-webdriver/bidi/serialization/record.js b/javascript/selenium-webdriver/bidi/serialization/record.js index b39a8c29af7d3..3f8c983a1b3d5 100644 --- a/javascript/selenium-webdriver/bidi/serialization/record.js +++ b/javascript/selenium-webdriver/bidi/serialization/record.js @@ -19,15 +19,19 @@ const { register, resolve } = require('./registry') class ValidationError extends Error {} -// Validates a *present* value against a resolved type node. This check is -// identical for outbound and inbound — a structurally wrong value is always -// an error, whether it's being sent or received. Only presence (required) -// and extras (undeclared properties) differ by direction, handled separately -// in the constructor and fromWire() below. -// `direction` only affects how a nested ref-to-record/union is itself validated. +// Validates a *present* value against a resolved type node, and returns the value to +// assign for it — usually the same value, but for a nested ref-to-record/union parsed +// inbound (fromWire()/build() results, not the raw wire object) and for list/map/inline +// record values (a fresh, deeply-frozen copy, so validity can't be corrupted after the +// fact by mutating an array or object a caller still holds a reference to). +// Outbound nested refs deliberately keep the caller's own value instead of the +// newly-constructed instance — the nested `new referenced.RecordClass(value)` / +// `referenced.build(value)` call below exists only to validate, matching the +// constructor's existing outbound behavior of trusting the caller's own object shape. +// `direction` only affects how a nested ref-to-record/union is itself validated/parsed. function validateValue(typeNode, value, path, direction) { if (value === null) { - if (typeNode.nullable) return + if (typeNode.nullable) return null throw new ValidationError(`${path}: null is not allowed`) } @@ -36,12 +40,22 @@ function validateValue(typeNode, value, path, direction) { if (expected && typeof value !== expected) { throw new ValidationError(`${path}: expected ${typeNode.primitive}, got ${typeof value}`) } + // JSON has no representation for NaN/±Infinity — reject them for both numeric + // primitives before the integer-specific check narrows further. (Number.isInteger + // already excludes them too, so this is only load-bearing for a bare `number`.) + if ((typeNode.primitive === 'integer' || typeNode.primitive === 'number') && !Number.isFinite(value)) { + throw new ValidationError(`${path}: expected a finite ${typeNode.primitive}, got ${value}`) + } // `number` admits any JSON number; `integer` rejects a fractional value // (5.7) while still accepting one written 5.0 (Number.isInteger(5.0) is true). if (typeNode.primitive === 'integer' && !Number.isInteger(value)) { throw new ValidationError(`${path}: expected an integer, got ${value}`) } - return + // An inline literal choice (project_bidi_schema.mjs's enumNode()) carries both + // `primitive` and `enum` — the primitive check above narrows the type, but the + // closed vocabulary below still needs to run, so only return early when there + // is no `enum` to fall through to. + if (typeNode.enum === undefined) return value } if (typeNode.const !== undefined) { @@ -50,7 +64,7 @@ function validateValue(typeNode, value, path, direction) { `${path}: expected constant ${JSON.stringify(typeNode.const)}, got ${JSON.stringify(value)}`, ) } - return + return value } if (typeNode.enum !== undefined) { @@ -59,30 +73,30 @@ function validateValue(typeNode, value, path, direction) { `${path}: "${value}" is not a valid value; expected one of: ${typeNode.enum.join(', ')}`, ) } - return + return value } if (typeNode.list !== undefined) { if (!Array.isArray(value)) { throw new ValidationError(`${path}: expected a list, got ${typeof value}`) } - value.forEach((item, i) => validateValue(typeNode.list, item, `${path}[${i}]`, direction)) - return + return Object.freeze(value.map((item, i) => validateValue(typeNode.list, item, `${path}[${i}]`, direction))) } if (typeNode.map !== undefined) { if (typeof value !== 'object' || Array.isArray(value) || value === null) { throw new ValidationError(`${path}: expected an object, got ${typeof value}`) } + const result = {} for (const [key, entry] of Object.entries(value)) { - validateValue(typeNode.map, entry, `${path}.${key}`, direction) + result[key] = validateValue(typeNode.map, entry, `${path}.${key}`, direction) } - return + return Object.freeze(result) } if (typeNode.ref !== undefined) { const referenced = resolve(typeNode.ref) - if (referenced === undefined) return // not yet registered — best-effort, skip deep validation + if (referenced === undefined) return value // not yet registered — best-effort, skip deep validation if (referenced.kind === 'enum') { if (!referenced.includes(value)) { @@ -90,63 +104,107 @@ function validateValue(typeNode, value, path, direction) { `${path}: "${value}" is not a valid ${typeNode.ref} value; expected one of: ${referenced.values.join(', ')}`, ) } - return + return value } if (referenced.kind === 'record') { - if (value instanceof referenced.RecordClass) return // already validated + if (value instanceof referenced.RecordClass) return value // already validated if (typeof value !== 'object' || Array.isArray(value) || value === null) { throw new ValidationError(`${path}: expected an object, got ${typeof value}`) } - // Recurse through the same-direction path so a nested field gets the - // same tolerance (inbound) or strictness (outbound) as its parent. + // Recurse through the same-direction path so a nested field gets the same + // tolerance (inbound) or strictness (outbound) as its parent. Inbound returns + // the parsed instance itself, so a typed parent record ends up with a typed + // nested value instead of the raw wire object; outbound only validates this + // way (the caller's own value is what gets kept, see the note above). if (direction === 'inbound') { - referenced.RecordClass.fromWire(value) - } else { - new referenced.RecordClass(value) + return referenced.RecordClass.fromWire(value) } - return + new referenced.RecordClass(value) + return value } if (referenced.kind === 'union') { if (direction === 'inbound') { - referenced.fromWire(value) - } else { - referenced.build(value) + return referenced.fromWire(value) } - return + referenced.build(value) + return value } if (referenced.kind === 'alias') { - validateValue(referenced.type, value, path, direction) - return + return validateValue(referenced.type, value, path, direction) } - return + return value } if (typeNode.union !== undefined) { const errors = [] for (const variant of typeNode.union) { try { - validateValue(variant, value, path, direction) - return + return validateValue(variant, value, path, direction) } catch (err) { errors.push(err.message) } } throw new ValidationError(`${path}: value did not match any variant (${errors.join('; ')})`) } + + // An inline (unnamed) record type node — project_bidi_schema.mjs's projectEntry() + // emits this for an anonymous CDDL group (e.g. a field typed as an inline `{ ... }` + // rather than a ref to a named, defineRecord()'d type) — same FieldSpec shape as a + // named record's `fields`, just with nowhere to register a class. Gets the same + // directional required/extra/nested-value handling a named record's constructor/ + // fromWire gives its fields, just built inline instead of through a Record class. + if (typeNode.record !== undefined) { + if (typeof value !== 'object' || Array.isArray(value) || value === null) { + throw new ValidationError(`${path}: expected an object, got ${typeof value}`) + } + const byWire = new Map(typeNode.record.map((f) => [f.wire, f])) + const result = {} + for (const field of typeNode.record) { + if (!Object.hasOwn(value, field.wire)) { + if (field.required) { + throw new ValidationError(`${path}.${field.wire}: required field is missing`) + } + continue + } + result[field.name] = validateValue(field.type, value[field.wire], `${path}.${field.wire}`, direction) + } + for (const wireKey of Object.keys(value)) { + if (byWire.has(wireKey)) continue + // No `extensible` concept exists for an inline record (project_bidi_schema.mjs + // never sets it there) — undeclared keys get exactly the non-extensible named- + // record treatment: rejected outbound, dropped-with-a-warning inbound. + if (direction === 'inbound') { + process.emitWarning(`${path}: undeclared property "${wireKey}"`, 'BiDiSchemaWarning') + } else { + throw new ValidationError(`${path}: unknown property "${wireKey}"`) + } + } + return Object.freeze(result) + } + + return value } /** + * Registers a schema `record` — a fixed set of named fields, each independently + * validated on the way out (constructor) and in (fromWire()). * @param {string} name Schema type name, e.g. 'network.AddInterceptParameters'. * @param {Array<{name: string, wire: string, required: boolean, type: object}>} fields * @param {{extensible?: boolean}} [options] + * @returns {{new (data: object): object, fromWire: function(unknown): object}} + * The generated Record class — `new Record(data)` validates and constructs + * outbound, `Record.fromWire(payload)` validates and parses inbound. */ function defineRecord(name, fields, options = {}) { const { extensible = false } = options const byWire = new Map(fields.map((f) => [f.wire, f])) + // JS property name -> wire key, the inverse of byWire — lets toJSON() below map an + // outbound instance's own (JS-facing) properties back to the wire's declared names. + const byName = new Map(fields.map((f) => [f.name, f.wire])) class Record { // Outbound: strict. Any value that doesn't match its declared shape is an error here. @@ -163,8 +221,7 @@ function defineRecord(name, fields, options = {}) { continue } const value = data[field.wire] - validateValue(field.type, value, `${name}.${field.wire}`, 'outbound') - this[field.name] = value + this[field.name] = validateValue(field.type, value, `${name}.${field.wire}`, 'outbound') } for (const wireKey of Object.keys(data)) { @@ -209,8 +266,7 @@ function defineRecord(name, fields, options = {}) { } const value = payload[field.wire] // A present value's shape is never tolerated, inbound or outbound. - validateValue(field.type, value, `${name}.${field.wire}`, 'inbound') - instance[field.name] = value + instance[field.name] = validateValue(field.type, value, `${name}.${field.wire}`, 'inbound') } for (const wireKey of Object.keys(payload)) { @@ -237,6 +293,20 @@ function defineRecord(name, fields, options = {}) { Object.freeze(instance) return instance } + + // The JSON.stringify hook: an instance stores its fields under their JS-facing + // property names (this[field.name]), but the wire needs the spec's own key + // (field.wire) — the two differ whenever a generator picks an idiomatic JS name + // distinct from the raw spec key. Runs automatically wherever this instance is + // serialized (directly, or nested inside another value being stringified), so a + // caller never has to remember to call it. + toJSON() { + const wire = {} + for (const key of Object.keys(this)) { + wire[byName.get(key) ?? key] = this[key] // extras have no JS-name mapping — already wire-keyed + } + return wire + } } Object.defineProperty(Record, 'name', { value: name }) diff --git a/javascript/selenium-webdriver/bidi/serialization/union.d.ts b/javascript/selenium-webdriver/bidi/serialization/union.d.ts index 6f420168ce8af..37e4516530ea7 100644 --- a/javascript/selenium-webdriver/bidi/serialization/union.d.ts +++ b/javascript/selenium-webdriver/bidi/serialization/union.d.ts @@ -24,4 +24,8 @@ export interface UnionClass { fromWire(payload: unknown): Readonly } +/** + * Registers a schema `union` — a value that may be any one of several variant + * record types, resolved by a discriminator field or by structural shape. + */ export function defineUnion(name: string, selector: unknown, options?: UnionOptions): UnionClass diff --git a/javascript/selenium-webdriver/bidi/serialization/union.js b/javascript/selenium-webdriver/bidi/serialization/union.js index 4c6c987484a9c..b2ea7dcb80287 100644 --- a/javascript/selenium-webdriver/bidi/serialization/union.js +++ b/javascript/selenium-webdriver/bidi/serialization/union.js @@ -40,9 +40,14 @@ function selectVariant(selector, data, hasKey) { } /** + * Registers a schema `union` — a value that may be any one of several variant + * record types, resolved by a discriminator field or by structural shape. * @param {string} name Schema type name, e.g. 'session.ProxyConfiguration'. * @param {object} selector The schema's `selector` node for this union. * @param {{objectOnly?: boolean}} [options] + * @returns {{build: function(unknown): object, fromWire: function(unknown): object}} + * The registered union — `build(data)` resolves and constructs the matching + * variant outbound, `fromWire(payload)` resolves and parses it inbound. */ function defineUnion(name, selector, options = {}) { const { objectOnly = false } = options diff --git a/javascript/selenium-webdriver/test/bidi/domain_test.js b/javascript/selenium-webdriver/test/bidi/domain_test.js index 5f605235220ee..995c6ef651f28 100644 --- a/javascript/selenium-webdriver/test/bidi/domain_test.js +++ b/javascript/selenium-webdriver/test/bidi/domain_test.js @@ -18,6 +18,7 @@ 'use strict' const assert = require('node:assert') +const { EventEmitter } = require('node:events') const { Domain, event, DOMAIN_TOKEN } = require('selenium-webdriver/bidi/domain') const { defineRecord } = require('selenium-webdriver/bidi/serialization/record') @@ -25,19 +26,21 @@ const EntryAdded = defineRecord('test.domain.EntryAdded', [ { name: 'text', wire: 'text', required: true, type: { primitive: 'string' } }, ]) -// Fake replacing the real BiDi transport — records what addCallback/removeCallback -// receive and lets the test drive delivery directly, without a socket. +// Fake replacing the real BiDi transport (bidi/index.js's Index). Mirrors its +// actual shape — an EventEmitter with subscribe()/unsubscribe() added — rather +// than inventing its own addCallback/removeCallback surface, since Domain talks +// to the connection's real on/off/listenerCount/subscribe/unsubscribe directly. function fakeBidi() { - return { - registered: undefined, - async addCallback(method, handler) { - this.registered = { method, handler } - return { id: 'sub-1', unsubscribe: async () => {} } - }, - async removeCallback(subscriptionId) { - this.removedId = subscriptionId - }, + const bidi = new EventEmitter() + bidi.subscribeCalls = [] + bidi.unsubscribeCalls = [] + bidi.subscribe = async (method) => { + bidi.subscribeCalls.push(method) } + bidi.unsubscribe = async (method) => { + bidi.unsubscribeCalls.push(method) + } + return bidi } describe('Domain addCallback', function () { @@ -49,7 +52,7 @@ describe('Domain addCallback', function () { const received = [] await domain.addCallback(descriptor, (params) => received.push(params)) - bidi.registered.handler({ text: 'hello' }) + bidi.emit('test.entryAdded', { text: 'hello' }) assert.strictEqual(received.length, 1) assert.ok(received[0] instanceof EntryAdded) @@ -64,20 +67,68 @@ describe('Domain addCallback', function () { const received = [] await domain.addCallback(descriptor, (params) => received.push(params)) - bidi.registered.handler({ anything: 'goes' }) + bidi.emit('test.untyped', { anything: 'goes' }) assert.strictEqual(received.length, 1) assert.deepStrictEqual(received[0], { anything: 'goes' }) assert.ok(!(received[0] instanceof EntryAdded)) }) - it('removeCallback forwards the subscription id to the underlying transport', async function () { + it('subscribes remotely on the first listener, and not again for a second one', async function () { + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.entryAdded', EntryAdded) + + await domain.addCallback(descriptor, () => {}) + await domain.addCallback(descriptor, () => {}) + + assert.deepStrictEqual(bidi.subscribeCalls, ['test.entryAdded']) + }) + + it('unsubscribes remotely only once the last local listener is gone', async function () { const bidi = fakeBidi() const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.entryAdded', EntryAdded) + + const first = await domain.addCallback(descriptor, () => {}) + const second = await domain.addCallback(descriptor, () => {}) + + await first.unsubscribe() + assert.deepStrictEqual(bidi.unsubscribeCalls, []) + + await second.unsubscribe() + assert.deepStrictEqual(bidi.unsubscribeCalls, ['test.entryAdded']) + }) + + it('stops delivering to a handler once its own subscription is unsubscribed', async function () { + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const received = [] + const subscription = await domain.addCallback(descriptor, (params) => received.push(params)) + await subscription.unsubscribe() + + bidi.emit('test.untyped', { anything: 'goes' }) + + assert.strictEqual(received.length, 0) + }) + + it('does not disturb another still-active listener on the same event', async function () { + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const receivedByFirst = [] + const receivedBySecond = [] + const first = await domain.addCallback(descriptor, (params) => receivedByFirst.push(params)) + await domain.addCallback(descriptor, (params) => receivedBySecond.push(params)) - await domain.removeCallback('sub-1') + await first.unsubscribe() + bidi.emit('test.untyped', { anything: 'goes' }) - assert.strictEqual(bidi.removedId, 'sub-1') + assert.strictEqual(receivedByFirst.length, 0) + assert.strictEqual(receivedBySecond.length, 1) }) }) diff --git a/javascript/selenium-webdriver/test/bidi/serialization/record_test.js b/javascript/selenium-webdriver/test/bidi/serialization/record_test.js index 6c4e8c05bfdfd..d366b29923de1 100644 --- a/javascript/selenium-webdriver/test/bidi/serialization/record_test.js +++ b/javascript/selenium-webdriver/test/bidi/serialization/record_test.js @@ -19,7 +19,7 @@ const assert = require('node:assert') const { defineEnum } = require('selenium-webdriver/bidi/serialization/enum') -const { defineRecord, ValidationError } = require('selenium-webdriver/bidi/serialization/record') +const { defineRecord, defineAlias, ValidationError } = require('selenium-webdriver/bidi/serialization/record') // Real fixtures from the WebDriver BiDi schema (network.InterceptPhase, // network.AddInterceptParameters, network.BeforeRequestSentParameters), @@ -42,6 +42,15 @@ const BeforeRequestSentParameters = defineRecord('test.record.BeforeRequestSentP { name: 'timestamp', wire: 'timestamp', required: true, type: { primitive: 'integer' } }, ]) +// network.Intercept aliases a plain string — a name with no fields of its own, +// just a pointer to another type node. A ref to it should validate through +// whatever it points at. +defineAlias('test.record.Intercept', { primitive: 'string' }) + +const RemoveInterceptParameters = defineRecord('test.record.RemoveInterceptParameters', [ + { name: 'intercept', wire: 'intercept', required: true, type: { ref: 'test.record.Intercept' } }, +]) + describe('serialization/record', function () { describe('outbound (constructor)', function () { it('accepts a valid object', function () { @@ -143,6 +152,47 @@ describe('serialization/record', function () { ValidationError, ) }) + + it('rejects NaN and Infinity outbound', function () { + for (const bad of [NaN, Infinity, -Infinity]) { + assert.throws( + () => new BeforeRequestSentParameters({ context: null, isBlocked: true, timestamp: bad }), + ValidationError, + ) + } + }) + + it('rejects NaN and Infinity inbound', function () { + for (const bad of [NaN, Infinity, -Infinity]) { + assert.throws( + () => BeforeRequestSentParameters.fromWire({ context: null, isBlocked: true, timestamp: bad }), + ValidationError, + ) + } + }) + }) + + describe('number validation', function () { + const NumberField = defineRecord('test.record.NumberField', [ + { name: 'value', wire: 'value', required: true, type: { primitive: 'number' } }, + ]) + + it('accepts a fractional value, unlike integer', function () { + const built = new NumberField({ value: 1.5 }) + assert.strictEqual(built.value, 1.5) + }) + + it('rejects NaN and Infinity outbound', function () { + for (const bad of [NaN, Infinity, -Infinity]) { + assert.throws(() => new NumberField({ value: bad }), ValidationError) + } + }) + + it('rejects NaN and Infinity inbound', function () { + for (const bad of [NaN, Infinity, -Infinity]) { + assert.throws(() => NumberField.fromWire({ value: bad }), ValidationError) + } + }) }) describe('extensible types', function () { @@ -188,4 +238,116 @@ describe('serialization/record', function () { assert.deepStrictEqual(built.__proto__, { pwned: true }) }) }) + + describe('defineAlias', function () { + it('accepts, through a record field, a value matching the aliased type', function () { + const params = new RemoveInterceptParameters({ intercept: 'intercept-1' }) + assert.strictEqual(params.intercept, 'intercept-1') + }) + + it('rejects, through a record field, a value not matching the aliased type', function () { + assert.throws(() => new RemoveInterceptParameters({ intercept: 42 }), ValidationError) + }) + + it('validates the same way inbound', function () { + const parsed = RemoveInterceptParameters.fromWire({ intercept: 'intercept-1' }) + assert.strictEqual(parsed.intercept, 'intercept-1') + assert.throws(() => RemoveInterceptParameters.fromWire({ intercept: 42 }), ValidationError) + }) + }) + + describe('inline type nodes', function () { + // project_bidi_schema.mjs's enumNode() emits both `primitive` and `enum` on an + // inline literal choice the normalizer didn't hoist to a named enum. + const InlineEnumField = defineRecord('test.record.InlineEnumField', [ + { + name: 'phase', + wire: 'phase', + required: true, + type: { primitive: 'string', enum: ['beforeRequestSent', 'responseStarted'] }, + }, + ]) + + it('enforces the closed vocabulary of an inline enum, not just its shared primitive', function () { + assert.throws(() => new InlineEnumField({ phase: 'notARealPhase' }), ValidationError) + assert.doesNotThrow(() => new InlineEnumField({ phase: 'beforeRequestSent' })) + }) + + const InlineRecordField = defineRecord('test.record.InlineRecordField', [ + { + name: 'origin', + wire: 'origin', + required: true, + type: { + record: [ + { name: 'host', wire: 'host', required: true, type: { primitive: 'string' } }, + { name: 'port', wire: 'port', required: false, type: { primitive: 'integer' } }, + ], + }, + }, + ]) + + it('validates an inline record field the same way a named record is validated', function () { + const built = new InlineRecordField({ origin: { host: 'example.com', port: 443 } }) + assert.deepStrictEqual(built.origin, { host: 'example.com', port: 443 }) + }) + + it('rejects a missing required field inside an inline record', function () { + assert.throws(() => new InlineRecordField({ origin: { port: 443 } }), ValidationError) + }) + + it('rejects an unknown property inside an inline record outbound', function () { + assert.throws(() => new InlineRecordField({ origin: { host: 'x', bogus: true } }), ValidationError) + }) + + it('warns (not throws) on an unknown property inside an inline record inbound', async function () { + const warnings = [] + const onWarning = (w) => warnings.push(w.message) + process.on('warning', onWarning) + const parsed = InlineRecordField.fromWire({ origin: { host: 'x', bogus: true } }) + await new Promise((resolve) => setTimeout(resolve, 20)) + process.off('warning', onWarning) + assert.strictEqual(parsed.origin.bogus, undefined) + assert.ok(warnings.some((m) => m.includes('bogus'))) + }) + }) + + describe('nested ref parsing', function () { + const InnerRecord = defineRecord('test.record.InnerRecord', [ + { name: 'value', wire: 'value', required: true, type: { primitive: 'string' } }, + ]) + const OuterRecord = defineRecord('test.record.OuterRecord', [ + { name: 'inner', wire: 'inner', required: true, type: { ref: 'test.record.InnerRecord' } }, + ]) + + it('assigns the parsed nested record instance inbound, not the raw wire object', function () { + const parsed = OuterRecord.fromWire({ inner: { value: 'x' } }) + assert.ok(parsed.inner instanceof InnerRecord) + assert.strictEqual(parsed.inner.value, 'x') + }) + + it('keeps the caller-provided value outbound, not a newly constructed instance', function () { + const rawInner = { value: 'x' } + const built = new OuterRecord({ inner: rawInner }) + assert.strictEqual(built.inner, rawInner) // same reference — outbound behavior preserved + assert.ok(!(built.inner instanceof InnerRecord)) + }) + + it('still validates a nested ref outbound even though the raw value is kept', function () { + assert.throws(() => new OuterRecord({ inner: { value: 42 } }), ValidationError) + }) + }) + + describe('deep immutability', function () { + it('freezes a validated list so it cannot be mutated after construction', function () { + const params = new AddInterceptParameters({ phases: ['beforeRequestSent'] }) + assert.ok(Object.isFrozen(params.phases)) + assert.throws(() => params.phases.push('responseStarted'), TypeError) + }) + + it('freezes a validated list parsed inbound too', function () { + const parsed = AddInterceptParameters.fromWire({ phases: ['beforeRequestSent'] }) + assert.ok(Object.isFrozen(parsed.phases)) + }) + }) }) diff --git a/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js b/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js index f7583bbb6c369..b8da8ce6ea91a 100644 --- a/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js +++ b/javascript/selenium-webdriver/test/bidi/serialization/wire_contract_test.js @@ -112,6 +112,13 @@ describe('wire contract — representation', function () { assert.strictEqual(parsed.jsPropertyName, 'hello') assert.strictEqual(Object.hasOwn(parsed, 'specWireKey'), false) }) + + it('writes to the literal wire key, not the JS property name, when serialized', function () { + const built = new NameMirror({ specWireKey: 'hello' }) + assert.strictEqual(JSON.stringify(built), JSON.stringify({ specWireKey: 'hello' })) + // The instance itself is still JS-facing — only its wire representation changes. + assert.strictEqual(built.jsPropertyName, 'hello') + }) }) describe("preserve a numeric value's full range and precision", function () { From 9b8fb890c6b95d32e0e41e43f7f388bb746bec40 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Mon, 24 Aug 2026 14:30:58 +0530 Subject: [PATCH 3/5] Address comments --- .../selenium-webdriver/bidi/domain.d.ts | 20 ++- javascript/selenium-webdriver/bidi/domain.js | 61 ++++++++-- .../bidi/serialization/enum.d.ts | 8 +- .../bidi/serialization/record.d.ts | 5 + .../bidi/serialization/record.js | 12 +- .../bidi/serialization/union.d.ts | 5 + .../test/bidi/domain_test.js | 115 ++++++++++++++++++ .../test/bidi/serialization/record_test.js | 31 +++++ 8 files changed, 237 insertions(+), 20 deletions(-) diff --git a/javascript/selenium-webdriver/bidi/domain.d.ts b/javascript/selenium-webdriver/bidi/domain.d.ts index 1444e69a51fb1..e24f501c0dd30 100644 --- a/javascript/selenium-webdriver/bidi/domain.d.ts +++ b/javascript/selenium-webdriver/bidi/domain.d.ts @@ -20,7 +20,14 @@ export interface EventDescriptor { readonly type?: { fromWire(payload: unknown): T } } -/** Describes one subscribable BiDi event, for use with Domain#addCallback(). */ +/** + * Describes one subscribable BiDi event, for use with Domain#addCallback(). + * @param {string} method The event's wire method name, e.g. 'log.entryAdded'. + * @param {{fromWire(payload: unknown): T}} [type] Runtime record/union class for the + * event's params, if the schema declares one — addCallback() parses each delivered + * payload through it before the caller's handler runs. + * @returns {EventDescriptor} The event descriptor, ready to pass to Domain#addCallback(). + */ export function event(method: string, type?: { fromWire(payload: unknown): T }): EventDescriptor /** Internal construction guard — only a generated `Class.create(driver)` passes this. Never use directly. */ @@ -37,11 +44,12 @@ export declare class Domain { * attaches `handler` as a local listener for it. Remote subscription is * ref-counted against the connection's own listener count, so it's shared * correctly across every Domain instance on that connection, not just this one. - * @param descriptor An event descriptor from event(). - * @param handler Invoked with the event's parsed params each time it fires. - * @returns A handle for this subscription; call `unsubscribe()` to stop - * receiving the event (and, if it was the last listener for it on this - * connection, to tell the remote end to stop sending it). + * @param {EventDescriptor} descriptor An event descriptor from event(). + * @param {function(T): void} handler Invoked with the event's parsed params each time it fires. + * @returns {Promise<{unsubscribe: function(): Promise}>} A handle for this + * subscription; call `unsubscribe()` to stop receiving the event (and, if it + * was the last listener for it on this connection, to tell the remote end to + * stop sending it). */ addCallback( descriptor: EventDescriptor, diff --git a/javascript/selenium-webdriver/bidi/domain.js b/javascript/selenium-webdriver/bidi/domain.js index 1cc3ba012dae2..e669ac6dbfccf 100644 --- a/javascript/selenium-webdriver/bidi/domain.js +++ b/javascript/selenium-webdriver/bidi/domain.js @@ -42,6 +42,31 @@ function event(method, type) { return { method, type } } +// Serializes subscribe()/unsubscribe() transitions per (connection, method), so +// concurrent addCallback()/unsubscribe() calls touching the same event — from any +// Domain instance sharing that connection, not just one — can't interleave into an +// inconsistent remote subscription state (e.g. a listener left attached locally +// after a same-method unsubscribe-in-flight elsewhere wins the race and tells the +// browser to stop sending it). Keyed by the connection object itself via a WeakMap, +// not any one Domain instance — module-level, not Domain state, so Domain itself +// stays a plain, stateless wrapper around `send()` plus this queuing. +const subscriptionQueues = new WeakMap() + +function queueSubscriptionChange(bidi, method, change) { + let methods = subscriptionQueues.get(bidi) + if (methods === undefined) { + methods = new Map() + subscriptionQueues.set(bidi, methods) + } + const previous = methods.get(method) ?? Promise.resolve() + const next = previous.then(change, change) // run `change` next regardless of a prior failure + methods.set( + method, + next.catch(() => {}), + ) // ...but don't let that failure jam the queue for later callers + return next +} + /** Shared base for every generated BiDi domain class. See domain.d.ts for the typed surface. */ class Domain { #bidi @@ -75,7 +100,13 @@ class Domain { * the connection's own listener count — shared truth across every Domain * instance on the same connection — so a second caller subscribing to the * same event doesn't re-subscribe remotely, and unsubscribing doesn't cut - * off another caller still listening for it. + * off another caller still listening for it. The listener is attached + * before `subscribe()` is awaited (not after), so an event the browser + * starts sending as soon as it processes the subscription can't arrive in + * a gap where nothing is listening yet; and every subscribe/unsubscribe + * transition for this event, from any caller, is serialized (see + * queueSubscriptionChange()) so concurrent callers can't interleave into + * an inconsistent remote state. * @param {{method: string, type: ({fromWire(payload: unknown): unknown}|undefined)}} descriptor * An event descriptor from event(). * @param {function(unknown): void} handler Invoked with the event's params @@ -85,17 +116,25 @@ class Domain { */ async addCallback(descriptor, handler) { const dispatch = descriptor.type === undefined ? handler : (params) => handler(descriptor.type.fromWire(params)) - if (this.#bidi.listenerCount(descriptor.method) === 0) { - await this.#bidi.subscribe(descriptor.method) - } - this.#bidi.on(descriptor.method, dispatch) - return { - unsubscribe: async () => { - this.#bidi.off(descriptor.method, dispatch) - if (this.#bidi.listenerCount(descriptor.method) === 0) { - await this.#bidi.unsubscribe(descriptor.method) + await queueSubscriptionChange(this.#bidi, descriptor.method, async () => { + this.#bidi.on(descriptor.method, dispatch) + if (this.#bidi.listenerCount(descriptor.method) === 1) { + try { + await this.#bidi.subscribe(descriptor.method) + } catch (err) { + this.#bidi.off(descriptor.method, dispatch) // don't leak a listener for a subscription that never took + throw err } - }, + } + }) + return { + unsubscribe: () => + queueSubscriptionChange(this.#bidi, descriptor.method, async () => { + this.#bidi.off(descriptor.method, dispatch) + if (this.#bidi.listenerCount(descriptor.method) === 0) { + await this.#bidi.unsubscribe(descriptor.method) + } + }), } } } diff --git a/javascript/selenium-webdriver/bidi/serialization/enum.d.ts b/javascript/selenium-webdriver/bidi/serialization/enum.d.ts index 9a3334f6fbbae..4b0be54149e5e 100644 --- a/javascript/selenium-webdriver/bidi/serialization/enum.d.ts +++ b/javascript/selenium-webdriver/bidi/serialization/enum.d.ts @@ -20,5 +20,11 @@ export interface EnumEntry { includes(value: unknown): value is T } -/** Registers a schema `enum` — a closed set of string values a field may hold. */ +/** + * Registers a schema `enum` — a closed set of string values a field may hold. + * @param name Schema type name, e.g. 'network.InterceptPhase'. + * @param values The enum's valid values. + * @returns The registered entry, used by validateValue() to check a ref'd value's + * membership; also returned so a generator can build a discoverable constant from it. + */ export function defineEnum(name: string, values: readonly T[]): EnumEntry diff --git a/javascript/selenium-webdriver/bidi/serialization/record.d.ts b/javascript/selenium-webdriver/bidi/serialization/record.d.ts index 6e2fb91afadfb..39867e87275d7 100644 --- a/javascript/selenium-webdriver/bidi/serialization/record.d.ts +++ b/javascript/selenium-webdriver/bidi/serialization/record.d.ts @@ -59,6 +59,11 @@ export interface RecordClass { /** * Registers a schema `record` — a fixed set of named fields, each independently * validated on the way out (constructor) and in (fromWire()). + * @param name Schema type name, e.g. 'network.AddInterceptParameters'. + * @param fields The record's field specs. + * @param options + * @returns The generated Record class — `new Record(data)` validates and constructs + * outbound, `Record.fromWire(payload)` validates and parses inbound. */ export function defineRecord(name: string, fields: FieldSpec[], options?: RecordOptions): RecordClass diff --git a/javascript/selenium-webdriver/bidi/serialization/record.js b/javascript/selenium-webdriver/bidi/serialization/record.js index 3f8c983a1b3d5..78d9af02a17f2 100644 --- a/javascript/selenium-webdriver/bidi/serialization/record.js +++ b/javascript/selenium-webdriver/bidi/serialization/record.js @@ -87,7 +87,10 @@ function validateValue(typeNode, value, path, direction) { if (typeof value !== 'object' || Array.isArray(value) || value === null) { throw new ValidationError(`${path}: expected an object, got ${typeof value}`) } - const result = {} + // Object.create(null), not `{}`: `key` is wire-controlled and a literal "__proto__" + // entry assigned via bracket notation would hijack result's prototype instead of + // becoming a data property (CWE-1321) — a null-prototype object has no such trap. + const result = Object.create(null) for (const [key, entry] of Object.entries(value)) { result[key] = validateValue(typeNode.map, entry, `${path}.${key}`, direction) } @@ -301,7 +304,12 @@ function defineRecord(name, fields, options = {}) { // serialized (directly, or nested inside another value being stringified), so a // caller never has to remember to call it. toJSON() { - const wire = {} + // Object.create(null), not `{}`: an extra's key is wire-controlled (extensible + // types preserve undeclared properties verbatim, see the constructor above), and + // a literal "__proto__" key assigned via bracket notation would hijack wire's + // prototype instead of becoming a data property (CWE-1321) — same hazard the + // constructor/fromWire already guard against for the instance itself. + const wire = Object.create(null) for (const key of Object.keys(this)) { wire[byName.get(key) ?? key] = this[key] // extras have no JS-name mapping — already wire-keyed } diff --git a/javascript/selenium-webdriver/bidi/serialization/union.d.ts b/javascript/selenium-webdriver/bidi/serialization/union.d.ts index 37e4516530ea7..82c948467e689 100644 --- a/javascript/selenium-webdriver/bidi/serialization/union.d.ts +++ b/javascript/selenium-webdriver/bidi/serialization/union.d.ts @@ -27,5 +27,10 @@ export interface UnionClass { /** * Registers a schema `union` — a value that may be any one of several variant * record types, resolved by a discriminator field or by structural shape. + * @param name Schema type name, e.g. 'session.ProxyConfiguration'. + * @param selector The schema's `selector` node for this union. + * @param options + * @returns The registered union — `build(data)` resolves and constructs the matching + * variant outbound, `fromWire(payload)` resolves and parses it inbound. */ export function defineUnion(name: string, selector: unknown, options?: UnionOptions): UnionClass diff --git a/javascript/selenium-webdriver/test/bidi/domain_test.js b/javascript/selenium-webdriver/test/bidi/domain_test.js index 995c6ef651f28..b32c1085d1d7c 100644 --- a/javascript/selenium-webdriver/test/bidi/domain_test.js +++ b/javascript/selenium-webdriver/test/bidi/domain_test.js @@ -43,6 +43,33 @@ function fakeBidi() { return bidi } +// Same shape as fakeBidi(), but subscribe()/unsubscribe() stay pending until the +// test explicitly resolves them — widens the race window addCallback()/unsubscribe() +// must handle correctly (an event arriving mid-subscribe, two callers racing to +// subscribe/unsubscribe the same method) instead of hoping a real await happens to +// interleave the wrong way. +function controllableFakeBidi() { + const bidi = new EventEmitter() + bidi.subscribeCalls = [] + bidi.unsubscribeCalls = [] + const pendingSubscribes = [] + const pendingUnsubscribes = [] + bidi.subscribe = (method) => { + bidi.subscribeCalls.push(method) + return new Promise((resolve, reject) => pendingSubscribes.push({ resolve, reject })) + } + bidi.unsubscribe = (method) => { + bidi.unsubscribeCalls.push(method) + return new Promise((resolve, reject) => pendingUnsubscribes.push({ resolve, reject })) + } + bidi.resolveNextSubscribe = () => pendingSubscribes.shift().resolve() + bidi.rejectNextSubscribe = (err) => pendingSubscribes.shift().reject(err) + bidi.resolveNextUnsubscribe = () => pendingUnsubscribes.shift().resolve() + return bidi +} + +const tick = () => new Promise((resolve) => setTimeout(resolve, 0)) + describe('Domain addCallback', function () { it('parses delivered payloads through the descriptor type before the handler runs', async function () { const bidi = fakeBidi() @@ -130,6 +157,94 @@ describe('Domain addCallback', function () { assert.strictEqual(receivedByFirst.length, 0) assert.strictEqual(receivedBySecond.length, 1) }) + + describe('concurrency', function () { + it('does not miss an event that arrives while subscribe() is still pending', async function () { + const bidi = controllableFakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + const received = [] + + const addP = domain.addCallback(descriptor, (params) => received.push(params)) + await tick() // let addCallback reach its (still-pending) subscribe() call + assert.strictEqual(bidi.subscribeCalls.length, 1) + + bidi.emit('test.untyped', { anything: 'goes' }) // must already be listening, not just subscribing + bidi.resolveNextSubscribe() + await addP + + assert.strictEqual(received.length, 1) + }) + + it('removes the listener if subscribe() rejects, instead of leaking it', async function () { + const bidi = controllableFakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const addP = domain.addCallback(descriptor, () => {}) + await tick() + bidi.rejectNextSubscribe(new Error('boom')) + + await assert.rejects(addP, /boom/) + assert.strictEqual(bidi.listenerCount('test.untyped'), 0) + }) + + it('serializes concurrent addCallback calls for the same event so only one subscribe is sent', async function () { + const bidi = controllableFakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const first = domain.addCallback(descriptor, () => {}) + const second = domain.addCallback(descriptor, () => {}) + await tick() + + // The second call must not have started (and raced its own listenerCount + // check) before the first's subscribe() — still pending — resolves. + assert.strictEqual(bidi.subscribeCalls.length, 1) + assert.strictEqual(bidi.listenerCount('test.untyped'), 1) + + bidi.resolveNextSubscribe() + await first + await second + + assert.strictEqual(bidi.subscribeCalls.length, 1) + assert.strictEqual(bidi.listenerCount('test.untyped'), 2) + }) + + it('serializes an unsubscribe against a concurrent addCallback for the same event', async function () { + const bidi = controllableFakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const first = domain.addCallback(descriptor, () => {}) + await tick() + bidi.resolveNextSubscribe() + const subscription = await first + bidi.subscribeCalls.length = 0 // only the part under test matters from here + + // Start unsubscribing the only listener, then — before that finishes — start + // a second addCallback for the same event. Unserialized, the second caller + // could attach and see itself as remotely subscribed while the in-flight + // unsubscribe() is still telling the browser to stop sending the event, + // leaving it un-subscribed remotely despite a live local listener. + const unsubP = subscription.unsubscribe() + const secondP = domain.addCallback(descriptor, () => {}) + await tick() + + assert.strictEqual(bidi.unsubscribeCalls.length, 1) // unsubscribe() is in flight... + assert.strictEqual(bidi.subscribeCalls.length, 0) // ...and the second caller hasn't jumped ahead of it + + bidi.resolveNextUnsubscribe() + await unsubP + await tick() + bidi.resolveNextSubscribe() + await secondP + + assert.strictEqual(bidi.unsubscribeCalls.length, 1) + assert.strictEqual(bidi.subscribeCalls.length, 1) // re-subscribed only after unsubscribe() had finished + assert.strictEqual(bidi.listenerCount('test.untyped'), 1) + }) + }) }) describe('Domain construction guard', function () { diff --git a/javascript/selenium-webdriver/test/bidi/serialization/record_test.js b/javascript/selenium-webdriver/test/bidi/serialization/record_test.js index d366b29923de1..a51ad3d4febb2 100644 --- a/javascript/selenium-webdriver/test/bidi/serialization/record_test.js +++ b/javascript/selenium-webdriver/test/bidi/serialization/record_test.js @@ -237,6 +237,37 @@ describe('serialization/record', function () { assert.ok(built instanceof ExtensibleParams) assert.deepStrictEqual(built.__proto__, { pwned: true }) }) + + it('serializes a "__proto__" extra as data, not as the wire object\'s own prototype', function () { + const raw = '{"proxyType":"autodetect","__proto__":{"pwned":true}}' + const built = new ExtensibleParams(JSON.parse(raw)) + const wire = JSON.parse(JSON.stringify(built)) + assert.strictEqual(wire.proxyType, 'autodetect') + assert.strictEqual(Object.getPrototypeOf(wire), Object.prototype) // real prototype unaffected + assert.deepStrictEqual(wire.__proto__, { pwned: true }) // preserved as data, not applied as a prototype + }) + }) + + describe('map values', function () { + const MapField = defineRecord('test.record.MapField', [ + { name: 'headers', wire: 'headers', required: true, type: { map: { primitive: 'string' } } }, + ]) + + // CWE-1321: `result[key] = ...` with a wire-controlled key would hijack a plain + // `{}`'s prototype for a literal "__proto__" key; Object.create(null) has no such trap. + it('does not let a "__proto__" key hijack a validated map value inbound', function () { + const raw = '{"headers":{"__proto__":"pwned"}}' + const parsed = MapField.fromWire(JSON.parse(raw)) + assert.strictEqual(Object.getPrototypeOf(parsed.headers), null) + assert.strictEqual(Object.getOwnPropertyDescriptor(parsed.headers, '__proto__').value, 'pwned') + }) + + it('does not let a "__proto__" key hijack a validated map value outbound', function () { + const raw = '{"headers":{"__proto__":"pwned"}}' + const built = new MapField(JSON.parse(raw)) + assert.strictEqual(Object.getPrototypeOf(built.headers), null) + assert.strictEqual(Object.getOwnPropertyDescriptor(built.headers, '__proto__').value, 'pwned') + }) }) describe('defineAlias', function () { From 8bec29dc52d6275ac61a37377957e190bc32f127 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Mon, 24 Aug 2026 16:50:56 +0530 Subject: [PATCH 4/5] Address comments --- javascript/selenium-webdriver/bidi/domain.js | 18 +++++++++---- .../test/bidi/domain_test.js | 25 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/javascript/selenium-webdriver/bidi/domain.js b/javascript/selenium-webdriver/bidi/domain.js index e669ac6dbfccf..ef5113ac5e06e 100644 --- a/javascript/selenium-webdriver/bidi/domain.js +++ b/javascript/selenium-webdriver/bidi/domain.js @@ -49,7 +49,9 @@ function event(method, type) { // after a same-method unsubscribe-in-flight elsewhere wins the race and tells the // browser to stop sending it). Keyed by the connection object itself via a WeakMap, // not any one Domain instance — module-level, not Domain state, so Domain itself -// stays a plain, stateless wrapper around `send()` plus this queuing. +// stays a plain, stateless wrapper around `send()` plus this queuing. The per-method +// entry is pruned once idle (see below), so a long-lived connection subscribing to +// many distinct methods over its lifetime doesn't grow this map without bound. const subscriptionQueues = new WeakMap() function queueSubscriptionChange(bidi, method, change) { @@ -60,10 +62,16 @@ function queueSubscriptionChange(bidi, method, change) { } const previous = methods.get(method) ?? Promise.resolve() const next = previous.then(change, change) // run `change` next regardless of a prior failure - methods.set( - method, - next.catch(() => {}), - ) // ...but don't let that failure jam the queue for later callers + const queued = next.catch(() => {}) // don't let that failure jam the queue for later callers + methods.set(method, queued) + queued.finally(() => { + // Only this entry's own settlement prunes it, and only if nothing newer was + // enqueued for `method` in the meantime — otherwise this would delete a later + // caller's still-pending entry out from under it. + if (methods.get(method) === queued) { + methods.delete(method) + } + }) return next } diff --git a/javascript/selenium-webdriver/test/bidi/domain_test.js b/javascript/selenium-webdriver/test/bidi/domain_test.js index b32c1085d1d7c..745e020bcdfd2 100644 --- a/javascript/selenium-webdriver/test/bidi/domain_test.js +++ b/javascript/selenium-webdriver/test/bidi/domain_test.js @@ -158,6 +158,31 @@ describe('Domain addCallback', function () { assert.strictEqual(receivedBySecond.length, 1) }) + it('prunes its internal per-method subscription queue entry once idle', async function () { + // queueSubscriptionChange()'s per-method Map has no other seam to observe from + // outside the module, so temporarily watch Map.prototype.delete rather than + // exporting a test-only hook into the shipped module. + const bidi = fakeBidi() + const domain = new Domain(bidi, DOMAIN_TOKEN) + const descriptor = event('test.untyped') + + const originalDelete = Map.prototype.delete + const deletedKeys = [] + Map.prototype.delete = function (key) { + deletedKeys.push(key) + return originalDelete.call(this, key) + } + try { + const subscription = await domain.addCallback(descriptor, () => {}) + await subscription.unsubscribe() + await tick() // let the queued promise's own .finally() pruning run + } finally { + Map.prototype.delete = originalDelete + } + + assert.ok(deletedKeys.includes('test.untyped')) + }) + describe('concurrency', function () { it('does not miss an event that arrives while subscribe() is still pending', async function () { const bidi = controllableFakeBidi() From 4670a18344609534c364e599dd8563225e77a334 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Mon, 24 Aug 2026 18:14:27 +0530 Subject: [PATCH 5/5] Decouple event listening logic and add placeholder --- .../selenium-webdriver/bidi/domain.d.ts | 13 +- javascript/selenium-webdriver/bidi/domain.js | 80 ++------- .../test/bidi/domain_test.js | 158 ++---------------- 3 files changed, 28 insertions(+), 223 deletions(-) diff --git a/javascript/selenium-webdriver/bidi/domain.d.ts b/javascript/selenium-webdriver/bidi/domain.d.ts index e24f501c0dd30..812c760fc644d 100644 --- a/javascript/selenium-webdriver/bidi/domain.d.ts +++ b/javascript/selenium-webdriver/bidi/domain.d.ts @@ -39,17 +39,14 @@ export declare class Domain { protected send(method: string, params: Record): Promise /** - * Subscribes `handler` to a BiDi event — asks the remote end to start - * sending it (only if nothing else on this connection already has), then - * attaches `handler` as a local listener for it. Remote subscription is - * ref-counted against the connection's own listener count, so it's shared - * correctly across every Domain instance on that connection, not just this one. + * Subscribes `handler` to a BiDi event — asks the remote end to start sending it, + * then attaches `handler` as a local listener for it. Placeholder for now: always + * subscribes/unsubscribes remotely on every call, with no ref-counting across + * callers — connection-wide listener bookkeeping is follow-up work. * @param {EventDescriptor} descriptor An event descriptor from event(). * @param {function(T): void} handler Invoked with the event's parsed params each time it fires. * @returns {Promise<{unsubscribe: function(): Promise}>} A handle for this - * subscription; call `unsubscribe()` to stop receiving the event (and, if it - * was the last listener for it on this connection, to tell the remote end to - * stop sending it). + * subscription; call `unsubscribe()` to stop receiving the event. */ addCallback( descriptor: EventDescriptor, diff --git a/javascript/selenium-webdriver/bidi/domain.js b/javascript/selenium-webdriver/bidi/domain.js index ef5113ac5e06e..561b08627336a 100644 --- a/javascript/selenium-webdriver/bidi/domain.js +++ b/javascript/selenium-webdriver/bidi/domain.js @@ -42,39 +42,6 @@ function event(method, type) { return { method, type } } -// Serializes subscribe()/unsubscribe() transitions per (connection, method), so -// concurrent addCallback()/unsubscribe() calls touching the same event — from any -// Domain instance sharing that connection, not just one — can't interleave into an -// inconsistent remote subscription state (e.g. a listener left attached locally -// after a same-method unsubscribe-in-flight elsewhere wins the race and tells the -// browser to stop sending it). Keyed by the connection object itself via a WeakMap, -// not any one Domain instance — module-level, not Domain state, so Domain itself -// stays a plain, stateless wrapper around `send()` plus this queuing. The per-method -// entry is pruned once idle (see below), so a long-lived connection subscribing to -// many distinct methods over its lifetime doesn't grow this map without bound. -const subscriptionQueues = new WeakMap() - -function queueSubscriptionChange(bidi, method, change) { - let methods = subscriptionQueues.get(bidi) - if (methods === undefined) { - methods = new Map() - subscriptionQueues.set(bidi, methods) - } - const previous = methods.get(method) ?? Promise.resolve() - const next = previous.then(change, change) // run `change` next regardless of a prior failure - const queued = next.catch(() => {}) // don't let that failure jam the queue for later callers - methods.set(method, queued) - queued.finally(() => { - // Only this entry's own settlement prunes it, and only if nothing newer was - // enqueued for `method` in the meantime — otherwise this would delete a later - // caller's still-pending entry out from under it. - if (methods.get(method) === queued) { - methods.delete(method) - } - }) - return next -} - /** Shared base for every generated BiDi domain class. See domain.d.ts for the typed surface. */ class Domain { #bidi @@ -99,22 +66,13 @@ class Domain { } /** - * Subscribes `handler` to a BiDi event. Two distinct things happen: remote - * subscription (telling the browser to start sending this event at all, - * via the underlying connection's `subscribe()`) and local listening - * (attaching `handler` to the connection so it runs when the event - * arrives) — a descriptor's event only ever reaches `handler` once both - * are in place. Remote subscription/unsubscription is ref-counted against - * the connection's own listener count — shared truth across every Domain - * instance on the same connection — so a second caller subscribing to the - * same event doesn't re-subscribe remotely, and unsubscribing doesn't cut - * off another caller still listening for it. The listener is attached - * before `subscribe()` is awaited (not after), so an event the browser - * starts sending as soon as it processes the subscription can't arrive in - * a gap where nothing is listening yet; and every subscribe/unsubscribe - * transition for this event, from any caller, is serialized (see - * queueSubscriptionChange()) so concurrent callers can't interleave into - * an inconsistent remote state. + * Subscribes `handler` to a BiDi event: tells the remote end to start sending it + * (the underlying connection's `subscribe()`), then attaches `handler` as a local + * listener for it. Placeholder for now — always subscribes/unsubscribes remotely on + * every call, with no ref-counting across callers or connection-wide listener + * bookkeeping (that belongs at the connection level, shared across every Domain + * instance, mirroring Ruby's WebSocketConnection#add_callback/remove_callback or + * Java's equivalent — tracked as follow-up work, not folded into this PR). * @param {{method: string, type: ({fromWire(payload: unknown): unknown}|undefined)}} descriptor * An event descriptor from event(). * @param {function(unknown): void} handler Invoked with the event's params @@ -124,25 +82,13 @@ class Domain { */ async addCallback(descriptor, handler) { const dispatch = descriptor.type === undefined ? handler : (params) => handler(descriptor.type.fromWire(params)) - await queueSubscriptionChange(this.#bidi, descriptor.method, async () => { - this.#bidi.on(descriptor.method, dispatch) - if (this.#bidi.listenerCount(descriptor.method) === 1) { - try { - await this.#bidi.subscribe(descriptor.method) - } catch (err) { - this.#bidi.off(descriptor.method, dispatch) // don't leak a listener for a subscription that never took - throw err - } - } - }) + await this.#bidi.subscribe(descriptor.method) + this.#bidi.on(descriptor.method, dispatch) return { - unsubscribe: () => - queueSubscriptionChange(this.#bidi, descriptor.method, async () => { - this.#bidi.off(descriptor.method, dispatch) - if (this.#bidi.listenerCount(descriptor.method) === 0) { - await this.#bidi.unsubscribe(descriptor.method) - } - }), + unsubscribe: async () => { + this.#bidi.off(descriptor.method, dispatch) + await this.#bidi.unsubscribe(descriptor.method) + }, } } } diff --git a/javascript/selenium-webdriver/test/bidi/domain_test.js b/javascript/selenium-webdriver/test/bidi/domain_test.js index 745e020bcdfd2..c9656705348b6 100644 --- a/javascript/selenium-webdriver/test/bidi/domain_test.js +++ b/javascript/selenium-webdriver/test/bidi/domain_test.js @@ -29,7 +29,7 @@ const EntryAdded = defineRecord('test.domain.EntryAdded', [ // Fake replacing the real BiDi transport (bidi/index.js's Index). Mirrors its // actual shape — an EventEmitter with subscribe()/unsubscribe() added — rather // than inventing its own addCallback/removeCallback surface, since Domain talks -// to the connection's real on/off/listenerCount/subscribe/unsubscribe directly. +// to the connection's real on/off/subscribe/unsubscribe directly. function fakeBidi() { const bidi = new EventEmitter() bidi.subscribeCalls = [] @@ -43,33 +43,6 @@ function fakeBidi() { return bidi } -// Same shape as fakeBidi(), but subscribe()/unsubscribe() stay pending until the -// test explicitly resolves them — widens the race window addCallback()/unsubscribe() -// must handle correctly (an event arriving mid-subscribe, two callers racing to -// subscribe/unsubscribe the same method) instead of hoping a real await happens to -// interleave the wrong way. -function controllableFakeBidi() { - const bidi = new EventEmitter() - bidi.subscribeCalls = [] - bidi.unsubscribeCalls = [] - const pendingSubscribes = [] - const pendingUnsubscribes = [] - bidi.subscribe = (method) => { - bidi.subscribeCalls.push(method) - return new Promise((resolve, reject) => pendingSubscribes.push({ resolve, reject })) - } - bidi.unsubscribe = (method) => { - bidi.unsubscribeCalls.push(method) - return new Promise((resolve, reject) => pendingUnsubscribes.push({ resolve, reject })) - } - bidi.resolveNextSubscribe = () => pendingSubscribes.shift().resolve() - bidi.rejectNextSubscribe = (err) => pendingSubscribes.shift().reject(err) - bidi.resolveNextUnsubscribe = () => pendingUnsubscribes.shift().resolve() - return bidi -} - -const tick = () => new Promise((resolve) => setTimeout(resolve, 0)) - describe('Domain addCallback', function () { it('parses delivered payloads through the descriptor type before the handler runs', async function () { const bidi = fakeBidi() @@ -101,7 +74,10 @@ describe('Domain addCallback', function () { assert.ok(!(received[0] instanceof EntryAdded)) }) - it('subscribes remotely on the first listener, and not again for a second one', async function () { + // Placeholder behavior: every addCallback() call subscribes remotely, and every + // unsubscribe() call unsubscribes remotely — no ref-counting across callers yet. + // See the comment on Domain#addCallback() in domain.js. + it('calls bidi.subscribe() on every addCallback call', async function () { const bidi = fakeBidi() const domain = new Domain(bidi, DOMAIN_TOKEN) const descriptor = event('test.entryAdded', EntryAdded) @@ -109,10 +85,10 @@ describe('Domain addCallback', function () { await domain.addCallback(descriptor, () => {}) await domain.addCallback(descriptor, () => {}) - assert.deepStrictEqual(bidi.subscribeCalls, ['test.entryAdded']) + assert.deepStrictEqual(bidi.subscribeCalls, ['test.entryAdded', 'test.entryAdded']) }) - it('unsubscribes remotely only once the last local listener is gone', async function () { + it('calls bidi.unsubscribe() on every unsubscribe() call', async function () { const bidi = fakeBidi() const domain = new Domain(bidi, DOMAIN_TOKEN) const descriptor = event('test.entryAdded', EntryAdded) @@ -121,10 +97,9 @@ describe('Domain addCallback', function () { const second = await domain.addCallback(descriptor, () => {}) await first.unsubscribe() - assert.deepStrictEqual(bidi.unsubscribeCalls, []) - await second.unsubscribe() - assert.deepStrictEqual(bidi.unsubscribeCalls, ['test.entryAdded']) + + assert.deepStrictEqual(bidi.unsubscribeCalls, ['test.entryAdded', 'test.entryAdded']) }) it('stops delivering to a handler once its own subscription is unsubscribed', async function () { @@ -141,7 +116,7 @@ describe('Domain addCallback', function () { assert.strictEqual(received.length, 0) }) - it('does not disturb another still-active listener on the same event', async function () { + it('does not remove another still-active local listener on the same event', async function () { const bidi = fakeBidi() const domain = new Domain(bidi, DOMAIN_TOKEN) const descriptor = event('test.untyped') @@ -157,119 +132,6 @@ describe('Domain addCallback', function () { assert.strictEqual(receivedByFirst.length, 0) assert.strictEqual(receivedBySecond.length, 1) }) - - it('prunes its internal per-method subscription queue entry once idle', async function () { - // queueSubscriptionChange()'s per-method Map has no other seam to observe from - // outside the module, so temporarily watch Map.prototype.delete rather than - // exporting a test-only hook into the shipped module. - const bidi = fakeBidi() - const domain = new Domain(bidi, DOMAIN_TOKEN) - const descriptor = event('test.untyped') - - const originalDelete = Map.prototype.delete - const deletedKeys = [] - Map.prototype.delete = function (key) { - deletedKeys.push(key) - return originalDelete.call(this, key) - } - try { - const subscription = await domain.addCallback(descriptor, () => {}) - await subscription.unsubscribe() - await tick() // let the queued promise's own .finally() pruning run - } finally { - Map.prototype.delete = originalDelete - } - - assert.ok(deletedKeys.includes('test.untyped')) - }) - - describe('concurrency', function () { - it('does not miss an event that arrives while subscribe() is still pending', async function () { - const bidi = controllableFakeBidi() - const domain = new Domain(bidi, DOMAIN_TOKEN) - const descriptor = event('test.untyped') - const received = [] - - const addP = domain.addCallback(descriptor, (params) => received.push(params)) - await tick() // let addCallback reach its (still-pending) subscribe() call - assert.strictEqual(bidi.subscribeCalls.length, 1) - - bidi.emit('test.untyped', { anything: 'goes' }) // must already be listening, not just subscribing - bidi.resolveNextSubscribe() - await addP - - assert.strictEqual(received.length, 1) - }) - - it('removes the listener if subscribe() rejects, instead of leaking it', async function () { - const bidi = controllableFakeBidi() - const domain = new Domain(bidi, DOMAIN_TOKEN) - const descriptor = event('test.untyped') - - const addP = domain.addCallback(descriptor, () => {}) - await tick() - bidi.rejectNextSubscribe(new Error('boom')) - - await assert.rejects(addP, /boom/) - assert.strictEqual(bidi.listenerCount('test.untyped'), 0) - }) - - it('serializes concurrent addCallback calls for the same event so only one subscribe is sent', async function () { - const bidi = controllableFakeBidi() - const domain = new Domain(bidi, DOMAIN_TOKEN) - const descriptor = event('test.untyped') - - const first = domain.addCallback(descriptor, () => {}) - const second = domain.addCallback(descriptor, () => {}) - await tick() - - // The second call must not have started (and raced its own listenerCount - // check) before the first's subscribe() — still pending — resolves. - assert.strictEqual(bidi.subscribeCalls.length, 1) - assert.strictEqual(bidi.listenerCount('test.untyped'), 1) - - bidi.resolveNextSubscribe() - await first - await second - - assert.strictEqual(bidi.subscribeCalls.length, 1) - assert.strictEqual(bidi.listenerCount('test.untyped'), 2) - }) - - it('serializes an unsubscribe against a concurrent addCallback for the same event', async function () { - const bidi = controllableFakeBidi() - const domain = new Domain(bidi, DOMAIN_TOKEN) - const descriptor = event('test.untyped') - - const first = domain.addCallback(descriptor, () => {}) - await tick() - bidi.resolveNextSubscribe() - const subscription = await first - bidi.subscribeCalls.length = 0 // only the part under test matters from here - - // Start unsubscribing the only listener, then — before that finishes — start - // a second addCallback for the same event. Unserialized, the second caller - // could attach and see itself as remotely subscribed while the in-flight - // unsubscribe() is still telling the browser to stop sending the event, - // leaving it un-subscribed remotely despite a live local listener. - const unsubP = subscription.unsubscribe() - const secondP = domain.addCallback(descriptor, () => {}) - await tick() - - assert.strictEqual(bidi.unsubscribeCalls.length, 1) // unsubscribe() is in flight... - assert.strictEqual(bidi.subscribeCalls.length, 0) // ...and the second caller hasn't jumped ahead of it - - bidi.resolveNextUnsubscribe() - await unsubP - await tick() - bidi.resolveNextSubscribe() - await secondP - - assert.strictEqual(bidi.unsubscribeCalls.length, 1) - assert.strictEqual(bidi.subscribeCalls.length, 1) // re-subscribed only after unsubscribe() had finished - assert.strictEqual(bidi.listenerCount('test.untyped'), 1) - }) - }) }) describe('Domain construction guard', function () {