Add protocol schema validation for WebDriver BiDi
Categories
(Remote Protocol :: WebDriver BiDi, task, P3)
Tracking
(Not tracked)
People
(Reporter: ato, Unassigned)
References
Details
(Whiteboard: [webdriver:backlog])
We want to add (WebDriver BiDi-ish/CDP) protocol schema validation
to the remote agent for a number of different reasons:
-
It is currently possible to call intended-to-be-private methods
on both parent- and child domains because we never verify that an
incoming metod is a valid member of the protocol. -
We are being intentionally lax with type checks for the JSON
request body, which is good because it allows us to write freer JS,
but not so good because specific types could trigger behaviour that
was not intended due to type coercions in JS. -
We also don’t verify that the output of responses and events are
correct, which is a source for implementation bugs. -
The forthcoming bi-di proposal in WebDriver will insist on strict
input and output checks, and protocol schema validation will help
achieve conformance quicker.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
When I did the original prototype for the remote agent I started
work on a protocol schema validator written as JS module. This was
similar in nature to the one that existed in Juggler:
https://github.com/puppeteer/juggler/blob/master/testing/juggler/protocol/Protocol.js
We later decided to scrap this approach because we had more important
things to take care of. There was also a sense at the time that it
would be good if we could reuse the serde traits offered by
https://github.com/devtools-html/rust-cdp.
With the forthcoming proposal for bi-di extension to WebDriver that
is going to be CDP-like, the requirements we have for schema
validation has sightly changed: we likely need to support a subset
of CDP, but also the new bi-di protocol as it is forthcoming. I
don’t know if this means we’ll reuse rust-cdp
for one part and
invent a new crate with serde traits for the latter, or if we will
just put everything into one single entity. Time will show.
A proposal was made that we should do the schema validation using
serde in Rust. The work I’m doing in bug 1543115 will unlock some
integration across XPIDL between Rust and JS. The advantages of
this implementation strategy over a JS-based validator are that
-
serde is quite good and we have prior experience how to use
it; -
and if we move to a Rust-based HTTPD in the future we will
inevitably need serde traits.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 2•2 years ago
|
||
I don't think that we will do this anymore for CDP so lets move this bug to WebDriver BiDi.
On the WebDriver BiDi repository we use the cddl
rust crate to validate the generated CDDL definitions for the local client and the remote end. The files are attached as artifacts for each GitHub actions job.
With the new 0.9.0 release of cddl
the parser and validator got heavily improved and is mostly perfect now. The only issue that I've seen is with optional fields which are based on a choice. I filed https://github.com/anweiss/cddl/issues/139 to get this fixed.
Nevertheless it would be good to see this Rust crate integrated into our code base and to have it running eg. in an experimental state like pref'ed off by default.
Comment 3•2 years ago
|
||
It would be good to also check the output in debug mode maybe to ensure that the schema make sense. I'll work on this whenever I have some time.
Updated•2 years ago
|
Description
•