From d2a1025b3fbec4fa874975d9ce4b19a73587c78d Mon Sep 17 00:00:00 2001 From: zzz <zzz@mail.i2p> Date: Sun, 10 Nov 2013 20:07:46 +0000 Subject: [PATCH] about 20 findbugs fixes all over --- .../java/src/org/klomp/snark/Snark.java | 2 +- .../src/org/klomp/snark/SnarkManager.java | 4 +- .../src/org/klomp/snark/bencode/BDecoder.java | 2 +- .../src/org/klomp/snark/web/BasicServlet.java | 5 ++- .../src/org/klomp/snark/web/FetchAndAdd.java | 2 +- .../src/net/i2p/i2ptunnel/irc/IRCFilter.java | 2 +- .../net/i2p/router/web/ConfigNetHelper.java | 4 +- .../org/mortbay/servlet/MultiPartRequest.java | 8 ++-- .../support/CPUInformation/CPUIDCPUInfo.java | 14 +++---- core/java/src/net/i2p/data/DataHelper.java | 40 +++++++++++++++++++ core/java/src/net/i2p/util/SipHashInline.java | 18 ++++----- .../java/src/net/i2p/util/ZipFileComment.java | 4 +- .../metanotion/io/block/index/BSkipSpan.java | 1 + router/java/src/net/i2p/router/Blocklist.java | 10 +++-- .../src/net/i2p/router/transport/GeoIPv6.java | 2 +- .../src/net/i2p/router/transport/UPnP.java | 12 +++++- 16 files changed, 92 insertions(+), 38 deletions(-) diff --git a/apps/i2psnark/java/src/org/klomp/snark/Snark.java b/apps/i2psnark/java/src/org/klomp/snark/Snark.java index fb4de137a9..ccb9226897 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/Snark.java +++ b/apps/i2psnark/java/src/org/klomp/snark/Snark.java @@ -232,7 +232,7 @@ public class Snark private byte[] id; private final byte[] infoHash; private String additionalTrackerURL; - private final I2PSnarkUtil _util; + protected final I2PSnarkUtil _util; private final Log _log; private final PeerCoordinatorSet _peerCoordinatorSet; private volatile String trackerProblems; diff --git a/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java b/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java index 43fb40e8ad..6517b4cd5d 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java +++ b/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java @@ -531,7 +531,7 @@ public class SnarkManager implements CompleteListener { _util.setStartupDelay(minutes); changed = true; _config.setProperty(PROP_STARTUP_DELAY, Integer.toString(minutes)); - addMessage(_("Startup delay changed to {0}", DataHelper.formatDuration2(minutes * 60 * 1000))); + addMessage(_("Startup delay changed to {0}", DataHelper.formatDuration2(minutes * (60L * 1000)))); } } @@ -1489,7 +1489,7 @@ public class SnarkManager implements CompleteListener { private class DirMonitor implements Runnable { public void run() { // don't bother delaying if auto start is false - long delay = 60 * 1000 * getStartupDelayMinutes(); + long delay = (60L * 1000) * getStartupDelayMinutes(); if (delay > 0 && shouldAutoStart()) { addMessage(_("Adding torrents in {0}", DataHelper.formatDuration2(delay))); try { Thread.sleep(delay); } catch (InterruptedException ie) {} diff --git a/apps/i2psnark/java/src/org/klomp/snark/bencode/BDecoder.java b/apps/i2psnark/java/src/org/klomp/snark/bencode/BDecoder.java index 5076b77051..2dc4e57b10 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/bencode/BDecoder.java +++ b/apps/i2psnark/java/src/org/klomp/snark/bencode/BDecoder.java @@ -60,7 +60,7 @@ public class BDecoder private int indicator = 0; // Used for ugly hack to get SHA hash over the metainfo info map - private final String special_map = "info"; + private static final String special_map = "info"; private boolean in_special_map = false; /** creation deferred until we encounter the special map, to make processing of announce replies more efficient */ private MessageDigest sha_digest; diff --git a/apps/i2psnark/java/src/org/klomp/snark/web/BasicServlet.java b/apps/i2psnark/java/src/org/klomp/snark/web/BasicServlet.java index 4c98a4c80f..c4112ac27d 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/web/BasicServlet.java +++ b/apps/i2psnark/java/src/org/klomp/snark/web/BasicServlet.java @@ -36,6 +36,7 @@ import javax.servlet.http.HttpServletResponse; import net.i2p.I2PAppContext; import net.i2p.data.ByteArray; +import net.i2p.data.DataHelper; import net.i2p.util.ByteCache; import net.i2p.util.Log; import net.i2p.util.SystemVersion; @@ -469,7 +470,7 @@ class BasicServlet extends HttpServlet { String cpath = getServletContext().getContextPath(); // this won't work if we aren't at top level - String cname = cpath == "" ? "i2psnark" : cpath.substring(1).replace("/", "_"); + String cname = "".equals(cpath) ? "i2psnark" : cpath.substring(1).replace("/", "_"); return (new File(_context.getBaseDir(), "webapps/" + cname + ".war")).lastModified(); } @@ -578,7 +579,7 @@ class BasicServlet extends HttpServlet byte[] buf = ba.getData(); try { if (skip > 0) - in.skip(skip); + DataHelper.skip(in, skip); int read = 0; long tot = 0; boolean done = false; diff --git a/apps/i2psnark/java/src/org/klomp/snark/web/FetchAndAdd.java b/apps/i2psnark/java/src/org/klomp/snark/web/FetchAndAdd.java index e9c84f4d7f..cf2a55c6f4 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/web/FetchAndAdd.java +++ b/apps/i2psnark/java/src/org/klomp/snark/web/FetchAndAdd.java @@ -198,7 +198,7 @@ public class FetchAndAdd extends Snark implements EepGet.StatusListener, Runnabl //_total = -1; _transferred = 0; _failCause = null; - _started = _ctx.clock().now(); + _started = _util.getContext().clock().now(); _isRunning = true; _active = false; _thread = new I2PAppThread(this, "Torrent File EepGet", true); diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/irc/IRCFilter.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/irc/IRCFilter.java index 58f00b9759..f4c7e1ec35 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/irc/IRCFilter.java +++ b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/irc/IRCFilter.java @@ -71,7 +71,7 @@ abstract class IRCFilter { // Allow numerical responses try { - Integer.valueOf(command); + Integer.parseInt(command); return s; } catch(NumberFormatException nfe){} diff --git a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java index 3263e2fa8d..71f1cf63e0 100644 --- a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java +++ b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHelper.java @@ -238,8 +238,8 @@ public class ConfigNetHelper extends HelperBase { return kbytesToBits(getShareBandwidth()); } private String kbytesToBits(int kbytes) { - return DataHelper.formatSize(kbytes * 8 * 1024) + ' ' + _("bits per second") + - ' ' + _("or {0} bytes per month maximum", DataHelper.formatSize(kbytes * 1024l * 60 * 60 * 24 * 31)); + return DataHelper.formatSize(kbytes * (8 * 1024L)) + ' ' + _("bits per second") + + ' ' + _("or {0} bytes per month maximum", DataHelper.formatSize(kbytes * (1024L * 60 * 60 * 24 * 31))); } public String getInboundBurstRate() { return "" + _context.bandwidthLimiter().getInboundBurstKBytesPerSecond(); diff --git a/apps/susimail/src/src/org/mortbay/servlet/MultiPartRequest.java b/apps/susimail/src/src/org/mortbay/servlet/MultiPartRequest.java index ee45d7685d..c919180059 100644 --- a/apps/susimail/src/src/org/mortbay/servlet/MultiPartRequest.java +++ b/apps/susimail/src/src/org/mortbay/servlet/MultiPartRequest.java @@ -435,11 +435,11 @@ public class MultiPartRequest } /* ------------------------------------------------------------ */ - private class Part + private static class Part { - String _name=null; - String _filename=null; + String _name; + String _filename; Hashtable _headers= new Hashtable(10); - byte[] _data=null; + byte[] _data; } }; diff --git a/core/java/src/freenet/support/CPUInformation/CPUIDCPUInfo.java b/core/java/src/freenet/support/CPUInformation/CPUIDCPUInfo.java index b68e5959fd..07bf42f343 100644 --- a/core/java/src/freenet/support/CPUInformation/CPUIDCPUInfo.java +++ b/core/java/src/freenet/support/CPUInformation/CPUIDCPUInfo.java @@ -14,30 +14,30 @@ abstract class CPUIDCPUInfo } public boolean hasMMX() { - return (CPUID.getEDXCPUFlags() & 0x800000) >0; //EDX Bit 23 + return (CPUID.getEDXCPUFlags() & 0x800000) != 0; //EDX Bit 23 } public boolean hasSSE(){ - return (CPUID.getEDXCPUFlags() & 0x2000000) >0; //EDX Bit 25 + return (CPUID.getEDXCPUFlags() & 0x2000000) != 0; //EDX Bit 25 } public boolean hasSSE2() { - return (CPUID.getEDXCPUFlags() & 0x4000000) >0; //EDX Bit 26 + return (CPUID.getEDXCPUFlags() & 0x4000000) != 0; //EDX Bit 26 } public boolean hasSSE3() { - return (CPUID.getEDXCPUFlags() & 0x1) >0; //ECX Bit 0 + return (CPUID.getEDXCPUFlags() & 0x1) != 0; //ECX Bit 0 } public boolean hasSSE41() { - return (CPUID.getEDXCPUFlags() & 0x80000) >0; //ECX Bit 19 + return (CPUID.getEDXCPUFlags() & 0x80000) != 0; //ECX Bit 19 } public boolean hasSSE42() { - return (CPUID.getEDXCPUFlags() & 0x100000) >0; //ECX Bit 20 + return (CPUID.getEDXCPUFlags() & 0x100000) != 0; //ECX Bit 20 } public boolean hasSSE4A() { - return (CPUID.getExtendedECXCPUFlags() & 0x40) >0; //Extended ECX Bit 6 + return (CPUID.getExtendedECXCPUFlags() & 0x40) != 0; //Extended ECX Bit 6 } public abstract boolean hasX64(); diff --git a/core/java/src/net/i2p/data/DataHelper.java b/core/java/src/net/i2p/data/DataHelper.java index 93a476cbb4..36b36a068e 100644 --- a/core/java/src/net/i2p/data/DataHelper.java +++ b/core/java/src/net/i2p/data/DataHelper.java @@ -1140,6 +1140,46 @@ public class DataHelper { return c; } + /** + * This is different than InputStream.skip(), in that it + * does repeated reads until the full amount is skipped. + * To fix findbugs issues with skip(). + * + * Guaranteed to skip exactly n bytes or throw an IOE. + * + * http://stackoverflow.com/questions/14057720/robust-skipping-of-data-in-a-java-io-inputstream-and-its-subtypes + * http://stackoverflow.com/questions/11511093/java-inputstream-skip-return-value-near-end-of-file + * + * @since 0.9.9 + */ + public static void skip(InputStream in, long n) throws IOException { + if (n < 0) + throw new IllegalArgumentException(); + if (n == 0) + return; + long read = 0; + long nm1 = n - 1; + if (nm1 > 0) { + // skip all but the last byte + do { + long c = in.skip(nm1 - read); + if (c < 0) + throw new EOFException("EOF while skipping " + n + ", read only " + read); + if (c == 0) { + // see second SO link above + if (in.read() == -1) + throw new EOFException("EOF while skipping " + n + ", read only " + read); + read++; + } else { + read += c; + } + } while (read < nm1); + } + // read the last byte to check for EOF + if (in.read() == -1) + throw new EOFException("EOF while skipping " + n + ", read only " + read); + } + /** * This is different than InputStream.read(target), in that it * does repeated reads until the full data is received. diff --git a/core/java/src/net/i2p/util/SipHashInline.java b/core/java/src/net/i2p/util/SipHashInline.java index 3bd190a9a8..d49273e190 100644 --- a/core/java/src/net/i2p/util/SipHashInline.java +++ b/core/java/src/net/i2p/util/SipHashInline.java @@ -53,14 +53,14 @@ abstract class SipHashInline { // processing 8 bytes blocks in data while (i < last) { // pack a block to long, as LE 8 bytes - m = (long) data[i++] | - (long) data[i++] << 8 | - (long) data[i++] << 16 | - (long) data[i++] << 24 | - (long) data[i++] << 32 | - (long) data[i++] << 40 | - (long) data[i++] << 48 | - (long) data[i++] << 56 ; + m = ((((long) data[i++]) & 0xff) ) | + ((((long) data[i++]) & 0xff) << 8) | + ((((long) data[i++]) & 0xff) << 16) | + ((((long) data[i++]) & 0xff) << 24) | + ((((long) data[i++]) & 0xff) << 32) | + ((((long) data[i++]) & 0xff) << 40) | + ((((long) data[i++]) & 0xff) << 48) | + ((((long) data[i++]) & 0xff) << 56); // MSGROUND { v3 ^= m; @@ -132,7 +132,7 @@ abstract class SipHashInline { // packing the last block to long, as LE 0-7 bytes + the length in the top byte m = 0; for (i = off + len - 1; i >= last; --i) { - m <<= 8; m |= (long) data[i]; + m <<= 8; m |= (long) (data[i] & 0xff); } m |= (long) len << 56; // MSGROUND { diff --git a/core/java/src/net/i2p/util/ZipFileComment.java b/core/java/src/net/i2p/util/ZipFileComment.java index 9a57787684..a20407fe84 100644 --- a/core/java/src/net/i2p/util/ZipFileComment.java +++ b/core/java/src/net/i2p/util/ZipFileComment.java @@ -69,12 +69,12 @@ public abstract class ZipFileComment { try { in = new FileInputStream(file); if (skip > 0) - in.skip(skip); + DataHelper.skip(in, skip); byte[] hdr = new byte[HEADER_LEN]; DataHelper.read(in, hdr); if (!DataHelper.eq(hdr, magicStart)) throw new ZipException("Not a zip file: " + file); - in.skip(fileLen - (skip + HEADER_LEN + buffer.length)); + DataHelper.skip(in, fileLen - (skip + HEADER_LEN + buffer.length)); DataHelper.read(in, buffer); return getComment(buffer); } finally { diff --git a/core/java/src/net/metanotion/io/block/index/BSkipSpan.java b/core/java/src/net/metanotion/io/block/index/BSkipSpan.java index 5e4481c8c7..a934d2b1dd 100644 --- a/core/java/src/net/metanotion/io/block/index/BSkipSpan.java +++ b/core/java/src/net/metanotion/io/block/index/BSkipSpan.java @@ -390,6 +390,7 @@ public class BSkipSpan extends SkipSpan { this.prev = null; BSkipSpan bss = this; + // findbugs ok (set in load() above) int np = nextPage; while(np != 0) { BSkipSpan temp = bsl.spanHash.get(Integer.valueOf(np)); diff --git a/router/java/src/net/i2p/router/Blocklist.java b/router/java/src/net/i2p/router/Blocklist.java index 7385d87384..11a4cc5c42 100644 --- a/router/java/src/net/i2p/router/Blocklist.java +++ b/router/java/src/net/i2p/router/Blocklist.java @@ -525,11 +525,15 @@ public class Blocklist { byte[] pib = pa.getIP(); if (pib == null) continue; // O(n**2) + boolean dup = false; for (int i = 0; i < rv.size(); i++) { - // findbugs triggered on this, looks like unfinished work - //if (DataHelper.eq(rv.get(i), pib)) continue; + if (DataHelper.eq(rv.get(i), pib)) { + dup = true; + break; + } } - rv.add(pib); + if (!dup) + rv.add(pib); } return rv; } diff --git a/router/java/src/net/i2p/router/transport/GeoIPv6.java b/router/java/src/net/i2p/router/transport/GeoIPv6.java index c9309ee8a7..0fdf783107 100644 --- a/router/java/src/net/i2p/router/transport/GeoIPv6.java +++ b/router/java/src/net/i2p/router/transport/GeoIPv6.java @@ -83,7 +83,7 @@ class GeoIPv6 { if (!DataHelper.eq(magic, DataHelper.getASCII(MAGIC))) throw new IOException("Not a IPv6 geoip data file"); // skip timestamp and comments - in.skip(HEADER_LEN - MAGIC.length()); + DataHelper.skip(in, HEADER_LEN - MAGIC.length()); byte[] buf = new byte[18]; while (DataHelper.read(in, buf) == 18 && idx < search.length) { long ip1 = readLong(buf, 0); diff --git a/router/java/src/net/i2p/router/transport/UPnP.java b/router/java/src/net/i2p/router/transport/UPnP.java index 32678f750d..fd17053bec 100644 --- a/router/java/src/net/i2p/router/transport/UPnP.java +++ b/router/java/src/net/i2p/router/transport/UPnP.java @@ -415,7 +415,11 @@ class UPnP extends ControlPoint implements DeviceChangeListener, EventListener { if(getIP == null || !getIP.postControlAction()) return -1; - return Integer.parseInt(getIP.getOutputArgumentList().getArgument("NewUpstreamMaxBitRate").getValue()); + try { + return Integer.parseInt(getIP.getOutputArgumentList().getArgument("NewUpstreamMaxBitRate").getValue()); + } catch (NumberFormatException nfe) { + return -1; + } } /** @@ -429,7 +433,11 @@ class UPnP extends ControlPoint implements DeviceChangeListener, EventListener { if(getIP == null || !getIP.postControlAction()) return -1; - return Integer.parseInt(getIP.getOutputArgumentList().getArgument("NewDownstreamMaxBitRate").getValue()); + try { + return Integer.parseInt(getIP.getOutputArgumentList().getArgument("NewDownstreamMaxBitRate").getValue()); + } catch (NumberFormatException nfe) { + return -1; + } } /** debug only */ -- GitLab