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

[Feature] Implement spotbugs scanning #11

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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: 1 addition & 1 deletion .github/workflows/build-pr.yml
Original file line number Diff line number Diff line change
@@ -21,4 +21,4 @@ jobs:
run: |
git config --global user.email "no-reply@github.com"
git config --global user.name "Github Actions"
./gradlew classes
./gradlew check
28 changes: 28 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import com.github.spotbugs.snom.Confidence
import com.github.spotbugs.snom.Effort
import com.github.spotbugs.snom.SpotBugsTask
import java.util.*

plugins {
@@ -6,6 +9,7 @@ plugins {
id("minestom.native-conventions")
alias(libs.plugins.blossom)
signing
id("com.github.spotbugs") version "6.0.15"
}

var baseVersion by extra("1.3.2")
@@ -47,6 +51,15 @@ java {
withSourcesJar()
}

spotbugs {
effort = Effort.MAX
reportLevel = Confidence.LOW
toolVersion = "4.8.5"
ignoreFailures = false
showStackTraces = true
showProgress = true
}

tasks {
jar {
manifest {
@@ -77,7 +90,18 @@ tasks {
minHeapSize = "512m"
maxHeapSize = "1024m"
}
withType<SpotBugsTask> {
reports.create("html") {
required = true
outputLocation = file("${layout.buildDirectory.get()}/reports/spotbugs.html")
setStylesheet("fancy-hist.xsl")
}

reports.create("xml") {
required = true
outputLocation = file("${layout.buildDirectory.get()}/reports/spotbugs.xml")
}
}

}

@@ -123,5 +147,9 @@ dependencies {

// BStats
api(libs.bstats.base)

spotbugsPlugins("com.h3xstream.findsecbugs:findsecbugs-plugin:1.12.0")
implementation("com.google.code.findbugs:annotations:3.0.1")

}

8 changes: 4 additions & 4 deletions src/main/java/net/minestom/server/ServerProcessImpl.java
Original file line number Diff line number Diff line change
@@ -238,7 +238,7 @@ public void start(@NotNull SocketAddress socketAddress) {
extension.start();
extension.gotoPreInit();

LOGGER.info("Starting " + MinecraftServer.getBrandName() + " server.");
LOGGER.info("Starting {} server.", MinecraftServer.getBrandName());

extension.gotoInit();

@@ -255,7 +255,7 @@ public void start(@NotNull SocketAddress socketAddress) {

extension.gotoPostInit();

LOGGER.info(MinecraftServer.getBrandName() + " server started successfully.");
LOGGER.info("{} server started successfully.", MinecraftServer.getBrandName());

if (ServerFlag.ATTRIBUTES_ENABLED) {
Attributes.registerAttributes();
@@ -281,7 +281,7 @@ public void start(@NotNull SocketAddress socketAddress) {
public void stop() {
if (!stopped.compareAndSet(false, true))
return;
LOGGER.info("Stopping " + MinecraftServer.getBrandName() + " server.");
LOGGER.info("Stopping {} server.", MinecraftServer.getBrandName());
LOGGER.info("Unloading all extensions.");
extension.shutdown();
scheduler.shutdown();
@@ -292,7 +292,7 @@ public void stop() {
MinestomTerminal.stop();
dispatcher.shutdown();
this.metrics.shutdown();
LOGGER.info(MinecraftServer.getBrandName() + " server stopped successfully.");
LOGGER.info("{MinecraftServer.getBrandName()} server stopped successfully.");
}

@Override
Original file line number Diff line number Diff line change
@@ -184,7 +184,7 @@ private static PhysicsResult computePhysics(@NotNull BoundingBox boundingBox,

return new PhysicsResult(finalPos, new Vec(remainingX, remainingY, remainingZ),
collisionY, collisionX, collisionY, collisionZ,
Vec.ZERO, null, null, false, finalResult);
Vec.ZERO, new Point[3], new Shape[3], false, finalResult);
}

private static void slowPhysics(@NotNull BoundingBox boundingBox,
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@
import org.jetbrains.annotations.Nullable;

import java.util.Iterator;
import java.util.Objects;

/**
* See https://wiki.vg/Entity_metadata#Mobs_2
@@ -260,4 +261,9 @@ public boolean equals(Object o) {
Vec dimensions = max.sub(min);
return new BoundingBox(dimensions.x(), dimensions.y(), dimensions.z(), min);
}

@Override
public int hashCode() {
return Objects.hash(width, height, depth, offset, relativeEnd);
}
}
Original file line number Diff line number Diff line change
@@ -319,7 +319,7 @@ record ValidExecutableCmd(CommandCondition condition, CommandExecutor globalList
executor().apply(sender, context);
return new ExecutionResultImpl(ExecutableCommand.Result.Type.SUCCESS, context.getReturnData());
} catch (Exception e) {
LOGGER.error("An exception was encountered while executing command: " + input(), e);
LOGGER.error("An exception was encountered while executing command: {}", new Object[]{input()}, e);
return ExecutionResultImpl.EXECUTOR_EXCEPTION;
}
}
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ public class EntityMeta {
private final WeakReference<Entity> entityRef;
protected final Metadata metadata;

public EntityMeta(@Nullable Entity entity, @NotNull Metadata metadata) {
public EntityMeta(@NotNull Entity entity, @NotNull Metadata metadata) {
this.entityRef = new WeakReference<>(entity);
this.metadata = metadata;
}
Original file line number Diff line number Diff line change
@@ -4,14 +4,13 @@
import net.minestom.server.entity.Metadata;
import net.minestom.server.entity.metadata.EntityMeta;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public class InteractionMeta extends EntityMeta {

public static final byte OFFSET = EntityMeta.MAX_OFFSET;
public static final byte MAX_OFFSET = OFFSET + 3;

public InteractionMeta(@Nullable Entity entity, @NotNull Metadata metadata) {
public InteractionMeta(@NotNull Entity entity, @NotNull Metadata metadata) {
super(entity, metadata);
}

Original file line number Diff line number Diff line change
@@ -78,7 +78,7 @@ public enum Type {
SWAMP,
TAIGA;

public final static Type[] VALUES = values();
public static final Type[] VALUES = values();
}

public enum Profession {
@@ -99,7 +99,7 @@ public enum Profession {
TOOLSMITH,
WEAPONSMITH;

public final static Profession[] VALUES = values();
public static final Profession[] VALUES = values();
}

public enum Level {
@@ -109,7 +109,7 @@ public enum Level {
EXPERT,
MASTER;

public final static Level[] VALUES = values();
public static final Level[] VALUES = values();
}

}
Original file line number Diff line number Diff line change
@@ -5,6 +5,8 @@
import net.minestom.server.network.packet.client.common.ClientPluginMessagePacket;
import org.jetbrains.annotations.NotNull;

import java.nio.charset.StandardCharsets;

/**
* Called when a player send {@link ClientPluginMessagePacket}.
*/
@@ -47,7 +49,7 @@ public byte[] getMessage() {
*/
@NotNull
public String getMessageString() {
return new String(message);
return new String(message, StandardCharsets.UTF_8);
}

@Override
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.net.MalformedURLException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.*;
import java.util.stream.Collectors;
@@ -394,7 +395,7 @@ private Extension loadExtension(@NotNull DiscoveredExtension discoveredExtension
LOGGER.info("Found indev folders for extension. Adding to list of discovered extensions.");
final String extensionClasses = System.getProperty(INDEV_CLASSES_FOLDER);
final String extensionResources = System.getProperty(INDEV_RESOURCES_FOLDER);
try (InputStreamReader reader = new InputStreamReader(new FileInputStream(new File(extensionResources, "extension.json")))) {
try (InputStreamReader reader = new InputStreamReader(new FileInputStream(new File(extensionResources, "extension.json")), StandardCharsets.UTF_8)) {
DiscoveredExtension extension = GSON.fromJson(reader, DiscoveredExtension.class);
extension.files.add(new File(extensionClasses).toURI().toURL());
extension.files.add(new File(extensionResources).toURI().toURL());
@@ -427,7 +428,7 @@ private Extension loadExtension(@NotNull DiscoveredExtension discoveredExtension
if (entry == null)
throw new IllegalStateException("Missing extension.json in extension " + file.getName() + ".");

InputStreamReader reader = new InputStreamReader(f.getInputStream(entry));
InputStreamReader reader = new InputStreamReader(f.getInputStream(entry), StandardCharsets.UTF_8);

// Initialize DiscoveredExtension from GSON.
DiscoveredExtension extension = GSON.fromJson(reader, DiscoveredExtension.class);
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@

import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.Key;
import java.security.MessageDigest;
@@ -38,7 +39,7 @@ public static void enable(@NotNull String secret) {
Check.stateCondition(MojangAuth.isEnabled(), "Velocity modern forwarding should not be enabled with MojangAuth");

VelocityProxy.enabled = true;
VelocityProxy.key = new SecretKeySpec(secret.getBytes(), MAC_ALGORITHM);
VelocityProxy.key = new SecretKeySpec(secret.getBytes(StandardCharsets.UTF_8), MAC_ALGORITHM);
}

/**
2 changes: 1 addition & 1 deletion src/main/java/net/minestom/server/instance/Section.java
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@
import static net.minestom.server.instance.light.LightCompute.emptyContent;
import static net.minestom.server.network.NetworkBuffer.SHORT;

public final class Section implements NetworkBuffer.Writer {
public final class Section implements NetworkBuffer.Writer, Cloneable {
private final Palette blockPalette;
private final Palette biomePalette;
private final Light skyLight;
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@
/**
* Palette containing a single value. Useful for both empty and full palettes.
*/
record FilledPalette(byte dim, int value) implements SpecializedPalette.Immutable {
record FilledPalette(byte dim, int value) implements SpecializedPalette.Immutable, Cloneable {
@Override
public int get(int x, int y, int z) {
return value;
Original file line number Diff line number Diff line change
@@ -37,4 +37,9 @@ public boolean equals(Object obj) {
if (this == obj) return true;
return obj != null && getClass() == obj.getClass();
}

@Override
public int hashCode() {
return super.hashCode();
}
}
Original file line number Diff line number Diff line change
@@ -75,12 +75,14 @@ private EntitySoundEffectPacket(@NotNull EntitySoundEffectPacket packet) {

@Override
public void write(@NotNull NetworkBuffer writer) {
if (soundEvent != null) {
if (soundEvent != null && soundName == null) {
writer.write(VAR_INT, soundEvent.id() + 1);
} else {
} else if (soundName != null && soundEvent == null){
writer.write(VAR_INT, 0);
writer.write(STRING, soundName);
writer.writeOptional(FLOAT, range);
} else {
return;
}
writer.write(VAR_INT, AdventurePacketConvertor.getSoundSourceValue(source));
writer.write(VAR_INT, entityId);
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@

import static net.minestom.server.network.NetworkBuffer.*;

public record ServerDataPacket(@Nullable Component motd, byte @Nullable [] iconBase64,
public record ServerDataPacket(@NotNull Component motd, byte @Nullable [] iconBase64,
boolean enforcesSecureChat) implements ServerPacket.Play {
public ServerDataPacket(@NotNull NetworkBuffer reader) {
this(reader.read(COMPONENT), reader.readOptional(BYTE_ARRAY),
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ public record SoundEffectPacket(
) implements ServerPacket.Play {

public SoundEffectPacket {
Check.argCondition(soundEvent == null && soundName == null, "soundEvent and soundName cannot both be null");
Check.argCondition(soundEvent == null || soundName == null, "soundEvent and soundName cannot both be null");
Check.argCondition(soundEvent != null && soundName != null, "soundEvent and soundName cannot both be present");
Check.argCondition(soundName == null && range != null, "range cannot be present if soundName is null");
}
@@ -81,12 +81,14 @@ private SoundEffectPacket(@NotNull SoundEffectPacket packet) {

@Override
public void write(@NotNull NetworkBuffer writer) {
if (soundEvent != null) {
if (soundEvent != null && soundName == null) {
writer.write(VAR_INT, soundEvent.id() + 1);
} else {
} else if (soundName != null && soundEvent == null) {
writer.write(VAR_INT, 0);
writer.write(STRING, soundName);
writer.writeOptional(FLOAT, range);
} else {
return;
}
writer.write(VAR_INT, AdventurePacketConvertor.getSoundSourceValue(source));
writer.write(INT, x * 8);
5 changes: 3 additions & 2 deletions src/main/java/net/minestom/server/registry/Registry.java
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
@@ -83,7 +84,7 @@ public static Map<String, Map<String, Object>> load(Resource resource) {
Map<String, Map<String, Object>> map = new HashMap<>();
try (InputStream resourceStream = Registry.class.getClassLoader().getResourceAsStream(resource.name)) {
Check.notNull(resourceStream, "Resource {0} does not exist!", resource);
try (JsonReader reader = new JsonReader(new InputStreamReader(resourceStream))) {
try (JsonReader reader = new JsonReader(new InputStreamReader(resourceStream, StandardCharsets.UTF_8))) {
reader.beginObject();
while (reader.hasNext()) map.put(reader.nextName(), (Map<String, Object>) readObject(reader));
reader.endObject();
@@ -189,7 +190,7 @@ public Collection<T> values() {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Container<?> container)) return false;
if (!(o instanceof DynamicContainer<?> container)) return false;
return resource == container.resource;
}

7 changes: 5 additions & 2 deletions src/main/java/net/minestom/server/utils/url/URLUtils.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
package net.minestom.server.utils.url;

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

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URI;

public final class URLUtils {

private URLUtils() {

}

@SuppressFBWarnings("")
public static String getText(String url) throws IOException {
HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection();
HttpURLConnection connection = (HttpURLConnection) URI.create(url).toURL().openConnection();
//add headers to the connection, or check the status if desired..

// handle error response code it occurs
Original file line number Diff line number Diff line change
@@ -68,7 +68,7 @@ public void addBiome(@NotNull Biome biome) {
public void removeBiome(@NotNull Biome biome) {
var id = idMappings.get(biome.namespace());
if (id != null) {
biomes.remove(id);
biomes.remove(biome);
biomesByName.remove(biome.namespace());
idMappings.remove(biome.namespace());
nbtCache = null;
Loading