Skip to content

Commit

Permalink
Fix Sonatype Lift findings
Browse files Browse the repository at this point in the history
Signed-off-by: Kai Hudalla <kai.hudalla@bosch.io>
  • Loading branch information
sophokles73 committed Sep 6, 2022
1 parent 212fe46 commit 40e1d98
Show file tree
Hide file tree
Showing 37 changed files with 237 additions and 81 deletions.
2 changes: 2 additions & 0 deletions .lift.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# These two Infer rules seem to only produce false positives in Hono
# ignoreRules = [ "INTERFACE_NOT_THREAD_SAFE", "RESOURCE_LEAK" ]
13 changes: 13 additions & 0 deletions bom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,19 @@ quarkus.vertx.resolver.cache-max-time-to-live=0
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
<version>4.7.1</version>
<optional>true</optional>
<scope>provided</scope>
<exclusions>
<exclusion>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.bol</groupId>
<artifactId>cryptvault</artifactId>
Expand Down
5 changes: 5 additions & 0 deletions cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@
<artifactId>jansi</artifactId>
<version>1.18</version>
</dependency>
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
<optional>true</optional>
</dependency>

<!-- testing -->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.quarkus.runtime.ShutdownEvent;
import io.vertx.core.Future;
import io.vertx.core.Promise;
Expand All @@ -83,6 +84,13 @@
mixinStandardHelpOptions = true,
versionProvider = PropertiesVersionProvider.class,
sortOptions = false)
@SuppressFBWarnings(
value = "HARD_CODE_PASSWORD",
justification = """
We use the default passwords of the Hono Sandbox installation throughout this class
for ease of use. The passwords are publicly documented and do not affect any
private installations of Hono.
""")
public class AmqpAdapter implements Callable<Integer> {

private static final Logger LOG = LoggerFactory.getLogger(AmqpAdapter.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ private void printResponse(final DownstreamMessage<?> response) {
.orElse("-")));
}

