From fdee6f13a4c5bf5bd24f2c237c2996aea01cc5ff Mon Sep 17 00:00:00 2001 From: Adrian Cole and Jesse Wilson Date: Wed, 5 Feb 2014 11:49:21 +0100 Subject: [PATCH] FindBugs sweep. --- .../okhttp/benchmarks/UrlConnection.java | 2 +- .../okhttp/internal/spdy/SpdyServer.java | 15 ++++++---- .../okhttp/mockwebserver/MockWebServer.java | 3 -- .../com/squareup/okhttp/internal/Util.java | 15 ---------- .../okhttp/internal/bytes/ByteString.java | 2 +- .../okhttp/internal/spdy/Http20Draft09.java | 6 ++-- .../squareup/okhttp/internal/spdy/Spdy3.java | 6 ++-- .../okhttp/internal/spdy/SpdyStream.java | 2 +- .../internal/spdy/Http20Draft09Test.java | 24 ++++++++++++++++ .../internal/spdy/SpdyConnectionTest.java | 2 +- .../com/squareup/okhttp/ConnectionPool.java | 12 ++++---- .../com/squareup/okhttp/OkHttpClient.java | 3 -- .../okhttp/internal/DiskLruCache.java | 28 ++++++++++--------- .../okhttp/sample/OkHttpContributors.java | 2 +- .../squareup/okhttp/sample/SampleServer.java | 9 ++++-- 15 files changed, 71 insertions(+), 60 deletions(-) diff --git a/benchmarks/src/main/java/com/squareup/okhttp/benchmarks/UrlConnection.java b/benchmarks/src/main/java/com/squareup/okhttp/benchmarks/UrlConnection.java index 53d2a4b9e9f3..79abb69eeb85 100644 --- a/benchmarks/src/main/java/com/squareup/okhttp/benchmarks/UrlConnection.java +++ b/benchmarks/src/main/java/com/squareup/okhttp/benchmarks/UrlConnection.java @@ -50,7 +50,7 @@ class UrlConnection extends SynchronousHttpClient { return new UrlConnectionRequest(url); } - class UrlConnectionRequest implements Runnable { + static class UrlConnectionRequest implements Runnable { private final URL url; public UrlConnectionRequest(URL url) { diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java b/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java index e135ef7ff6b5..79dc4bbb411f 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java @@ -18,6 +18,7 @@ import com.squareup.okhttp.Protocol; import com.squareup.okhttp.internal.SslContextBuilder; +import com.squareup.okhttp.internal.Util; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -130,17 +131,21 @@ private void serveDirectory(SpdyStream stream, String[] files) throws IOExceptio } private void serveFile(SpdyStream stream, File file) throws IOException { - InputStream in = new FileInputStream(file); byte[] buffer = new byte[8192]; stream.reply( headerEntries(":status", "200", ":version", "HTTP/1.1", "content-type", contentType(file)), true); + InputStream in = new FileInputStream(file); OutputStream out = stream.getOutputStream(); - int count; - while ((count = in.read(buffer)) != -1) { - out.write(buffer, 0, count); + try { + int count; + while ((count = in.read(buffer)) != -1) { + out.write(buffer, 0, count); + } + } finally { + Util.closeQuietly(in); + Util.closeQuietly(out); } - out.close(); } private String contentType(File file) { diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java index 71c7c860fa9a..3fdaf676e8b5 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java @@ -181,9 +181,6 @@ public void setNpnProtocols(List protocols) { if (protocols.contains(null)) { throw new IllegalArgumentException("protocols must not contain null"); } - if (protocols.contains(ByteString.EMPTY)) { - throw new IllegalArgumentException("protocols contains an empty string"); - } this.npnProtocols = Util.immutableList(protocols); } diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java index fbbf46fd0d3d..e609db9528f9 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java @@ -32,7 +32,6 @@ import java.net.SocketTimeoutException; import java.net.URI; import java.net.URL; -import java.nio.ByteOrder; import java.nio.charset.Charset; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -88,20 +87,6 @@ public static void checkOffsetAndCount(long arrayLength, long offset, long count } } - public static void pokeInt(byte[] dst, int offset, int value, ByteOrder order) { - if (order == ByteOrder.BIG_ENDIAN) { - dst[offset++] = (byte) ((value >> 24) & 0xff); - dst[offset++] = (byte) ((value >> 16) & 0xff); - dst[offset++] = (byte) ((value >> 8) & 0xff); - dst[offset] = (byte) ((value >> 0) & 0xff); - } else { - dst[offset++] = (byte) ((value >> 0) & 0xff); - dst[offset++] = (byte) ((value >> 8) & 0xff); - dst[offset++] = (byte) ((value >> 16) & 0xff); - dst[offset] = (byte) ((value >> 24) & 0xff); - } - } - /** Returns true if two possibly-null objects are equal. */ public static boolean equal(Object a, Object b) { return a == b || (a != null && a.equals(b)); diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/ByteString.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/ByteString.java index 64a1183a5566..9a6a799edcd8 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/ByteString.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/bytes/ByteString.java @@ -71,7 +71,7 @@ public boolean equalsAscii(String ascii) { if (ascii == null || data.length != ascii.length()) { return false; } - if (ascii == this.utf8) { + if (ascii == this.utf8) { // not using String.equals to avoid looping twice. return true; } for (int i = 0; i < data.length; i++) { diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java index 307a4a52b6f6..0bc07a87bfe1 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java @@ -460,11 +460,9 @@ public synchronized void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[ out.close(); } - private void frameHeader(int length, byte type, byte flags, int streamId) - throws IOException { + void frameHeader(int length, byte type, byte flags, int streamId) throws IOException { if (length > 16383) throw illegalArgument("FRAME_SIZE_ERROR length > 16383: %s", length); - if ((streamId & 0x80000000) == 1) throw illegalArgument("(streamId & 0x80000000) == 1: %s", - streamId); + if ((streamId & 0x80000000) != 0) throw illegalArgument("reserved bit set: %s", streamId); out.writeInt((length & 0x3fff) << 16 | (type & 0xff) << 8 | (flags & 0xff)); out.writeInt(streamId & 0x7fffffff); } diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java index 75060445aea0..75afc37555b3 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java @@ -200,7 +200,7 @@ private void readSynStream(Handler handler, int flags, int length) throws IOExce int streamId = w1 & 0x7fffffff; int associatedStreamId = w2 & 0x7fffffff; int priority = (s3 & 0xe000) >>> 13; - int slot = s3 & 0xff; + // int slot = s3 & 0xff; List
headerBlock = headerBlockReader.readNameValueBlock(length - 10); boolean inFinished = (flags & FLAG_FIN) != 0; @@ -248,7 +248,7 @@ private void readWindowUpdate(Handler handler, int flags, int length) throws IOE private void readPing(Handler handler, int flags, int length) throws IOException { if (length != 4) throw ioException("TYPE_PING length: %d != 4", length); int id = in.readInt(); - boolean ack = client == ((id % 2) == 1); + boolean ack = client == ((id & 1) == 1); handler.ping(ack, id, 0); } @@ -439,7 +439,7 @@ private void writeNameValueBlockToBuffer(List
headerBlock) throws IOExce @Override public synchronized void ping(boolean reply, int payload1, int payload2) throws IOException { - boolean payloadIsReply = client != ((payload1 % 2) == 1); + boolean payloadIsReply = client != ((payload1 & 1) == 1); if (reply != payloadIsReply) throw new IllegalArgumentException("payload != reply"); int type = TYPE_PING; int flags = 0; diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java index 87ce18a0ba28..e36c8ebadf0a 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java @@ -108,7 +108,7 @@ public synchronized boolean isOpen() { /** Returns true if this stream was created by this peer. */ public boolean isLocallyInitiated() { - boolean streamIsClient = (id % 2 == 1); + boolean streamIsClient = ((id & 1) == 1); return connection.client == streamIsClient; } diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java index 4f94d27c4941..80237341302f 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java @@ -477,6 +477,30 @@ private Http20Draft09.Reader newReader(ByteArrayOutputStream out) { return new Http20Draft09.Reader(new ByteArrayInputStream(out.toByteArray()), 4096, false); } + @Test public void frameSizeError() throws IOException { + Http20Draft09.Writer writer = new Http20Draft09.Writer(new ByteArrayOutputStream(), true); + + try { + writer.frameHeader(16384, Http20Draft09.TYPE_DATA, Http20Draft09.FLAG_NONE, 0); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("FRAME_SIZE_ERROR length > 16383: 16384", e.getMessage()); + } + } + + @Test public void streamIdHasReservedBit() throws IOException { + Http20Draft09.Writer writer = new Http20Draft09.Writer(new ByteArrayOutputStream(), true); + + try { + int streamId = 3; + streamId |= 1L << 31; // set reserved bit + writer.frameHeader(16383, Http20Draft09.TYPE_DATA, Http20Draft09.FLAG_NONE, streamId); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("reserved bit set: -2147483645", e.getMessage()); + } + } + private byte[] literalHeaders(List
sentHeaders) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); new HpackDraft05.Writer(new DataOutputStream(out)).writeHeaders(sentHeaders); diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java index c5634915e089..acf2f5c44d3d 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java @@ -1376,7 +1376,7 @@ private void clientSendsEmptyDataServerDoesntSendWindowUpdate(Variant variant) new Header(Header.TARGET_AUTHORITY, "squareup.com"), new Header(Header.TARGET_PATH, "/cached") )); - peer.sendFrame().synReply(true, 1, Arrays.asList( + peer.sendFrame().synReply(true, 2, Arrays.asList( new Header(Header.RESPONSE_STATUS, "200") )); peer.acceptFrame(); // RST_STREAM diff --git a/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java b/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java index 5eb4874aa41c..240cf83bebc1 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java +++ b/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java @@ -23,7 +23,6 @@ import java.util.LinkedList; import java.util.List; import java.util.ListIterator; -import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; @@ -83,8 +82,8 @@ public class ConnectionPool { private final ExecutorService executorService = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue(), Util.threadFactory("OkHttp ConnectionPool", true)); - private final Callable connectionsCleanupCallable = new Callable() { - @Override public Void call() throws Exception { + private final Runnable connectionsCleanupRunnable = new Runnable() { + @Override public void run() { List expiredConnections = new ArrayList(MAX_CONNECTIONS_TO_CLEANUP); int idleConnectionCount = 0; synchronized (ConnectionPool.this) { @@ -113,7 +112,6 @@ public class ConnectionPool { for (Connection expiredConnection : expiredConnections) { Util.closeQuietly(expiredConnection); } - return null; } }; @@ -205,7 +203,7 @@ public synchronized Connection get(Address address) { connections.addFirst(foundConnection); // Add it back after iteration. } - executorService.submit(connectionsCleanupCallable); + executorService.execute(connectionsCleanupRunnable); return foundConnection; } @@ -239,7 +237,7 @@ public void recycle(Connection connection) { connection.resetIdleStartTime(); } - executorService.submit(connectionsCleanupCallable); + executorService.execute(connectionsCleanupRunnable); } /** @@ -247,7 +245,7 @@ public void recycle(Connection connection) { * continue to use {@code connection}. */ public void maybeShare(Connection connection) { - executorService.submit(connectionsCleanupCallable); + executorService.execute(connectionsCleanupRunnable); if (!connection.isSpdy()) { // Only SPDY connections are sharable. return; diff --git a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java index 951f59a416eb..68e1cfadb9bc 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java +++ b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java @@ -349,9 +349,6 @@ public OkHttpClient setProtocols(List protocols) { if (protocols.contains(null)) { throw new IllegalArgumentException("protocols must not contain null"); } - if (protocols.contains(ByteString.EMPTY)) { - throw new IllegalArgumentException("protocols contains an empty string"); - } this.protocols = Util.immutableList(protocols); return this; } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java index 19e4cee61f46..1dbaa88c0e8c 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java @@ -33,7 +33,6 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; -import java.util.concurrent.Callable; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -160,19 +159,22 @@ public final class DiskLruCache implements Closeable { /** This cache uses a single background thread to evict entries. */ final ThreadPoolExecutor executorService = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue(), Util.threadFactory("OkHttp DiskLruCache", true)); - private final Callable cleanupCallable = new Callable() { - public Void call() throws Exception { + private final Runnable cleanupRunnable = new Runnable() { + public void run() { synchronized (DiskLruCache.this) { if (journalWriter == null) { - return null; // Closed. + return; // Closed. } - trimToSize(); - if (journalRebuildRequired()) { - rebuildJournal(); - redundantOpCount = 0; + try { + trimToSize(); + if (journalRebuildRequired()) { + rebuildJournal(); + redundantOpCount = 0; + } + } catch (IOException e) { + throw new RuntimeException(e); } } - return null; } }; @@ -431,7 +433,7 @@ public synchronized Snapshot get(String key) throws IOException { redundantOpCount++; journalWriter.append(READ + ' ' + key + '\n'); if (journalRebuildRequired()) { - executorService.submit(cleanupCallable); + executorService.execute(cleanupRunnable); } return new Snapshot(key, entry.sequenceNumber, ins, entry.lengths); @@ -488,7 +490,7 @@ public long getMaxSize() { */ public synchronized void setMaxSize(long maxSize) { this.maxSize = maxSize; - executorService.submit(cleanupCallable); + executorService.execute(cleanupRunnable); } /** @@ -551,7 +553,7 @@ private synchronized void completeEdit(Editor editor, boolean success) throws IO journalWriter.flush(); if (size > maxSize || journalRebuildRequired()) { - executorService.submit(cleanupCallable); + executorService.execute(cleanupRunnable); } } @@ -593,7 +595,7 @@ public synchronized boolean remove(String key) throws IOException { lruEntries.remove(key); if (journalRebuildRequired()) { - executorService.submit(cleanupCallable); + executorService.execute(cleanupRunnable); } return true; diff --git a/samples/simple-client/src/main/java/com/squareup/okhttp/sample/OkHttpContributors.java b/samples/simple-client/src/main/java/com/squareup/okhttp/sample/OkHttpContributors.java index 8969f472364b..c6424e2dece4 100644 --- a/samples/simple-client/src/main/java/com/squareup/okhttp/sample/OkHttpContributors.java +++ b/samples/simple-client/src/main/java/com/squareup/okhttp/sample/OkHttpContributors.java @@ -18,7 +18,7 @@ public class OkHttpContributors { new TypeToken>() { }; - class Contributor { + static class Contributor { String login; int contributions; } diff --git a/samples/static-server/src/main/java/com/squareup/okhttp/sample/SampleServer.java b/samples/static-server/src/main/java/com/squareup/okhttp/sample/SampleServer.java index 274bf9dd4885..cb0e24e37aaa 100644 --- a/samples/static-server/src/main/java/com/squareup/okhttp/sample/SampleServer.java +++ b/samples/static-server/src/main/java/com/squareup/okhttp/sample/SampleServer.java @@ -9,6 +9,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.InputStream; import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.SecureRandom; @@ -116,8 +117,12 @@ public static void main(String[] args) throws Exception { private static SSLContext sslContext(String keystoreFile, String password) throws GeneralSecurityException, IOException { KeyStore keystore = KeyStore.getInstance(KeyStore.getDefaultType()); - keystore.load(new FileInputStream(keystoreFile), password.toCharArray()); - + InputStream in = new FileInputStream(keystoreFile); + try { + keystore.load(in, password.toCharArray()); + } finally { + Util.closeQuietly(in); + } KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); keyManagerFactory.init(keystore, password.toCharArray());