Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul the section on processing capabilities. #327

Closed
wants to merge 11 commits into from
Closed

Overhaul the section on processing capabilities. #327

wants to merge 11 commits into from

Conversation

jleyba
Copy link
Contributor

@jleyba jleyba commented Sep 6, 2016

There are several goals with these changes:

  1. The endpoint node should either be able to satisfy all capabilities
    requested by the local end, or fail to create a session.
  2. The local end should be able to submit multiple sets of acceptable
    capabilities with a clearly defined order of preference. This eliminates
    the need for extra, potentially expensive, new session requests when one
    set of capabilities cannot be satisfied.
  3. Permit the definition and processing of custom capabilities, either by
    the endpoint node or an intermediate node.
  4. Preserve wire compatibility with existing implementations (i.e. the changes
    do not depend on the "requiredCapabilities" and "desiredCapabilities"
    parameters).

When the local end truly has no preference for the features supported by a
new session, it can simply request an empty set of capabilities.

POST /session
{"capabilities": {}}

The local end requires a specific browser (Firefox), but has no preference
for the exact value:

POST /session
{"capabilities": {"alwaysMatch":{"browserName":"firefox"}}}

The local end requires Firefox on Linux, or Chrome on Windows: the order
specified dictates in which order the endpoint should try to satisfy the
request:

POST /session
{"capabilities": {
  "firstMatch": [
    {"browserName": "firefox", "platformName": "linux"},
    {"browserName": "chrome", "platformName": "windows"}
  ]}}

Capabilities that are always required may be specified as "alwaysMatch". These
are merged with each "firstMatch" entry, eliminating the need to specify
expensive capabilities (like an encoded profile) multiple times:

POST /session
{"capabilities": {
   "alwaysMatch": {
     "browserName": "firefox",
     "profile": "base64-encoded-blob"
   },
   "firstMatch": [
     {"platformName": "linux"},
     {"platformName": "windows"}
   ]}}

Version strings are complicated. The exact matching algorithms are explicitly
left for each vendor, however, implementors are encouraged to support basic
operators to define a range of versions. That is, to request a version of
Firefox older than 48, the local end would specify:

POST /session
{"capabilities": {
   "alwaysMatch": {
     "browserName": "firefox",
     "browserVersion": "< 48"
   }}}

If an intermediate node requires additional information (i.e. auth credentials),
it MAY define custom capabilities, but it SHOULD require this information be
passed as a top level parameter:

POST /session
{"username": "test-user",
 "password": "abc123",
 "capabilities": {}}

Capabilities may never be ignored. If the endpoint does not recognize a key, it
must fail to match on those capabilities:

POST /session
{"capabilities": {
   "alwaysMatch": {
     "browserName": "firefox",
     "unrecognized-capability": 1234
   }}}

Given that unrecognized capabilities will cause new session requests to fail,
the spec requires that intermediate nodes that use capabilities for routing
remove those capabilities before forwarding the request to the selected remote
end.


This change is Reviewable

There are several goals with these changes:

1.  The endpoint node should either be able to satisfy all capabilities
    requested by the local end, or fail to create a session.

2.  The local end should be able to submit multiple sets of acceptable
    capabilities with a clearly defined order of preference. This eliminates
    the need for extra, potentially expensive, new session requests when one
    set of capabilities cannot be satisfied.

3.  Permit the definition and processing of custom capabilities, either by
    the endpoint node or an intermediate node.

4.  Preserve wire compatibility with existing implementations (i.e. the changes
    do not depend on the "requiredCapabilities" and "desiredCapabilities"
    parameters).

----

When the local end truly has no preference for the features supported by a
new session, it can simply request an empty set of capabilities.

    POST /session
    {"capabilities": {}}

The local end requires a specific browser (Firefox), but has no preference
for the exact value:

    POST /session
    {"capabilities": {"alwaysMatch":{"browserName":"firefox"}}}

The local end requires Firefox on Linux, or Chrome on Windows: the order
specified dictates in which order the endpoint should try to satisfy the
request:

    POST /session
    {"capabilities": {
      "firstMatch": [
        {"browserName": "firefox", "platformName": "linux"},
        {"browserName": "chrome", "platformName": "windows"}
      ]}}

Capabilities that are always required may be specified as "alwaysMatch". These
are merged with each "firstMatch" entry, eliminating the need to specify
expensive capabilities (like an encoded profile) multiple times:

    POST /session
    {"capabilities": {
       "alwaysMatch": {
         "browserName": "firefox",
         "profile": "base64-encoded-blob"
       },
       "firstMatch": [
         {"platformName": "linux"},
         {"platformName": "windows"}
       ]}}