@SuppressWarnings("CatchAndPrintStackTrace")
private void readAndExecuteCommands() {
AnsiConsole.systemInstall();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.quarkus.runtime.ShutdownEvent;
import io.vertx.core.Future;
import io.vertx.core.Promise;
Expand All @@ -63,6 +64,13 @@
versionProvider = PropertiesVersionProvider.class,
sortOptions = false,
subcommands = { TelemetryAndEvent.class, CommandAndControl.class })
@SuppressFBWarnings(
value = "HARD_CODE_PASSWORD",
justification = """
We use the default passwords of the Hono Sandbox installation throughout this class
for ease of use. The passwords are publicly documented and do not affect any
private installations of Hono.
""")
public class NorthBoundApis {

private static final Logger LOG = LoggerFactory.getLogger(NorthBoundApis.class);
Expand Down
5 changes: 5 additions & 0 deletions clients/amqp-connection/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
<artifactId>smallrye-config</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
<optional>true</optional>
</dependency>

<!-- Testing -->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.eclipse.hono.util.Constants;
import org.eclipse.hono.util.Strings;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
* Common configuration properties required for accessing and authenticating to a remote server.
*
Expand Down Expand Up @@ -232,6 +234,12 @@ public final void setCredentialsPath(final String path) {
this.credentialsPath = path;
}

@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = """
The path that the credentials are read from is determined from configuration properties that
are supposed to be passed in during startup of the component only.
""")
private void loadCredentials() {

if (Strings.isNullOrEmpty(username) && Strings.isNullOrEmpty(password) && !Strings.isNullOrEmpty(credentialsPath)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,16 +636,15 @@ public final String getAddressRewriteRule() {
* {@link org.eclipse.hono.client.amqp.config.AddressHelper#rewrite(String, ClientConfigProperties)} method.
*
* @param addressRewriteRule The rewrite rule to be applied to the address.
* @throws PatternSyntaxException if the rewrite rule contains an invalid pattern definition.
*/
public final void setAddressRewriteRule(final String addressRewriteRule) {
this.addressRewriteRule = addressRewriteRule;
if (!Strings.isNullOrEmpty(addressRewriteRule)) {
final String[] elements = addressRewriteRule.split(" ", 2);
if (elements.length == 2) {
try {
addressRewritePattern = Pattern.compile(elements[0]);
addressRewriteReplacement = elements[1];
} catch (PatternSyntaxException pe) { }
addressRewritePattern = Pattern.compile(elements[0]);
addressRewriteReplacement = elements[1];
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.opentracing.Tracer;
import io.opentracing.noop.NoopTracerFactory;
import io.vertx.core.AsyncResult;
Expand Down Expand Up @@ -284,7 +285,7 @@ private void checkConnected(final Handler<AsyncResult<Void>> resultHandler, fina
*
* @return {@code true} if the connection is established.
*/
protected boolean isConnectedInternal() {
private boolean isConnectedInternal() {
return connection != null && !connection.isDisconnected() && session != null;
}

Expand Down Expand Up @@ -314,17 +315,6 @@ private void setConnection(final ProtonConnection connection, final ProtonSessio
}
}

/**
* Gets the underlying connection object that this client uses to interact with the server.
*
* @return The connection.
*/
protected ProtonConnection getConnection() {
synchronized (connectionLock) {
return this.connection;
}
}

@Override
public boolean supportsCapability(final Symbol capability) {
if (capability == null) {
Expand Down Expand Up @@ -1092,6 +1082,12 @@ private void connect() {
});
}

@SuppressFBWarnings(
value = "PREDICTABLE_RANDOM",
justification = """
The values returned by the ThreadLocalRandom are only used for calculating a
random amount of time to wait before trying to reconnect.
""")
private void reconnect(final Throwable connectionFailureCause) {
if (cancelled.get()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.eclipse.hono.client.amqp.config.AddressHelper;
import org.eclipse.hono.client.amqp.config.ClientConfigProperties;
import org.junit.jupiter.api.Test;

/**
Expand Down Expand Up @@ -58,7 +56,7 @@ public void testAddressRewriteWithNonValidSyntaxValue() {
*/
@Test
public void testAddressRewriteForNonMatchingPattern() {
assertEquals(address, AddressHelper.rewrite(address, createConfig("* *")));
assertEquals(address, AddressHelper.rewrite(address, createConfig("something $0")));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@

import java.time.Duration;
import java.time.Instant;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Deque;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -371,7 +372,7 @@ protected void onPartitionsAssignedBlocking(final Set<io.vertx.kafka.client.comm
// synchronized because offsetsMap is accessed from vert.x event loop and kafka polling thread
private synchronized void ensureOffsetCommitsExistForNewlyAssignedPartitions(
final Set<io.vertx.kafka.client.common.TopicPartition> partitionsSet) {
final List<TopicPartition> partitionsForNextCommit = new LinkedList<>();
final List<TopicPartition> partitionsForNextCommit = new ArrayList<>();

final Set<TopicPartition> partitionsToCheckCommittedOffsetsFor = partitionsSet.stream().map(Helper::to)
.filter(partition -> !offsetsMap.containsKey(partition) && lastKnownCommittedOffsets.get(partition) == null)
Expand Down Expand Up @@ -599,7 +600,7 @@ class TopicPartitionOffsets {

private static final long UNDEFINED_OFFSET = -2;
private final TopicPartition topicPartition;
private final Deque<OffsetsQueueEntry> queue = new LinkedList<>();
private final Deque<OffsetsQueueEntry> queue = new ArrayDeque<>();

private long lastSequentiallyCompletedOffset = UNDEFINED_OFFSET;
private long lastCommittedOffset = UNDEFINED_OFFSET;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -781,7 +780,7 @@ private void ensurePositionsHaveBeenSetIfNeeded(final Set<TopicPartition> assign
final var partitions = Helper.to(assignedPartitions);
// handle an exception across all position() invocations - the underlying server fetch that may trigger an exception is done for multiple partitions anyway
try {
final List<org.apache.kafka.common.TopicPartition> outOfRangeOffsetPartitions = new LinkedList<>();
final List<org.apache.kafka.common.TopicPartition> outOfRangeOffsetPartitions = new ArrayList<>();
final var beginningOffsets = getUnderlyingConsumer().beginningOffsets(partitions);
partitions.forEach(partition -> {
final long position = getUnderlyingConsumer().position(partition);
Expand Down Expand Up @@ -1030,6 +1029,7 @@ protected void runOnContext(final Handler<Void> codeToRun) {
* has been set yet on the Kafka consumer.
* @throws NullPointerException if handler is {@code null}.
*/
@SuppressWarnings("FutureReturnValueIgnored")
protected void runOnKafkaWorkerThread(final Handler<Void> handler) {
Objects.requireNonNull(handler);
if (kafkaConsumerWorker == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -66,7 +65,7 @@ public final class MicrometerKafkaClientMetricsSupport implements KafkaClientMet
private static final Logger LOG = LoggerFactory.getLogger(MicrometerKafkaClientMetricsSupport.class);
private static final String PREFIX_KAFKA = "kafka.";

private final List<MeterRegistry> boundMeterRegistries = new LinkedList<>();
private final List<MeterRegistry> boundMeterRegistries = new ArrayList<>();
private final Map<Producer<?, ?>, KafkaClientMetrics> producerMetricsMap = new HashMap<>();
private final Map<Consumer<?, ?>, KafkaClientMetrics> consumerMetricsMap = new HashMap<>();
private final boolean producerMetricsEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ private void logError(
TracingHelper.logError(span, cause);
}

@SuppressWarnings("unused")
private int getErrorCode(final Throwable t) {
/*
* TODO set error code depending on type of exception?
Expand Down
5 changes: 5 additions & 0 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
<optional>true</optional>
</dependency>

<!-- testing -->
<dependency>
Expand Down
19 changes: 19 additions & 0 deletions core/src/main/java/org/eclipse/hono/config/AbstractConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.vertx.core.net.JksOptions;
import io.vertx.core.net.KeyCertOptions;
import io.vertx.core.net.PemKeyCertOptions;
Expand Down Expand Up @@ -140,6 +141,12 @@ protected AbstractConfig(final GenericOptions other) {
* @return The password.
* @throws NullPointerException if any of the parameters are {@code null}.
*/
@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = """
The path that the password is read from is determined from configuration properties that
are supposed to be passed in during startup of the component only.
""")
protected final String getPassword(final String purpose, final String value) {

Objects.requireNonNull(purpose);
Expand Down Expand Up @@ -258,6 +265,12 @@ public final TrustOptions getTrustOptions() {
}


@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = """
The path that the trust store is read from is determined from configuration properties that
are supposed to be passed in during startup of the component only.
""")
private TrustOptions createTrustOptions() {

if (trustOptions != null) {
Expand Down Expand Up @@ -363,6 +376,12 @@ public final KeyCertOptions getKeyCertOptions() {
return keyCertOptions;
}

@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = """
The paths that the certificate and key are read from are determined from configuration properties that
are supposed to be passed in during startup of the component only.
""")
private KeyCertOptions createKeyCertOptions() {

if (!Strings.isNullOrEmpty(this.keyPath) && !Strings.isNullOrEmpty(this.certPath)) {
Expand Down
7 changes: 7 additions & 0 deletions core/src/main/java/org/eclipse/hono/config/KeyLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.vertx.core.Vertx;
import io.vertx.core.buffer.Buffer;
import io.vertx.core.net.impl.pkcs1.PrivateKeyParser;
Expand Down Expand Up @@ -180,6 +181,12 @@ private static PrivateKey generatePrivateKey(final String algorithm, final KeySp
.generatePrivate(keySpec);
}

@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = """
The path that the file is read from is determined from configuration properties that
are supposed to be passed in during startup of the component only.
""")
private static <R> R processFile(final Vertx vertx, final String pathName, final PemProcessor<R> processor) {

final Path path = Paths.get(pathName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
import javax.naming.ldap.Rdn;
import javax.security.auth.x500.X500Principal;

import org.eclipse.hono.util.RegistryManagementConstants;
import org.eclipse.hono.util.Strings;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
* A utility class for handling template used for generating device and authentication identifiers during
Expand Down Expand Up @@ -131,6 +130,9 @@ String getValue(final List<Rdn> rdns) {
* configured in the template are not present in the subject DN.
* @throws NullPointerException if any of the parameters are {@code null}.
*/
@SuppressFBWarnings(
value = "LDAP_INJECTION",
justification = "we never run an LDAP search but merely use the class for string parsing")
public String apply(final String subjectDN) {
Objects.requireNonNull(subjectDN, "subjectDN must not be null");

Expand Down
Loading

0 comments on commit 40e1d98

Please sign in to comment.