Skip to content

Commit

Permalink
FindBugs sweep.
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrian Cole and Jesse Wilson committed Feb 5, 2014
1 parent b149fec commit fdee6f1
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,6 @@ public void setNpnProtocols(List<Protocol> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Header> headerBlock = headerBlockReader.readNameValueBlock(length - 10);

boolean inFinished = (flags & FLAG_FIN) != 0;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -439,7 +439,7 @@ private void writeNameValueBlockToBuffer(List<Header> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Header> sentHeaders) throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
new HpackDraft05.Writer(new DataOutputStream(out)).writeHeaders(sentHeaders);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,8 +82,8 @@ public class ConnectionPool {
private final ExecutorService executorService = new ThreadPoolExecutor(0, 1,
60L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),
Util.threadFactory("OkHttp ConnectionPool", true));
private final Callable<Void> connectionsCleanupCallable = new Callable<Void>() {
@Override public Void call() throws Exception {
private final Runnable connectionsCleanupRunnable = new Runnable() {
@Override public void run() {
List<Connection> expiredConnections = new ArrayList<Connection>(MAX_CONNECTIONS_TO_CLEANUP);
int idleConnectionCount = 0;
synchronized (ConnectionPool.this) {
Expand Down Expand Up @@ -113,7 +112,6 @@ public class ConnectionPool {
for (Connection expiredConnection : expiredConnections) {
Util.closeQuietly(expiredConnection);
}
return null;
}
};

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -239,15 +237,15 @@ public void recycle(Connection connection) {
connection.resetIdleStartTime();
}

executorService.submit(connectionsCleanupCallable);
executorService.execute(connectionsCleanupRunnable);
}

/**
* Shares the SPDY connection with the pool. Callers to this method may
* 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;
Expand Down
3 changes: 0 additions & 3 deletions okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,6 @@ public OkHttpClient setProtocols(List<Protocol> 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;
}
Expand Down
28 changes: 15 additions & 13 deletions okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Runnable>(), Util.threadFactory("OkHttp DiskLruCache", true));
private final Callable<Void> cleanupCallable = new Callable<Void>() {
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;
}
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -488,7 +490,7 @@ public long getMaxSize() {
*/
public synchronized void setMaxSize(long maxSize) {
this.maxSize = maxSize;
executorService.submit(cleanupCallable);
executorService.execute(cleanupRunnable);
}

/**
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class OkHttpContributors {
new TypeToken<List<Contributor>>() {
};

class Contributor {
static class Contributor {
String login;
int contributions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit fdee6f1

Please sign in to comment.