SeleniumHQ / SeleniumHQ/selenium

[🐛 Bug]: DefaultSlotMatcher.extensionCapabilitiesMatch() only checks capabilities the stereotype declares, so a request differentiated solely by an identity extension capability can match an unrelated node

Open
#17,845 11 comments 0 reactions 0 assignees View on GitHub
A-needs-triaging I-defect
Dominant language
Java
Stars
34.5k
Forks
8.7k
Avg merge
2d 1h
Merged PRs (30d)
92

Description

### Description

## Summary

`DefaultSlotMatcher.extensionCapabilitiesMatch()` iterates `stereotype.getCapabilityNames()` — the node's declared capabilities — rather than the requested capabilities. As a result, if a session request specifies an extension capability the target stereotype simply doesn't declare at all, that capability is never inspected, and the request can match a node that has nothing to do with what was asked for.

This is a real production issue, not a theoretical edge case: on any grid mixing a native-automation node type (Appium, differentiated by `appium:automationName`) with an ordinary browser node that happens to share a platform, requests can silently misroute to the wrong node type.

## Reproduction

Stereotype for a Windows/Edge browser node:
```json
{"browserName": "MicrosoftEdge", "platformName": "Windows 10"}
```

Requested capabilities for a native Windows-app automation session:
```json
{"appium:automationName": "Windows", "platformName": "windows"}
```

`DefaultSlotMatcher.matches(stereotype, capabilities)` returns `true` for this pair on current `trunk`.

### Why

- `initialMatch()` — no non-extension capability names to check (the stereotype declares only `browserName`/`platformName`, both handled elsewhere).
- `extensionCapabilitiesMatch()` — the stereotype declares **zero** `:`-namespaced capabilities, so the stream (`stereotype.getCapabilityNames().stream().filter(name -> name.contains(":"))...`) is empty and short-circuits to `true` via `.orElse(true)`. `appium:automationName` is never inspected, on either side.
- `browserNameMatch` — the request doesn't specify `browserName`, so this is vacuously `true`.
- `platformNameMatch` — `"windows"` and `"Windows 10"` resolve to the same `Platform` family, so this passes too.

Every check passes despite the request asking for a native Windows-automation session and the stereotype describing a browser-only Edge node.

## Root cause

Current `extensionCapabilitiesMatch()` (trunk, `java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java`):

```java
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
return stereotype.getCapabilityNames().stream()
.filter(name -> name.contains(":"))
.filter(name -> !name.toLowerCase().contains("options"))
.filter(name -> capabilities.asMap().containsKey(name))
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
.map(name -> { /* ... equality check ... */ })
.reduce(Boolean::logicalAnd).orElse(true);
}
```

The iteration source is `stereotype.getCapabilityNames()`. Every filter after that only narrows *which of the stereotype's own declared names* get checked — none of them widen the check to cover a name the request specifies but the stereotype omits entirely. That asymmetry is the bug: a capability absent from the stereotype behaves identically to a capability present and matching, when it should behave like a mismatch whenever the request actually specifies a value for it.

This is worth distinguishing from #15481 / PR #15574, which addressed a different problem — *which* extension capabilities get excluded from consideration (the vendor-prefix list, the `options`-suffix exclusion). That work changed the filtering, not the direction of iteration. The directional issue described here predates that fix and is still present after it.

## Proposed fix

Don't rewrite the whole method — the vendor-prefix/options exclusion logic is orthogonal to this bug and shouldn't be touched by this fix. The minimal correct change is to check identity-relevant capabilities bidirectionally, driven by the **request's** capability names rather than the stereotype's, for the specific capabilities that actually function as node-identity signals (`automationName` being the primary one in practice):

```java
private Boolean automationNameMatch(Capabilities stereotype, Capabilities capabilities) {
return capabilities.getCapabilityNames().stream()
.filter(name -> name.contains("automationName"))
.map(name -> Objects.equals(stereotype.getCapability(name), capabilities.getCapability(name)))
.reduce(Boolean::logicalAnd).orElse(true);
}
```

Called alongside the existing `platformVersionMatch()` (which is already request-driven and doesn't have this bug), before `extensionCapabilitiesMatch()`, in `matches()`.

This is deliberately narrow rather than making *every* extension capability bidirectional: a blanket bidirectional rule over all extension capabilities breaks real Appium sessions, since a typical request carries several `appium:`-namespaced capabilities (`app`, `udid`, `noReset`, etc.) that are legitimate per-session parameters with no stereotype equivalent — requiring the stereotype to match those too would reject sessions that have nothing wrong with them. Scoping the bidirectional check to the specific capability that actually signals node identity avoids that regression while still closing the hole demonstrated above.

## Environment

- Reproduced by tracing current `trunk` source directly (commit referenced in this write-up: `b160e140e160ee4a8e3bd99283e294fc0e5933f3`); not yet confirmed against a specific tagged release, but the relevant method is unchanged from the version merged in #15574.

### Reproducible Code

```shell
package org.openqa.selenium.grid.data;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.openqa.selenium.MutableCapabilities;

/**
* Demonstrates that {@link DefaultSlotMatcher} matches a request against a stereotype that
* shares no relationship to it, because {@code extensionCapabilitiesMatch()} only inspects
* extension capability names the STEREOTYPE declares — never ones only the request specifies.
*


* Intended to be dropped into {@code java/test/org/openqa/selenium/grid/data/} in a Selenium
* checkout and run via the project's own test tooling (e.g. {@code bazel test}) — not verified
* against a live build in this environment; no network access to Maven Central to pull the real
* jars here, only to GitHub for reading source.
*/
class DefaultSlotMatcherDirectionalBugTest {

@Test
void requestDifferentiatedOnlyByUndeclaredExtensionCapabilityShouldNotMatch() {
DefaultSlotMatcher matcher = new DefaultSlotMatcher();

// Edge browser node — declares no extension (":") capabilities at all
MutableCapabilities edgeStereotype = new MutableCapabilities();
edgeStereotype.setCapability("browserName", "MicrosoftEdge");
edgeStereotype.setCapability("platformName", "Windows 10");

// Native Windows-automation session request, differentiated only by automationName
MutableCapabilities windowsRequest = new MutableCapabilities();
windowsRequest.setCapability("appium:automationName", "Windows");
windowsRequest.setCapability("platformName", "windows");

// This currently returns true on trunk — it should return false, since the stereotype
// has no relationship to the requested automationName at all.
assertThat(matcher.matches(edgeStereotype, windowsRequest)).isFalse();
}
}
```

Contributor guide

Open the contributing guide

Research direction

Start in java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java and trace matches(), platformVersionMatch(), and extensionCapabilitiesMatch(). Add the supplied regression test under java/test/org/openqa/selenium/grid/data/ and run it with the project’s test tooling, such as bazel test. Done means a request differentiated by an undeclared automationName does not match the unrelated stereotype while existing extension-capability behavior remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.