Skip to content

[rb] reject an inbound BiDi scalar outside its union's declared arms - #17947

Merged
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:rb-bidi-inbound-scalar-arms
Aug 25, 2026
Merged

[rb] reject an inbound BiDi scalar outside its union's declared arms#17947
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:rb-bidi-inbound-scalar-arms

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

Update to support the latest commit from ADR #17786 (66111c8)

💥 What does this PR do?

Reject an inbound bare scalar that is not one of its union's declared literals (e.g. anything other than "viewport"/"pointer" for input.Origin)

🔧 Implementation Notes

  • This is the inbound mirror of the existing outbound scalar_arm? check, and matches how an unknown object variant is already rejected.
  • No nil guard on @scalar_values: the generator raises at generation time if a non-object_only union lacks them, and verify-bidi-generated enforces that in CI.
  • input.Origin is the only union with scalar arms today and it never arrives inbound, so no traffic changes in practice; webdriverbidi-net has the same posture via its closed Origin type.

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code (Fable 5)
    • What was generated: the fix, tests, and this description
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Bug fix (backwards compatible)

@selenium-ci selenium-ci added C-rb Ruby Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Aug 25, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Reject undeclared inbound BiDi union scalars

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Reject inbound scalar values absent from a BiDi union’s declared literal arms.
• Preserve declared scalar arms and existing object-only union validation.
• Cover accepted and rejected input.Origin scalar deserialization.
Diagram

graph TD
  Payload["Inbound payload"] --> Kind{"Hash payload?"}
  Kind -->|Yes| Variant["Variant dispatch"] --> Typed["Typed BiDi value"]
  Kind -->|No| ObjectOnly{"Object only?"}
  ObjectOnly -->|Yes| Error["Serialization error"]
  ObjectOnly -->|No| Declared{"Declared scalar?"}
  Declared -->|Yes| Scalar["Scalar value"]
  Declared -->|No| Error
Loading
High-Level Assessment

The centralized inbound check is the best approach because it mirrors outbound validation through the existing scalar_arm? predicate and applies consistently to every generated union. Per-union validation would duplicate schema rules and risk inconsistent behavior.

Files changed (2) +18 / -5

Bug fix (1) +10 / -4
union.rbValidate inbound scalars against declared union arms +10/-4

Validate inbound scalars against declared union arms

• Non-object payloads now pass through only when they match a union's declared scalar literals. Object-only unions retain their existing error, while undeclared scalars raise a schema-specific 'SerializationError'.

rb/lib/selenium/webdriver/bidi/serialization/union.rb

Tests (1) +8 / -1
serialization_spec.rbTest declared and undeclared inbound union scalars +8/-1

Test declared and undeclared inbound union scalars

• Clarifies that declared 'input.Origin' scalars remain accepted and adds coverage proving an unknown scalar is rejected with a serialization error.

rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb

@qodo-code-review

qodo-code-review Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit de4cf0c 🧠 Deep

Results up to commit 762cd1e ⚖️ Balanced


No changes from previous review

Grey Divider

Qodo Logo

@titusfortner
titusfortner force-pushed the rb-bidi-inbound-scalar-arms branch from 762cd1e to de4cf0c Compare August 25, 2026 18:10
@titusfortner
titusfortner merged commit 7bfaedb into SeleniumHQ:trunk Aug 25, 2026
5 of 7 checks passed
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit de4cf0c

This was referenced Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants