Bug scrub

* Try to be better about unsynchronized accesses
* Don't expose references to arrays in our beans
* Make sure we don't dereference null pointers
* Update version string before calling aapt

14 files changed, 121 insertions, 80 deletions

diff --git a/src/com/trilead/ssh2/packets/PacketSessionPtyResize.java b/src/com/trilead/ssh2/packets/PacketSessionPtyResize.java
index 20d35cd..27b5f00 100644
--- a/src/com/trilead/ssh2/packets/PacketSessionPtyResize.java
+++ b/src/com/trilead/ssh2/packets/PacketSessionPtyResize.java
@@ -5,14 +5,12 @@ public class PacketSessionPtyResize {
 
 	public int recipientChannelID;
 	public boolean wantReply;
-	public String term;
 	public int width;
 	public int height;
 
 	public PacketSessionPtyResize(int recipientChannelID, boolean wantReply, int width, int height) {
 		this.recipientChannelID = recipientChannelID;
 		this.wantReply = wantReply;
-		this.term = term;
 		this.width = width;
 		this.height = height;
 	}
diff --git a/src/org/connectbot/ConsoleActivity.java b/src/org/connectbot/ConsoleActivity.java
index a142537..4485f5b 100644
--- a/src/org/connectbot/ConsoleActivity.java
+++ b/src/org/connectbot/ConsoleActivity.java
@@ -433,7 +433,7 @@ public class ConsoleActivity extends Activity {
 					int moved = (int)(totalY / terminal.bridge.charHeight);
 					
 					// consume as scrollback only if towards right half of screen
-					if(e2.getX() > flip.getWidth() / 2) {
+					if (e2.getX() > flip.getWidth() / 2.0) {
 						if(moved != 0) {
 							int base = terminal.bridge.buffer.getWindowBase();
 							terminal.bridge.buffer.setWindowBase(base + moved);
@@ -520,6 +520,7 @@ public class ConsoleActivity extends Activity {
 						
 						clipboard.setText(buffer.toString());
 						Toast.makeText(ConsoleActivity.this, getString(R.string.console_copy_done, buffer.length()), Toast.LENGTH_LONG).show();
+						// fall through to clear state
 						
 					case MotionEvent.ACTION_CANCEL:
 						// make sure we clear any highlighted area
diff --git a/src/org/connectbot/GeneratePubkeyActivity.java b/src/org/connectbot/GeneratePubkeyActivity.java
index 79be141..22b9bc3 100644
--- a/src/org/connectbot/GeneratePubkeyActivity.java
+++ b/src/org/connectbot/GeneratePubkeyActivity.java
@@ -204,7 +204,7 @@ public class GeneratePubkeyActivity extends Activity implements OnEntropyGathere
 	}
 	
 	public void onEntropyGathered(byte[] entropy) {
-		this.entropy = entropy;
+		this.entropy = entropy.clone();
 		
 		Log.d(TAG, "entropy gathered; attemping to generate key...");
 		startKeyGen();
diff --git a/src/org/connectbot/HostListActivity.java b/src/org/connectbot/HostListActivity.java
index c911d02..1c6a577 100644
--- a/src/org/connectbot/HostListActivity.java
+++ b/src/org/connectbot/HostListActivity.java
@@ -473,11 +473,17 @@ public class HostListActivity extends ListActivity {
 
 			HostBean host = hosts.get(position);
 			if (host == null) {
-				Log.e("HostAdapter", "What the fuck, host bean is null!");
+				// Well, something bad happened. We can't continue.
+				Log.e("HostAdapter", "Host bean is null!");
+				
+				nickname.setText("Error during lookup");
+				caption.setText("see 'adb logcat' for more");
+				return view;
 			}
+			
 			nickname.setText(host.getNickname());
 			
-			switch(this.getConnectedState(host)) {
+			switch (this.getConnectedState(host)) {
 			case STATE_UNKNOWN:
 				icon.setImageState(new int[] { }, true);
 				break;
@@ -497,7 +503,7 @@ public class HostListActivity extends ListActivity {
 			else if (HostDatabase.COLOR_BLUE.equals(host.getColor()))
 				chosen = this.blue;
 			
-			if(chosen != null) {
+			if (chosen != null) {
 				// set color normally if not selected 
 				nickname.setTextColor(chosen);
 				caption.setTextColor(chosen);
@@ -510,7 +516,7 @@ public class HostListActivity extends ListActivity {
 			long now = System.currentTimeMillis() / 1000;
 			
 			String nice = "never";
-			if(host.getLastConnect() > 0) {
+			if (host.getLastConnect() > 0) {
 				int minutes = (int)((now - host.getLastConnect()) / 60);
 				nice = view.getContext().getString(R.string.bind_minutes, minutes);
 				if (minutes >= 60) {
diff --git a/src/org/connectbot/PubkeyListActivity.java b/src/org/connectbot/PubkeyListActivity.java
index 4a1b3de..aa02a16 100644
--- a/src/org/connectbot/PubkeyListActivity.java
+++ b/src/org/connectbot/PubkeyListActivity.java
@@ -331,6 +331,7 @@ public class PubkeyListActivity extends ListActivity implements EventListener {
 				String message = getResources().getString(R.string.pubkey_failed_add, pubkey.getNickname());
 				Log.e(TAG, message, e);
 				Toast.makeText(PubkeyListActivity.this, message, Toast.LENGTH_LONG);
+				return;
 			}
 
 			// convert key to trilead format
@@ -399,7 +400,7 @@ public class PubkeyListActivity extends ListActivity implements EventListener {
 			public boolean onMenuItemClick(MenuItem item) {
 				try {
 					PublicKey pk = PubkeyUtils.decodePublic(pubkey.getPublicKey(), pubkey.getType());
-					String openSSHPubkey = new String(PubkeyUtils.convertToOpenSSHFormat(pk, pubkey.getNickname()));
+					String openSSHPubkey = PubkeyUtils.convertToOpenSSHFormat(pk, pubkey.getNickname());
 
 					clipboard.setText(openSSHPubkey);
 				} catch (Exception e) {
@@ -420,7 +421,7 @@ public class PubkeyListActivity extends ListActivity implements EventListener {
 						data = new String(pubkey.getPrivateKey());
 					else {
 						PrivateKey pk = PubkeyUtils.decodePrivate(pubkey.getPrivateKey(), pubkey.getType());
-						data = new String(PubkeyUtils.exportPEM(pk, null));
+						data = PubkeyUtils.exportPEM(pk, null);
 					}
 					
 					clipboard.setText(data);
diff --git a/src/org/connectbot/bean/HostBean.java b/src/org/connectbot/bean/HostBean.java
index 1b937c9..f501aa5 100644
--- a/src/org/connectbot/bean/HostBean.java
+++ b/src/org/connectbot/bean/HostBean.java
@@ -97,10 +97,10 @@ public class HostBean extends AbstractBean {
 		return hostKeyAlgo;
 	}
 	public void setHostKey(byte[] hostKey) {
-		this.hostKey = hostKey;
+		this.hostKey = hostKey.clone();
 	}
 	public byte[] getHostKey() {
-		return hostKey;
+		return hostKey.clone();
 	}
 	public void setLastConnect(long lastConnect) {
 		this.lastConnect = lastConnect;
@@ -183,6 +183,7 @@ public class HostBean extends AbstractBean {
 		return values;
 	}
 	
+	@Override
 	public boolean equals(Object o) {
 		if (!(o instanceof HostBean))
 			return false;
@@ -197,4 +198,17 @@ public class HostBean extends AbstractBean {
 			
 		return false;
 	}
+
+	@Override
+	public int hashCode() {
+		int hash = 7;
+		
+		hash = 31 * hash + (null == nickname ? 0 : nickname.hashCode());
+		hash = 31 * hash + (null == username ? 0 : username.hashCode());
+		hash = 31 * hash + (null == hostname ? 0 : hostname.hashCode());
+		hash = 31 * hash + port;
+		
+		return hash;
+	}
+	
 }
diff --git a/src/org/connectbot/bean/PubkeyBean.java b/src/org/connectbot/bean/PubkeyBean.java
index 183eeea..ceeb6f3 100644
--- a/src/org/connectbot/bean/PubkeyBean.java
+++ b/src/org/connectbot/bean/PubkeyBean.java
@@ -73,19 +73,19 @@ public class PubkeyBean extends AbstractBean {
 	}
 
 	public void setPrivateKey(byte[] privateKey) {
-		this.privateKey = privateKey;
+		this.privateKey = privateKey.clone();
 	}
 
 	public byte[] getPrivateKey() {
-		return privateKey;
+		return privateKey.clone();
 	}
 
 	public void setPublicKey(byte[] publicKey) {
-		this.publicKey = publicKey;
+		this.publicKey = publicKey.clone();
 	}
 
 	public byte[] getPublicKey() {
-		return publicKey;
+		return publicKey.clone();
 	}
 
 	public void setEncrypted(boolean encrypted) {
diff --git a/src/org/connectbot/service/PromptHelper.java b/src/org/connectbot/service/PromptHelper.java
index a1dc891..9b783bb 100644
--- a/src/org/connectbot/service/PromptHelper.java
+++ b/src/org/connectbot/service/PromptHelper.java
@@ -24,7 +24,7 @@ public class PromptHelper {
 	/**
 	 * Register a user interface handler, if available.
 	 */
-	public void setHandler(Handler handler) {
+	public synchronized void setHandler(Handler handler) {
 		this.handler = handler;
 	}
 	
diff --git a/src/org/connectbot/service/TerminalBridge.java b/src/org/connectbot/service/TerminalBridge.java
index 57e836c..8ffd729 100644
--- a/src/org/connectbot/service/TerminalBridge.java
+++ b/src/org/connectbot/service/TerminalBridge.java
@@ -504,7 +504,7 @@ public class TerminalBridge implements VDUDisplay, OnKeyListener, InteractiveCal
 	 * Convenience method for writing a line into the underlying MUD buffer.
 	 * Should never be called once the session is established.
 	 */
-	protected synchronized void outputLine(String line) {
+	protected void outputLine(String line) {
 		if (session != null)
 			Log.e(TAG, "Session established, cannot use outputLine!", new IOException("outputLine call traceback"));
 		
@@ -978,8 +978,8 @@ public class TerminalBridge implements VDUDisplay, OnKeyListener, InteractiveCal
 		return false;
 	}
 	
-	public void tryKeyVibrate() {
-		if(bumpyArrows && vibrator != null)
+	public synchronized void tryKeyVibrate() {
+		if (bumpyArrows && vibrator != null)
 			vibrator.vibrate(VIBRATE_DURATION);
 	}
 
@@ -1018,10 +1018,8 @@ public class TerminalBridge implements VDUDisplay, OnKeyListener, InteractiveCal
 		int width = parent.getWidth();
 		int height = parent.getHeight();
 		
-		this.bumpyArrows = manager.prefs.getBoolean(manager.res.getString(R.string.pref_bumpyarrows), true);
-		if(parent != null) {
-			this.vibrator = (Vibrator) parent.getContext().getSystemService(Context.VIBRATOR_SERVICE);
-		}
+		bumpyArrows = manager.prefs.getBoolean(manager.res.getString(R.string.pref_bumpyarrows), true);
+		vibrator = (Vibrator) parent.getContext().getSystemService(Context.VIBRATOR_SERVICE);
 
 		if (!forcedSize) {
 			// recalculate buffer size
@@ -1212,7 +1210,7 @@ public class TerminalBridge implements VDUDisplay, OnKeyListener, InteractiveCal
 	 * @param width
 	 * @param height
 	 */
-	public void resizeComputed(int cols, int rows, int width, int height) {
+	public synchronized void resizeComputed(int cols, int rows, int width, int height) {
 		float size = 8.0f;
 		float step = 8.0f;
 		float limit = 0.125f;
diff --git a/src/org/connectbot/service/TerminalManager.java b/src/org/connectbot/service/TerminalManager.java
index bb9622c..2ab3d78 100644
--- a/src/org/connectbot/service/TerminalManager.java
+++ b/src/org/connectbot/service/TerminalManager.java
@@ -140,8 +140,10 @@ public class TerminalManager extends Service implements BridgeDisconnectedListen
 			pubkeydb = null;
 		}
 		
-		if (idleTimer != null)
-			idleTimer.cancel();
+		synchronized (this) {
+			if (idleTimer != null)
+				idleTimer.cancel();
+		}
 		
 		if (wifilock != null && wifilock.isHeld())
 			wifilock.release();
@@ -283,9 +285,9 @@ public class TerminalManager extends Service implements BridgeDisconnectedListen
 			synchronized (this) {
 				if (idleTimer == null)
 					idleTimer = new Timer(true);
+					
+				idleTimer.schedule(new IdleTask(), IDLE_TIMEOUT);
 			}
-
-			idleTimer.schedule(new IdleTask(), IDLE_TIMEOUT);
 		} else {
 			Log.d(TAG, "Stopping background service immediately");
 			stopSelf();
diff --git a/src/org/connectbot/util/EntropyView.java b/src/org/connectbot/util/EntropyView.java
index 0c236ef..ff6f5ce 100644
--- a/src/org/connectbot/util/EntropyView.java
+++ b/src/org/connectbot/util/EntropyView.java
@@ -89,17 +89,17 @@ public class EntropyView extends View {
         		splitText = prompt.indexOf(" ", prompt.length() / 2);
         	
         	c.drawText(prompt.substring(0, splitText),
-        			getWidth() / 2,
-        			getHeight() / 2 + (mPaint.ascent() + mPaint.descent()),
+        			getWidth() / 2.0f,
+        			getHeight() / 2.0f + (mPaint.ascent() + mPaint.descent()),
         			mPaint);
         	c.drawText(prompt.substring(splitText),
-        			getWidth() / 2,
-        			getHeight() / 2 - (mPaint.ascent() + mPaint.descent()),
+        			getWidth() / 2.0f,
+        			getHeight() / 2.0f - (mPaint.ascent() + mPaint.descent()),
         			mPaint);
         } else {
         	c.drawText(prompt,
-        			getWidth() / 2,
-        			getHeight() / 2 - (mFontMetrics.ascent + mFontMetrics.descent) / 2,
+        			getWidth() / 2.0f,
+        			getHeight() / 2.0f - (mFontMetrics.ascent + mFontMetrics.descent) / 2,
         			mPaint);
         }
     }
diff --git a/src/org/connectbot/util/HostDatabase.java b/src/org/connectbot/util/HostDatabase.java
index f0f9a66..550f88e 100644
--- a/src/org/connectbot/util/HostDatabase.java
+++ b/src/org/connectbot/util/HostDatabase.java
@@ -274,11 +274,13 @@ public class HostDatabase extends SQLiteOpenHelper {
 		
 		HostBean host = null;
 		
-		if (c != null && c.moveToFirst()) {
-			host = createHostBean(c);
+		if (c != null) {
+			if (c.moveToFirst())
+				host = createHostBean(c);
+			
+			c.close();
 		}
 		
-		c.close();
 		db.close();
 		
 		return host;
@@ -297,11 +299,13 @@ public class HostDatabase extends SQLiteOpenHelper {
 		
 		HostBean host = null;
 		
-		if (c != null && c.moveToFirst()) {
-			host = createHostBean(c);
+		if (c != null) {
+			if (c.moveToFirst())
+				host = createHostBean(c);
+			
+			c.close();
 		}
 		
-		c.close();
 		db.close();
 		
 		return host;
@@ -362,31 +366,30 @@ public class HostDatabase extends SQLiteOpenHelper {
 				null, null, null, null, null);
 
 		if (c != null) {
-		
 			int COL_HOSTNAME = c.getColumnIndexOrThrow(FIELD_HOST_HOSTNAME),
 				COL_PORT = c.getColumnIndexOrThrow(FIELD_HOST_PORT),
 				COL_HOSTKEYALGO = c.getColumnIndexOrThrow(FIELD_HOST_HOSTKEYALGO),
 				COL_HOSTKEY = c.getColumnIndexOrThrow(FIELD_HOST_HOSTKEY);
 			
-			while(c.moveToNext()) {
+			while (c.moveToNext()) {
 				String hostname = c.getString(COL_HOSTNAME),
 					hostkeyalgo = c.getString(COL_HOSTKEYALGO);
 				int port = c.getInt(COL_PORT);
 				byte[] hostkey = c.getBlob(COL_HOSTKEY);
 				
-				if(hostkeyalgo == null || hostkeyalgo.length() == 0) continue;
-				if(hostkey == null || hostkey.length == 0) continue;
+				if (hostkeyalgo == null || hostkeyalgo.length() == 0) continue;
+				if (hostkey == null || hostkey.length == 0) continue;
 				
 				try {
 					known.addHostkey(new String[] { String.format("%s:%d", hostname, port) }, hostkeyalgo, hostkey);
 				} catch(Exception e) {
 					Log.e(TAG, "Problem while adding a known host from database", e);
 				}
-				
 			}
+			
+			c.close();
 		}
 		
-		c.close();
 		db.close();
 		
 		return known;
diff --git a/src/org/connectbot/util/PubkeyDatabase.java b/src/org/connectbot/util/PubkeyDatabase.java
index bf043d5..9dd24f8 100644
--- a/src/org/connectbot/util/PubkeyDatabase.java
+++ b/src/org/connectbot/util/PubkeyDatabase.java
@@ -117,30 +117,33 @@ public class PubkeyDatabase extends SQLiteOpenHelper {
 		List<PubkeyBean> pubkeys = new LinkedList<PubkeyBean>();
 
 		Cursor c = db.query(TABLE_PUBKEYS, null, selection, selectionArgs, null, null, null);
-
-		final int COL_ID = c.getColumnIndexOrThrow("_id"),
-			COL_NICKNAME = c.getColumnIndexOrThrow(FIELD_PUBKEY_NICKNAME),
-			COL_TYPE = c.getColumnIndexOrThrow(FIELD_PUBKEY_TYPE),
-			COL_PRIVATE = c.getColumnIndexOrThrow(FIELD_PUBKEY_PRIVATE),
-			COL_PUBLIC = c.getColumnIndexOrThrow(FIELD_PUBKEY_PUBLIC),
-			COL_ENCRYPTED = c.getColumnIndexOrThrow(FIELD_PUBKEY_ENCRYPTED),
-			COL_STARTUP = c.getColumnIndexOrThrow(FIELD_PUBKEY_STARTUP);
-
-		while (c.moveToNext()) {
-			PubkeyBean pubkey = new PubkeyBean();
-			
-			pubkey.setId(c.getLong(COL_ID));
-			pubkey.setNickname(c.getString(COL_NICKNAME));
-			pubkey.setType(c.getString(COL_TYPE));
-			pubkey.setPrivateKey(c.getBlob(COL_PRIVATE));
-			pubkey.setPublicKey(c.getBlob(COL_PUBLIC));
-			pubkey.setEncrypted(c.getInt(COL_ENCRYPTED) > 0);
-			pubkey.setStartup(c.getInt(COL_STARTUP) > 0);
+		
+		if (c != null) {
+			final int COL_ID = c.getColumnIndexOrThrow("_id"),
+				COL_NICKNAME = c.getColumnIndexOrThrow(FIELD_PUBKEY_NICKNAME),
+				COL_TYPE = c.getColumnIndexOrThrow(FIELD_PUBKEY_TYPE),
+				COL_PRIVATE = c.getColumnIndexOrThrow(FIELD_PUBKEY_PRIVATE),
+				COL_PUBLIC = c.getColumnIndexOrThrow(FIELD_PUBKEY_PUBLIC),
+				COL_ENCRYPTED = c.getColumnIndexOrThrow(FIELD_PUBKEY_ENCRYPTED),
+				COL_STARTUP = c.getColumnIndexOrThrow(FIELD_PUBKEY_STARTUP);
+	
+			while (c.moveToNext()) {
+				PubkeyBean pubkey = new PubkeyBean();
+				
+				pubkey.setId(c.getLong(COL_ID));
+				pubkey.setNickname(c.getString(COL_NICKNAME));
+				pubkey.setType(c.getString(COL_TYPE));
+				pubkey.setPrivateKey(c.getBlob(COL_PRIVATE));
+				pubkey.setPublicKey(c.getBlob(COL_PUBLIC));
+				pubkey.setEncrypted(c.getInt(COL_ENCRYPTED) > 0);
+				pubkey.setStartup(c.getInt(COL_STARTUP) > 0);
+				
+				pubkeys.add(pubkey);
+			}
 			
-			pubkeys.add(pubkey);
+			c.close();
 		}
 		
-		c.close();
 		db.close();
 				
 		return pubkeys;
@@ -159,11 +162,13 @@ public class PubkeyDatabase extends SQLiteOpenHelper {
 		
 		PubkeyBean pubkey = null;
 		
-		if (c != null && c.moveToFirst()) {
-			pubkey = createPubkeyBean(c);
+		if (c != null) {
+			if (c.moveToFirst())
+				pubkey = createPubkeyBean(c);
+			
+			c.close();
 		}
 		
-		c.close();
 		db.close();
 		
 		return pubkey;
@@ -194,11 +199,16 @@ public class PubkeyDatabase extends SQLiteOpenHelper {
 		Cursor c = db.query(TABLE_PUBKEYS, new String[] { "_id", column },
 				null, null, null, null, "_id ASC");
 		
-		int COL = c.getColumnIndexOrThrow(column);
-		while(c.moveToNext()) {
-			list.add(c.getString(COL));
+		if (c != null) {
+			int COL = c.getColumnIndexOrThrow(column);
+			
+			while (c.moveToNext())
+				list.add(c.getString(COL));
+			
+			c.close();
 		}
-		c.close();
+		
+		db.close();
 		
 		return list;
 	}
@@ -211,13 +221,18 @@ public class PubkeyDatabase extends SQLiteOpenHelper {
 				FIELD_PUBKEY_NICKNAME }, "_id = ?",
 				new String[] { Long.toString(id) }, null, null, null);
 		
-		if (c != null && c.moveToFirst())
-			nickname = c.getString(c.getColumnIndexOrThrow(FIELD_PUBKEY_NICKNAME));
+		if (c != null) {
+			if (c.moveToFirst())
+				nickname = c.getString(c.getColumnIndexOrThrow(FIELD_PUBKEY_NICKNAME));
+			
+			c.close();
+		}
+		
+		db.close();
 		
-		c.close();
 		return nickname;
-
 	}
+	
 /*	
 	public void setOnStart(long id, boolean onStart) {
 		
@@ -271,7 +286,9 @@ public class PubkeyDatabase extends SQLiteOpenHelper {
 	public PubkeyBean savePubkey(PubkeyBean pubkey) {
 		SQLiteDatabase db = this.getWritableDatabase();
 		boolean success = false;
+		
 		ContentValues values = pubkey.getValues();
+		
 		if (pubkey.getId() > 0) {
 			values.remove("_id");
 			if (db.update(TABLE_PUBKEYS, values, "_id = ?", new String[] { String.valueOf(pubkey.getId()) }) > 0)
diff --git a/src/org/connectbot/util/UpdateHelper.java b/src/org/connectbot/util/UpdateHelper.java
index 24cdfd8..b5061e3 100644
--- a/src/org/connectbot/util/UpdateHelper.java
+++ b/src/org/connectbot/util/UpdateHelper.java
@@ -63,7 +63,7 @@ import android.util.Log;
  * 
  * @author jsharkey
  */
-public class UpdateHelper implements Runnable {
+public final class UpdateHelper implements Runnable {
 	
 	public final static String TAG = UpdateHelper.class.toString();
 	public final static String UPDATE_URL = "http://connectbot.org/version";
@@ -118,6 +118,7 @@ public class UpdateHelper implements Runnable {
 
 		if(shouldCheck) {
 			// spawn thread to check for update
+			// Note that this class should be marked final because a thread is started in the constructor.
 			new Thread(this).start();
 			
 			// update our last-checked time