SeleniumHQ / SeleniumHQ/selenium
Latent NPE risk at ~25 call sites of Json.toType() (surfaced by nullability annotation in #17802)
- Dominant language
- Java
- Stars
- 34.5k
- Forks
- 8.7k
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 92
Description
While auditing usages of `Json.toType(...)` / `JsonTypeCoercer.coerce(...)` for #17802 (which marks these methods `@Nullable`, since they can legitimately return `null` for a top-level JSON `null`), I found ~25 production call sites that dereference or pass along the result without a null guard. These are **pre-existing** behaviors, not regressions from #17802 — the nullability annotation just makes them visible to static analysis (e.g. IntelliJ's null-checker). None have a known repro; they'd only trigger on malformed/unexpected input (a literal `null` body, a corrupted stored value, etc.).
Grouped by area:
**Grid node / capability parsing** — feed into `NodeOptions.enhanceStereotype`, which dereferences immediately with no guard:
- `NodeOptions.java:358,472`, `RelayOptions.java:168`, `DockerOptions.java:173`, `KubernetesOptions.java:150`
**Wire/session-decode paths** — no null guard, no catch for the null case:
- `ProtocolHandshake.java:97`, `RemoteScript.java:95`, `W3CHttpResponseCodec.java:106,160`, `GraphqlHandler.java:140`, `CustomLocatorHandler.java:111`, `NewNodeSession.java:48`, `RemoteNode.java:186`, `AddNode.java:57`, `LocalNode.java:1011,1122,1160` + `OneShotNode.java:360` (shared code), `RedisBackedNewSessionQueue.java:441,527`
**Lower severity** (already inside broad catches elsewhere in the file, or build-time tooling):
- `FirefoxProfile.java:88`, `Preferences.java:101`, `InspectContainer.java:67`, `ListImages.java:77`, `CdpEndpointFinder.java:75`, `CdpEventTypes.java:96`, `SeleniumManager.java:167`, `CdpClientGenerator.java:108`, `OneShotNode.java:149`
**Structural** — `Event.java:59`'s `getData(Type)` re-exports the nullability gap: its own return type isn't annotated `@Nullable`, silently passing the gap to every caller.
Suggest tackling these in small, targeted follow-up PRs per area (grid node config parsing vs. wire-protocol decode vs. the `Event` API), each with a regression test for the null-input case.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
Research direction
Start by reading Json.toType(...) and JsonTypeCoercer.coerce(...), then choose one area such as grid node configuration or wire/session decoding rather than attempting all listed call sites. Inspect the named Java files and trace how null results are consumed. Done means each targeted call site has an intentional null-handling outcome and a regression test covers null input.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100