Bug 742700 - Null URI crash in canceling J-PAKE. r=rnewman
authorChenxia Liu <liuche@mozilla.com>
Thu, 05 Apr 2012 19:25:34 -0700
changeset 94458 370a0a8ea1a20dfd6b2a8afeadbb5addc4a9937b
parent 94457 c527278f647a213cae9dac90b35ad1ddd2b82589
child 94459 3875882a5ff4b5edecfdd7ca9ec2d22c9bc0d63b
push idunknown
push userunknown
push dateunknown
reviewersrnewman
bugs742700
milestone14.0a1
Bug 742700 - Null URI crash in canceling J-PAKE. r=rnewman
mobile/android/base/sync/jpake/JPakeClient.java
mobile/android/base/sync/jpake/stage/DeleteChannel.java
--- a/mobile/android/base/sync/jpake/JPakeClient.java
+++ b/mobile/android/base/sync/jpake/JPakeClient.java
@@ -67,17 +67,17 @@ public class JPakeClient {
   // J-PAKE state.
   public boolean             paired                  = false;
   public boolean             finished                = false;
 
   // J-PAKE values.
   public int                 jpakePollInterval;
   public int                 jpakeMaxTries;
   public String              channel;
-  public String              channelUrl;
+  public volatile String     channelUrl;
 
   // J-PAKE session data.
   public KeyBundle           myKeyBundle;
   public JSONObject          jCreds;
 
   public ExtendedJSONObject  jOutgoing;
   public ExtendedJSONObject  jIncoming;
 
@@ -214,38 +214,43 @@ public class JPakeClient {
   /**
    * Run next stage of J-PAKE.
    */
   public void runNextStage() {
     if (finished || stages.size() == 0) {
       Logger.debug(LOG_TAG, "All stages complete.");
       return;
     }
-    JPakeStage nextStage = null; 
+    JPakeStage currentStage = null;
     try{
-      nextStage = stages.remove();
-      Logger.debug(LOG_TAG, "starting stage " + nextStage.toString());
-      nextStage.execute(this);
+      currentStage = stages.remove();
+      Logger.debug(LOG_TAG, "starting stage " + currentStage.toString());
+      currentStage.execute(this);
     } catch (Exception e) {
-      Logger.error(LOG_TAG, "Exception in stage " + nextStage, e);
+      Logger.error(LOG_TAG, "Exception in stage " + currentStage, e);
       abort("Stage exception.");
     }
   }
 
   /**
-   * Abort J-PAKE.
+   * Abort J-PAKE. This can propagate an error from the stages, or result from
+   * UI abort (onPause, user abort)
    *
    * @param reason
    *          Reason for abort.
    */
   public void abort(String reason) {
     finished = true;
+    // We do not need to clean up the channel in the following cases:
     if (Constants.JPAKE_ERROR_CHANNEL.equals(reason) ||
         Constants.JPAKE_ERROR_NETWORK.equals(reason) ||
-        Constants.JPAKE_ERROR_NODATA.equals(reason)) {
+        Constants.JPAKE_ERROR_NODATA.equals(reason) ||
+        channelUrl == null) {
+      // We may leak a channel if the activity aborts sync while requesting the channel.
+      // The server, however, will delete the channel anyways after a certain time has passed.
       displayAbort(reason);
     } else {
       // Delete channel, then call controller's displayAbort in callback.
       new DeleteChannel().execute(this, reason);
     }
   }
 
   public void displayAbort(String reason) {
--- a/mobile/android/base/sync/jpake/stage/DeleteChannel.java
+++ b/mobile/android/base/sync/jpake/stage/DeleteChannel.java
@@ -80,16 +80,16 @@ public class DeleteChannel {
 
       @Override
       public void handleTransportException(GeneralSecurityException e) {
         Logger.debug(LOG_TAG, "Encountered GeneralSecurityException, displaying abort anyway.");
         jClient.displayAbort(reason);
       }
     };
 
-    jClient.runOnThread(new Runnable() {
+    JPakeClient.runOnThread(new Runnable() {
       @Override
       public void run() {
         httpResource.delete();
       }
     });
   }
 }