From 81341135109241562f043437260dc737c2706fbf Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Mon, 23 Jan 2017 10:46:01 +0100 Subject: [PATCH] Cleaned up more PMD/Codacy warnings --- .../net/schmizz/sshj/examples/KeepAlive.java | 3 - .../net/schmizz/sshj/common/StreamCopier.java | 7 +- .../AbstractForwardedChannelOpener.java | 1 - .../net/schmizz/sshj/sftp/PacketReader.java | 1 - .../java/net/schmizz/sshj/sftp/Response.java | 3 +- .../sshj/signature/AbstractSignature.java | 2 +- .../sshj/signature/SignatureECDSA.java | 2 +- .../schmizz/sshj/transport/KeyExchanger.java | 2 +- .../schmizz/sshj/transport/TransportImpl.java | 65 ++++++++++--------- .../schmizz/sshj/userauth/UserAuthImpl.java | 24 ++++--- 10 files changed, 54 insertions(+), 56 deletions(-) diff --git a/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java b/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java index 0049b4e4..6c843811 100644 --- a/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java +++ b/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java @@ -3,14 +3,11 @@ package net.schmizz.sshj.examples; import net.schmizz.keepalive.KeepAliveProvider; import net.schmizz.sshj.DefaultConfig; import net.schmizz.sshj.SSHClient; -import net.schmizz.sshj.common.IOUtils; import net.schmizz.sshj.connection.channel.direct.Session; -import net.schmizz.sshj.connection.channel.direct.Session.Command; import net.schmizz.sshj.transport.verification.PromiscuousVerifier; import java.io.IOException; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; /** This examples demonstrates how to setup keep-alive to detect connection dropping. */ public class KeepAlive { diff --git a/src/main/java/net/schmizz/sshj/common/StreamCopier.java b/src/main/java/net/schmizz/sshj/common/StreamCopier.java index 8260b92e..1fedc075 100644 --- a/src/main/java/net/schmizz/sshj/common/StreamCopier.java +++ b/src/main/java/net/schmizz/sshj/common/StreamCopier.java @@ -68,8 +68,11 @@ public class StreamCopier { } public StreamCopier listener(Listener listener) { - if (listener == null) listener = NULL_LISTENER; - this.listener = listener; + if (listener == null) { + this.listener = NULL_LISTENER; + } else { + this.listener = listener; + } return this; } diff --git a/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java b/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java index d79835ee..b1a59563 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java @@ -21,7 +21,6 @@ import net.schmizz.sshj.connection.channel.Channel; import net.schmizz.sshj.connection.channel.OpenFailException; import net.schmizz.sshj.transport.TransportException; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; diff --git a/src/main/java/net/schmizz/sshj/sftp/PacketReader.java b/src/main/java/net/schmizz/sshj/sftp/PacketReader.java index 5dc29a33..a2db39ab 100644 --- a/src/main/java/net/schmizz/sshj/sftp/PacketReader.java +++ b/src/main/java/net/schmizz/sshj/sftp/PacketReader.java @@ -18,7 +18,6 @@ package net.schmizz.sshj.sftp; import net.schmizz.concurrent.Promise; import net.schmizz.sshj.common.SSHException; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; diff --git a/src/main/java/net/schmizz/sshj/sftp/Response.java b/src/main/java/net/schmizz/sshj/sftp/Response.java index a44fac9d..c269291a 100644 --- a/src/main/java/net/schmizz/sshj/sftp/Response.java +++ b/src/main/java/net/schmizz/sshj/sftp/Response.java @@ -20,7 +20,7 @@ import net.schmizz.sshj.common.Buffer; public final class Response extends SFTPPacket { - public static enum StatusCode { + public enum StatusCode { UNKNOWN(-1), OK(0), EOF(1), @@ -122,6 +122,7 @@ public final class Response return ensurePacketTypeIs(PacketType.STATUS).ensureStatusIs(StatusCode.OK); } + @SuppressWarnings("PMD.CompareObjectsWithEquals") public Response ensureStatusIs(StatusCode acceptable) throws SFTPException { final StatusCode sc = readStatusCode(); diff --git a/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java b/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java index 89b39917..9cd44a6f 100644 --- a/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java +++ b/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java @@ -84,7 +84,7 @@ public abstract class AbstractSignature | sig[i++] & 0x000000ff; byte[] newSig = new byte[j]; System.arraycopy(sig, i, newSig, 0, j); - sig = newSig; + return newSig; } return sig; } diff --git a/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java b/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java index 0abe8ca8..93e228a6 100644 --- a/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java +++ b/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java @@ -79,7 +79,7 @@ public class SignatureECDSA throw new SSHRuntimeException(String.format("Signature :: ecdsa-sha2-nistp256 expected, got %s", algo)); } final int rsLen = sigbuf.readUInt32AsInt(); - if (!(sigbuf.available() == rsLen)) { + if (sigbuf.available() != rsLen) { throw new SSHRuntimeException("Invalid key length"); } r = sigbuf.readBytes(); diff --git a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java index 998b0c80..12432063 100644 --- a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java +++ b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java @@ -26,7 +26,6 @@ import net.schmizz.sshj.transport.mac.MAC; import net.schmizz.sshj.transport.verification.AlgorithmsVerifier; import net.schmizz.sshj.transport.verification.HostKeyVerifier; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.math.BigInteger; import java.security.GeneralSecurityException; @@ -158,6 +157,7 @@ final class KeyExchanger "Key exchange packet received when key exchange was not ongoing"); } + @SuppressWarnings("PMD.CompareObjectsWithEquals") private static void ensureReceivedMatchesExpected(Message got, Message expected) throws TransportException { if (got != expected) diff --git a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java index 6d2073d7..8aba4a62 100644 --- a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java +++ b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java @@ -33,7 +33,9 @@ import java.io.OutputStream; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; -/** A thread-safe {@link Transport} implementation. */ +/** + * A thread-safe {@link Transport} implementation. + */ public final class TransportImpl implements Transport, DisconnectListener { @@ -49,11 +51,11 @@ public final class TransportImpl static final class ConnInfo { final String host; - final int port; final InputStream in; final OutputStream out; - public ConnInfo(String host, int port, InputStream in, OutputStream out) { + + ConnInfo(String host, int port, InputStream in, OutputStream out) { this.host = host; this.port = port; this.in = in; @@ -88,24 +90,32 @@ public final class TransportImpl private final Event close; - /** Client version identification string */ + /** + * Client version identification string + */ private final String clientID; private volatile int timeoutMs = 30 * 1000; // Crazy long, but it was the original default private volatile boolean authed = false; - /** Currently active service e.g. UserAuthService, ConnectionService */ + /** + * Currently active service e.g. UserAuthService, ConnectionService + */ private volatile Service service; private DisconnectListener disconnectListener; private ConnInfo connInfo; - /** Server version identification string */ + /** + * Server version identification string + */ private String serverID; - /** Message identifier of last packet received */ + /** + * Message identifier of last packet received + */ private Message msg; private final ReentrantLock writeLock = new ReentrantLock(); @@ -115,9 +125,9 @@ public final class TransportImpl this.loggerFactory = config.getLoggerFactory(); this.serviceAccept = new Event("service accept", TransportException.chainer, loggerFactory); this.close = new Event("transport close", TransportException.chainer, loggerFactory); - this.nullService = new NullService(this); + this.nullService = new NullService(this); this.service = nullService; - this.log = loggerFactory.getLogger(getClass()); + this.log = loggerFactory.getLogger(getClass()); this.disconnectListener = this; this.reader = new Reader(this); this.encoder = new Encoder(config.getRandomFactory().create(), writeLock, loggerFactory); @@ -138,7 +148,7 @@ public final class TransportImpl this.serviceAccept = new Event("service accept", TransportException.chainer, loggerFactory); this.close = new Event("transport close", TransportException.chainer, loggerFactory); this.log = loggerFactory.getLogger(getClass()); - this.nullService = new NullService(this); + this.nullService = new NullService(this); this.service = nullService; this.disconnectListener = this; this.reader = new Reader(this); @@ -194,6 +204,7 @@ public final class TransportImpl /** * Receive the server identification string. + * * @throws IOException If there was an error writing to the outputstream. */ private void sendClientIdent() throws IOException { @@ -212,9 +223,7 @@ public final class TransportImpl * This is not efficient but is only done once. * * @param buffer The buffer to read from. - * * @return empty string if full ident string has not yet been received - * * @throws IOException */ private String readIdentification(Buffer.PlainBuffer buffer) @@ -226,7 +235,7 @@ public final class TransportImpl if (!ident.startsWith("SSH-2.0-") && !ident.startsWith("SSH-1.99-")) throw new TransportException(DisconnectReason.PROTOCOL_VERSION_NOT_SUPPORTED, - "Server does not support SSHv2, identified as: " + ident); + "Server does not support SSHv2, identified as: " + ident); return ident; } @@ -337,7 +346,6 @@ public final class TransportImpl * Sends a service request for the specified service * * @param serviceName name of the service being requested - * * @throws TransportException if there is an error while sending the request */ private void sendServiceRequest(String serviceName) @@ -456,9 +464,9 @@ public final class TransportImpl log.debug("Sending SSH_MSG_DISCONNECT: reason=[{}], msg=[{}]", reason, message); try { write(new SSHPacket(Message.DISCONNECT) - .putUInt32(reason.toInt()) - .putString(message) - .putString("")); + .putUInt32(reason.toInt()) + .putString(message) + .putString("")); } catch (IOException worthless) { log.debug("Error writing packet: {}", worthless.toString()); } @@ -476,7 +484,6 @@ public final class TransportImpl * * @param msg the message identifer * @param buf buffer containg rest of the packet - * * @throws SSHException if an error occurs during handling (unrecoverable) */ @Override @@ -494,32 +501,27 @@ public final class TransportImpl else switch (msg) { - case DISCONNECT: { + case DISCONNECT: gotDisconnect(buf); break; - } - case IGNORE: { + case IGNORE: log.debug("Received SSH_MSG_IGNORE"); break; - } - case UNIMPLEMENTED: { + case UNIMPLEMENTED: gotUnimplemented(buf); break; - } - case DEBUG: { + case DEBUG: gotDebug(buf); break; - } - case SERVICE_ACCEPT: { + case SERVICE_ACCEPT: gotServiceAccept(); break; - } - case USERAUTH_BANNER: { + case USERAUTH_BANNER: log.debug("Received USERAUTH_BANNER"); break; - } default: sendUnimplemented(); + break; } } @@ -552,7 +554,7 @@ public final class TransportImpl try { if (!serviceAccept.hasWaiters()) throw new TransportException(DisconnectReason.PROTOCOL_ERROR, - "Got a service accept notification when none was awaited"); + "Got a service accept notification when none was awaited"); serviceAccept.set(); } finally { serviceAccept.unlock(); @@ -563,7 +565,6 @@ public final class TransportImpl * Got an SSH_MSG_UNIMPLEMENTED, so lets see where we're at and act accordingly. * * @param packet The 'unimplemented' packet received - * * @throws TransportException */ private void gotUnimplemented(SSHPacket packet) diff --git a/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java b/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java index 4171e49a..d6b7cc77 100644 --- a/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java +++ b/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java @@ -32,7 +32,9 @@ import java.util.LinkedList; import java.util.List; import java.util.concurrent.TimeUnit; -/** {@link UserAuth} implementation. */ +/** + * {@link UserAuth} implementation. + */ public class UserAuthImpl extends AbstractService implements UserAuth { @@ -111,22 +113,20 @@ public class UserAuthImpl try { switch (msg) { - case USERAUTH_BANNER: { + case USERAUTH_BANNER: banner = buf.readString(); - } - break; + break; - case USERAUTH_SUCCESS: { + case USERAUTH_SUCCESS: // In order to prevent race conditions, we immediately set the authenticated flag on the transport // And change the service before delivering the authenticated promise. // Should fix https://github.com/hierynomus/sshj/issues/237 trans.setAuthenticated(); // So it can put delayed compression into force if applicable trans.setService(nextService); // We aren't in charge anymore, next service is authenticated.deliver(true); - } - break; + break; - case USERAUTH_FAILURE: { + case USERAUTH_FAILURE: allowedMethods = Arrays.asList(buf.readString().split(",")); partialSuccess |= buf.readBoolean(); if (allowedMethods.contains(currentMethod.getName()) && currentMethod.shouldRetry()) { @@ -134,18 +134,16 @@ public class UserAuthImpl } else { authenticated.deliver(false); } - } - break; + break; - default: { + default: log.debug("Asking `{}` method to handle {} packet", currentMethod.getName(), msg); try { currentMethod.handle(msg, buf); } catch (UserAuthException e) { authenticated.deliverError(e); } - } - + break; } } finally { authenticated.unlock();