Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions java/src/org/openqa/selenium/chromium/ChromiumDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.function.Supplier;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.openqa.selenium.BuildInfo;
import org.openqa.selenium.Capabilities;
Expand Down Expand Up @@ -277,6 +278,7 @@ public void setFileDetector(FileDetector detector) {
}

@Override
@NullMarked
public <X> void onLogEvent(EventType<X> kind) {
Require.nonNull("Event type", kind);
kind.initializeListener(this);
Expand Down
3 changes: 2 additions & 1 deletion java/src/org/openqa/selenium/events/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("//java:defs.bzl", "java_library")
load("//java:defs.bzl", "artifact", "java_library")

java_library(
name = "events",
Expand All @@ -13,5 +13,6 @@ java_library(
"//java/src/org/openqa/selenium:core",
"//java/src/org/openqa/selenium/json",
"//java/src/org/openqa/selenium/status",
artifact("org.jspecify:jspecify"),
],
)
1 change: 1 addition & 0 deletions java/src/org/openqa/selenium/events/local/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ java_library(
"//java/src/org/openqa/selenium/events",
"//java/src/org/openqa/selenium/grid/config",
artifact("com.google.guava:guava"),
artifact("org.jspecify:jspecify"),
],
)
21 changes: 21 additions & 0 deletions java/src/org/openqa/selenium/events/local/package-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The SFC licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

@NullMarked
package org.openqa.selenium.events.local;

import org.jspecify.annotations.NullMarked;
21 changes: 21 additions & 0 deletions java/src/org/openqa/selenium/events/package-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The SFC licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

@NullMarked
package org.openqa.selenium.events;

import org.jspecify.annotations.NullMarked;
1 change: 1 addition & 0 deletions java/src/org/openqa/selenium/events/zeromq/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ java_library(
artifact("com.google.guava:guava"),
artifact("dev.failsafe:failsafe"),
artifact("org.zeromq:jeromq"),
artifact("org.jspecify:jspecify"),
],
)
48 changes: 24 additions & 24 deletions java/src/org/openqa/selenium/events/zeromq/UnboundZmqEventBus.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ class UnboundZmqEventBus implements EventBus {
private final Queue<UUID> recentMessages = EvictingQueue.create(128);
private final String encodedSecret;
private final ZMQ.Poller poller;

private ZMQ.Socket pub;
private ZMQ.Socket sub;
private final ZMQ.Socket pub;
private final ZMQ.Socket sub;

UnboundZmqEventBus(
ZContext context,
Expand Down Expand Up @@ -136,20 +135,26 @@ class UnboundZmqEventBus implements EventBus {
.build();

// Access to the zmq socket is safe here: no threads.
Failsafe.with(retryPolicy)
.run(
() -> {
sub = context.createSocket(SocketType.SUB);
sub.setIPv6(isSubAddressIPv6(publishConnection));
ZmqUtils.configureHeartbeat(sub, heartbeatPeriod, "SUB");
sub.connect(publishConnection);
sub.subscribe(new byte[0]);

pub = context.createSocket(SocketType.PUB);
pub.setIPv6(isSubAddressIPv6(subscribeConnection));
ZmqUtils.configureHeartbeat(pub, heartbeatPeriod, "PUB");
pub.connect(subscribeConnection);
});
ZMQ.Socket[] pubSub =
Failsafe.with(retryPolicy)
.get(
() -> {
ZMQ.Socket sub = context.createSocket(SocketType.SUB);
sub.setIPv6(isSubAddressIPv6(publishConnection));
ZmqUtils.configureHeartbeat(sub, heartbeatPeriod, "SUB");
sub.connect(publishConnection);
sub.subscribe(new byte[0]);

ZMQ.Socket pub = context.createSocket(SocketType.PUB);
pub.setIPv6(isSubAddressIPv6(subscribeConnection));
ZmqUtils.configureHeartbeat(pub, heartbeatPeriod, "PUB");
pub.connect(subscribeConnection);

return new ZMQ.Socket[] {pub, sub};
});
this.pub = pubSub[0];
this.sub = pubSub[1];

// Connections are already established
this.poller = context.createPoller(1);
this.poller.register(Objects.requireNonNull(sub), ZMQ.Poller.POLLIN);
Expand Down Expand Up @@ -227,13 +232,8 @@ public void close() {
ExecutorServices.shutdownGracefully(
"Event Bus Listener Notifier", listenerNotificationExecutor);
poller.close();

if (sub != null) {
sub.close();
}
if (pub != null) {
pub.close();
}
sub.close();
pub.close();
}

private class PollingRunnable implements Runnable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ public static class RejectedEvent {
this.data = data;
}

@SuppressWarnings("DataFlowIssue")
private static RejectedEvent fromJson(JsonInput input) {
EventName name = null;
Object data = null;
Comment on lines +149 to 152
Copy link
Contributor

Choose a reason for hiding this comment

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

Action required

1. Rejectedevent may be null 🐞 Bug ✓ Correctness

ZeroMqEventBus.RejectedEvent.fromJson can construct a RejectedEvent with null name/data when the
JSON omits those fields, but the class lives in an @NullMarked package so those fields/getters are
treated as non-null. This can cause runtime NPEs in rejected-event handlers and can also trigger
nullness-check failures in builds running NullAway.
Agent Prompt
## Issue description
`org.openqa.selenium.events.zeromq` is now `@NullMarked`, but `ZeroMqEventBus.RejectedEvent.fromJson` can still build a `RejectedEvent` with `null` `name`/`data` when the JSON payload is missing fields.

## Issue Context
Because the package is `@NullMarked`, `RejectedEvent.name`/`data` and their getters are implicitly non-null. Returning instances with nulls violates that contract and risks runtime NPEs.

## Fix Focus Areas
- java/src/org/openqa/selenium/events/zeromq/ZeroMqEventBus.java[140-173]
- java/src/org/openqa/selenium/events/zeromq/ZeroMqEventBus.java[175-181]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down
21 changes: 21 additions & 0 deletions java/src/org/openqa/selenium/events/zeromq/package-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The SFC licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

@NullMarked
package org.openqa.selenium.events.zeromq;

import org.jspecify.annotations.NullMarked;
8 changes: 2 additions & 6 deletions java/src/org/openqa/selenium/interactions/Actions.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.IntConsumer;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
Expand All @@ -46,7 +45,6 @@
*
* <p>Call {@link #perform()} at the end of the method chain to actually perform the actions.
*/
@NullMarked
public class Actions {

private final WebDriver driver;
Expand All @@ -57,7 +55,7 @@ public class Actions {
private @Nullable PointerInput activePointer;
private @Nullable KeyInput activeKeyboard;
private @Nullable WheelInput activeWheel;
private Duration actionDuration;
private final Duration actionDuration;

public Actions(WebDriver driver) {
this(driver, Duration.ofMillis(250));
Expand Down Expand Up @@ -155,9 +153,7 @@ public Actions sendKeys(WebElement target, CharSequence... keys) {
}

private Actions sendKeysInTicks(CharSequence... keys) {
if (keys == null) {
throw new IllegalArgumentException("Keys should be a not null CharSequence");
}
Require.nonNull("Keys", keys, "should be a not null CharSequence");
for (CharSequence key : keys) {
key.codePoints()
.forEach(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@

import java.util.ArrayList;
import java.util.List;
import org.jspecify.annotations.NullMarked;
import org.openqa.selenium.internal.Require;

/** An action for aggregating actions and triggering all of them at the same time. */
@NullMarked
public class CompositeAction implements Action {

private final List<Action> actionsList = new ArrayList<>();
Expand Down
2 changes: 0 additions & 2 deletions java/src/org/openqa/selenium/interactions/Coordinates.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@

package org.openqa.selenium.interactions;

import org.jspecify.annotations.NullMarked;
import org.openqa.selenium.Point;

/**
* Provides coordinates of an element for advanced interactions. Note that some coordinates (such as
* screen coordinates) are evaluated lazily since the element may have to be scrolled into view.
*/
@NullMarked
public interface Coordinates {

/**
Expand Down
5 changes: 2 additions & 3 deletions java/src/org/openqa/selenium/interactions/Encodable.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@
package org.openqa.selenium.interactions;

import java.util.Map;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* This interface allows a custom {@link Interaction} to be JSON encoded for the W3C wire format. It
* should not normally be exposed or used by user-facing APIs. Instead, these should traffic in the
* {@link Interaction} interface.
*/
@NullMarked
public interface Encodable {
Map<String, Object> encode();
Map<String, @Nullable Object> encode();
}
3 changes: 0 additions & 3 deletions java/src/org/openqa/selenium/interactions/InputSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@

package org.openqa.selenium.interactions;

import org.jspecify.annotations.NullMarked;

/**
* Models an <a href="https://www.w3.org/TR/webdriver/#dfn-input-source">input source</a> as defined
* and used by the W3C WebDriver spec.
*/
@NullMarked
public interface InputSource {
SourceType getInputType();

Expand Down
3 changes: 0 additions & 3 deletions java/src/org/openqa/selenium/interactions/Interaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@

import static java.util.Objects.requireNonNull;

import org.jspecify.annotations.NullMarked;

/**
* Used as the basis of {@link Sequence}s for the W3C WebDriver spec <a
* href="https://www.w3.org/TR/webdriver/#actions">Action commands</a>.
*/
@NullMarked
public abstract class Interaction {

private final InputSource source;
Expand Down
2 changes: 0 additions & 2 deletions java/src/org/openqa/selenium/interactions/Interactive.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@
package org.openqa.selenium.interactions;

import java.util.Collection;
import org.jspecify.annotations.NullMarked;

/**
* Indicates that a class can be used with the W3C WebDriver <a
* href="https://www.w3.org/TR/webdriver/#actions">Actions commands</a>.
*/
@NullMarked
public interface Interactive {
void perform(Collection<Sequence> actions);

Expand Down
6 changes: 2 additions & 4 deletions java/src/org/openqa/selenium/interactions/KeyInput.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Models a <a href="https://www.w3.org/TR/webdriver/#dfn-key-input-source">key input source</a>.
*/
@NullMarked
public class KeyInput implements InputSource, Encodable {

private final String name;
Expand Down Expand Up @@ -55,8 +53,8 @@ public Interaction createKeyUp(int codePoint) {
}

@Override
public Map<String, Object> encode() {
Map<String, Object> toReturn = new HashMap<>();
public Map<String, @Nullable Object> encode() {
Map<String, @Nullable Object> toReturn = new HashMap<>();

toReturn.put("type", getInputType().getType());
toReturn.put("id", name);
Expand Down
3 changes: 0 additions & 3 deletions java/src/org/openqa/selenium/interactions/Locatable.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@

package org.openqa.selenium.interactions;

import org.jspecify.annotations.NullMarked;

@NullMarked
public interface Locatable {
Coordinates getCoordinates();
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@

package org.openqa.selenium.interactions;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.openqa.selenium.WebDriverException;

/**
* Indicates that the target provided to the actions move() method is invalid - outside of the size
* of the window.
*/
@NullMarked
public class MoveTargetOutOfBoundsException extends WebDriverException {
public MoveTargetOutOfBoundsException(@Nullable String message) {
super(message);
Expand Down
2 changes: 0 additions & 2 deletions java/src/org/openqa/selenium/interactions/Pause.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import org.jspecify.annotations.NullMarked;

/** Indicates that a given {@link InputSource} should pause for a given duration. */
@NullMarked
public class Pause extends Interaction implements Encodable {

private final Duration duration;
Expand Down
6 changes: 2 additions & 4 deletions java/src/org/openqa/selenium/interactions/PointerInput.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.openqa.selenium.Point;
import org.openqa.selenium.WebElement;
Expand All @@ -35,7 +34,6 @@
* Models a <a href="https://www.w3.org/TR/webdriver/#dfn-pointer-input-source">pointer input
* source</a>.
*/
@NullMarked
public class PointerInput implements InputSource, Encodable {

private final Kind kind;
Expand All @@ -57,8 +55,8 @@ public SourceType getInputType() {
}

@Override
public Map<String, Object> encode() {
Map<String, Object> toReturn = new HashMap<>();
public Map<String, @Nullable Object> encode() {
Map<String, @Nullable Object> toReturn = new HashMap<>();

toReturn.put("type", getInputType().getType());
toReturn.put("id", name);
Expand Down
Loading
Loading