Version strings are complicated. The exact matching algorithms are explicitly
left for each vendor, however, implementors are encouraged to support basic
operators to define a range of versions. That is, to request a version of
Firefox older than 48, the local end would specify:

    POST /session
    {"capabilities": {
       "alwaysMatch": {
         "browserName": "firefox",
         "browserVersion": "< 48"
       }}}

If an intermediate node requires additional information (i.e. auth credentials),
it MAY define custom capabilities, but it SHOULD require this information be
passed as a top level parameter:

    POST /session
    {"username": "test-user",
     "password": "abc123",
     "capabilities": {}}

Capabilities may never be ignored. If the endpoint does not recognize a key, it
must fail to match on those capabilities:

    POST /session
    {"capabilities": {
       "alwaysMatch": {
         "browserName": "firefox",
         "unrecognized-capability": 1234
       }}}

Given that unrecognized capabilities will cause new session requests to fail,
the spec requires that intermediate nodes that use capabilities for routing
remove those capabilities before forwarding the request to the selected remote
end.
to describe the full feature set for a session.

<p>The following table provides an overview of the standard capabilities that
each implementation <em class="rfc2119" title="MUST">MUST</em> support.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the class or title.

@shs96c
Copy link
Contributor

shs96c commented Sep 7, 2016

I need to finish the review, but broadly in agreement with this change. I think it addresses concerns raised on IRC within the #selenium project, at face to face meetings, and in personal discussions.

<li><p>Let <var>socks proxy port</var> be the result of <a>getting a property</a>
named <code>socksProxyPort</code> from <var>proxy</var>.
<dt>"<code>acceptSslCerts</code>"
<dd>False, indicating the session <em>will not</em> implicitly trust untrusted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Boolean false" to differentiate from the string "false"

Implementation defined capabilities are only selected during the initial
processing stage. These capabilities must actually be applied when
initializing the new session.
<td>"<code>httpProxy</code>"
<td>string
<td>Defines the proxy <em>hostname</em> for HTTP traffic. Alternatively, an
implementation <em>MAY</em> accept a hostname and port through this property.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice for backwards compatibility. If a hostname:port pair is given here, AND httpProxyPort is set, which takes precedence?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest raising an error, explaining the clash. Let the user make up their mind rather than guessing.

Required capabilities always take precedence, and it is an error to
specify a capability as both a required capability and a desired
capability.
The null, undefined, and "*" values are all wildcard matchers for
browserName and match any name.
…pport

wildcards. A wildcard value is any of null, undefined, or "*"
If the processing capabilities step fails, the new session command
should return a "session not created" error that wraps the details
returned from the processing capabilities.
- Default "firstMatch" parameters to [{}], not [], ensuring there is
always one set of capabilities to match on.

- Clarify that intermediary nodes may forward custom parameters to the
remote end.
each implementation <em>MUST</em> support. An implementation <em>MAY</em> define
additional capabilities, although it is <em>RECOMMENDED</em> that custom
capabilities prepend the capability key with a vendor prefix, as outlined in
[[CSS21]].
Copy link
Contributor

@shs96c shs96c Sep 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a section on protocol extension in the spec. Refer to the protocol extension instead? With slashes replaced by "-"? The nice feature this offers is that it suggests that each vendor's product is also included in the key, which reduces the chance of a collision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section on protocol extension talks about vendor prefixs used in the URI path. There's no mention of extending parameters. Should that be added?

<p>The <a>Session Timeouts Configuration</a> capability is a JSON Object nested
within the primary capabilities. It consists of the properties listed in the
table below. These values may be changed after creating a <a>new session</a>
using the <a>Set Timeout</a> command.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These timeouts are all in the Set Timeout section. Refer to these rather than duplicate the information?

@shs96c shs96c self-assigned this Sep 14, 2016
@andreastt
Copy link
Member

@jleyba At the F2F we decided to adopt this proposal, congratulations!

There were noted several technical problems with the algorithm in the patch, but it was suggested that we would go ahead and merge it and fix these later. There is general agreement on the approach.

@shs96c
Copy link
Contributor

shs96c commented Oct 6, 2016

Adding @AutomatedTester as an assignee, since the action item from the Lisbon F2F was for him to make some edits and then land this patch.

@AutomatedTester
Copy link
Contributor

landed, with some updates (and I expect there to be a lot more) in d0ef240

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants