From 4284aac0aa0d0bf4ce6f0edbc65a9837c06e1c8e Mon Sep 17 00:00:00 2001 From: Adithya Abraham Philip Date: Sat, 31 Oct 2015 00:20:00 +0530 Subject: changed KeyserverSyncAdapterService to work with START_REDELIVER_INTENT --- .../keychain/operations/ImportOperation.java | 4 +- .../service/KeyserverSyncAdapterService.java | 122 ++++++++++++++------- 2 files changed, 87 insertions(+), 39 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java index 8a4998b8f..19a05790f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java @@ -540,7 +540,9 @@ public class ImportOperation extends BaseOperation { // adding required information to mResultType // special case,no keys requested for import - if (mBadKeys == 0 && mNewKeys == 0 && mUpdatedKeys == 0) { + if (mBadKeys == 0 && mNewKeys == 0 && mUpdatedKeys == 0 + && (mResultType & ImportKeyResult.RESULT_CANCELLED) + != ImportKeyResult.RESULT_CANCELLED) { mResultType = ImportKeyResult.RESULT_FAIL_NOTHING; } else { if (mNewKeys > 0) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeyserverSyncAdapterService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeyserverSyncAdapterService.java index 8aebae7aa..122eb6cf4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeyserverSyncAdapterService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeyserverSyncAdapterService.java @@ -59,11 +59,12 @@ public class KeyserverSyncAdapterService extends Service { // time since last update after which a key should be updated again, in s public static final long KEY_UPDATE_LIMIT = Constants.DEBUG_KEYSERVER_SYNC ? 1 : TimeUnit.DAYS.toSeconds(7); - // time by which a sync is postponed in case of a + // time by which a sync is postponed in case screen is on public static final long SYNC_POSTPONE_TIME = Constants.DEBUG_KEYSERVER_SYNC ? 30 * 1000 : TimeUnit.MINUTES.toMillis(5); // Time taken by Orbot before a new circuit is created - public static final int ORBOT_CIRCUIT_TIMEOUT = (int) TimeUnit.MINUTES.toMillis(10); + public static final int ORBOT_CIRCUIT_TIMEOUT_SECONDS = + Constants.DEBUG_KEYSERVER_SYNC ? 2 : (int) TimeUnit.MINUTES.toSeconds(10); private static final String ACTION_IGNORE_TOR = "ignore_tor"; @@ -77,10 +78,14 @@ public class KeyserverSyncAdapterService extends Service { @Override public int onStartCommand(final Intent intent, int flags, final int startId) { + if (intent == null || intent.getAction() == null) { + // introduced due to https://github.com/open-keychain/open-keychain/issues/1573 + return START_NOT_STICKY; // we can't act on this Intent and don't want it redelivered + } switch (intent.getAction()) { case ACTION_CANCEL: { mCancelled.set(true); - break; + return START_NOT_STICKY; } // the reason for the separation betweyeen SYNC_NOW and UPDATE_ALL is so that starting // the sync directly from the notification is possible while the screen is on with @@ -92,44 +97,47 @@ public class KeyserverSyncAdapterService extends Service { Constants.PROVIDER_AUTHORITY, new Bundle() ); - break; + return START_NOT_STICKY; } case ACTION_UPDATE_ALL: { // does not check for screen on/off - asyncKeyUpdate(this, new CryptoInputParcel()); - break; + asyncKeyUpdate(this, new CryptoInputParcel(), startId); + // we depend on handleUpdateResult to call stopSelf when it is no longer necessary + // for the intent to be redelivered + return START_REDELIVER_INTENT; } case ACTION_IGNORE_TOR: { NotificationManager manager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); manager.cancel(Constants.Notification.KEYSERVER_SYNC_FAIL_ORBOT); - asyncKeyUpdate(this, new CryptoInputParcel(ParcelableProxy.getForNoProxy())); - break; + asyncKeyUpdate(this, new CryptoInputParcel(ParcelableProxy.getForNoProxy()), + startId); + // we depend on handleUpdateResult to call stopSelf when it is no longer necessary + // for the intent to be redelivered + return START_REDELIVER_INTENT; } case ACTION_START_ORBOT: { - NotificationManager manager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); + NotificationManager manager = (NotificationManager) + getSystemService(NOTIFICATION_SERVICE); manager.cancel(Constants.Notification.KEYSERVER_SYNC_FAIL_ORBOT); + Intent startOrbot = new Intent(this, OrbotRequiredDialogActivity.class); startOrbot.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); startOrbot.putExtra(OrbotRequiredDialogActivity.EXTRA_START_ORBOT, true); + Messenger messenger = new Messenger( new Handler() { @Override public void handleMessage(Message msg) { switch (msg.what) { case OrbotRequiredDialogActivity.MESSAGE_ORBOT_STARTED: { - asyncKeyUpdate(KeyserverSyncAdapterService.this, - new CryptoInputParcel()); - break; - } - case OrbotRequiredDialogActivity.MESSAGE_ORBOT_IGNORE: { - asyncKeyUpdate(KeyserverSyncAdapterService.this, - new CryptoInputParcel( - ParcelableProxy.getForNoProxy())); + startServiceWithUpdateAll(); break; } + case OrbotRequiredDialogActivity.MESSAGE_ORBOT_IGNORE: case OrbotRequiredDialogActivity.MESSAGE_DIALOG_CANCEL: { - // just stop service - stopSelf(); + // not possible since we proceed to Orbot's Activity + // directly, by starting OrbotRequiredDialogActivity with + // EXTRA_START_ORBOT set to true break; } } @@ -138,13 +146,17 @@ public class KeyserverSyncAdapterService extends Service { ); startOrbot.putExtra(OrbotRequiredDialogActivity.EXTRA_MESSENGER, messenger); startActivity(startOrbot); - break; + // since we return START_NOT_STICKY, we also postpone the sync as a backup in case + // the service is killed before OrbotRequiredDialogActivity can get back to us + postponeSync(); + // if use START_REDELIVER_INTENT, we might annoy the user by repeatedly starting the + // Orbot Activity when our service is killed and restarted + return START_NOT_STICKY; } case ACTION_DISMISS_NOTIFICATION: { NotificationManager manager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); manager.cancel(Constants.Notification.KEYSERVER_SYNC_FAIL_ORBOT); - stopSelf(startId); - break; + return START_NOT_STICKY; } } return START_NOT_STICKY; @@ -167,10 +179,7 @@ public class KeyserverSyncAdapterService extends Service { boolean isScreenOn = pm.isScreenOn(); if (!isScreenOn) { - Intent serviceIntent = new Intent(KeyserverSyncAdapterService.this, - KeyserverSyncAdapterService.class); - serviceIntent.setAction(ACTION_UPDATE_ALL); - startService(serviceIntent); + startServiceWithUpdateAll(); } else { postponeSync(); } @@ -188,16 +197,24 @@ public class KeyserverSyncAdapterService extends Service { return new KeyserverSyncAdapter().getSyncAdapterBinder(); } - private void handleUpdateResult(ImportKeyResult result) { + /** + * Since we're returning START_REDELIVER_INTENT in onStartCommand, we need to remember to call + * stopSelf(int) to prevent the Intent from being redelivered if our work is already done + * + * @param result result of keyserver sync + * @param startId startId provided to the onStartCommand call which resulted in this sync + */ + private void handleUpdateResult(ImportKeyResult result, final int startId) { if (result.isPending()) { + Log.d(Constants.TAG, "Orbot required for sync but not running, attempting to start"); // result is pending due to Orbot not being started // try to start it silently, if disabled show notifications new OrbotHelper.SilentStartManager() { @Override protected void onOrbotStarted() { // retry the update - asyncKeyUpdate(KeyserverSyncAdapterService.this, - new CryptoInputParcel()); + startServiceWithUpdateAll(); + stopSelf(startId); // startServiceWithUpdateAll will deliver a new Intent } @Override @@ -207,16 +224,24 @@ public class KeyserverSyncAdapterService extends Service { (NotificationManager) getSystemService(NOTIFICATION_SERVICE); manager.notify(Constants.Notification.KEYSERVER_SYNC_FAIL_ORBOT, getOrbotNoification(KeyserverSyncAdapterService.this)); + // further action on user interaction with notification, intent should not be + // redelivered, therefore: + stopSelf(startId); } }.startOrbotAndListen(this, false); + // if we're killed before we get a response from Orbot, we need the intent to be + // redelivered, so no stopSelf(int) here } else if (isUpdateCancelled()) { Log.d(Constants.TAG, "Keyserver sync cancelled, postponing by" + SYNC_POSTPONE_TIME + "ms"); postponeSync(); + // postponeSync creates a new intent, so we don't need this to be redelivered + stopSelf(startId); } else { Log.d(Constants.TAG, "Keyserver sync completed: Updated: " + result.mUpdatedKeys + " Failed: " + result.mBadKeys); - stopSelf(); + // key sync completed successfully, we can stop + stopSelf(startId); } } @@ -234,12 +259,12 @@ public class KeyserverSyncAdapterService extends Service { } private void asyncKeyUpdate(final Context context, - final CryptoInputParcel cryptoInputParcel) { + final CryptoInputParcel cryptoInputParcel, final int startId) { new Thread(new Runnable() { @Override public void run() { ImportKeyResult result = updateKeysFromKeyserver(context, cryptoInputParcel); - handleUpdateResult(result); + handleUpdateResult(result, startId); } }).start(); } @@ -278,7 +303,6 @@ public class KeyserverSyncAdapterService extends Service { ); } - /** * will perform a staggered update of user's keys using delays to ensure new Tor circuits, as * performed by parcimonie. Relevant issue and method at: @@ -290,17 +314,31 @@ public class KeyserverSyncAdapterService extends Service { CryptoInputParcel cryptoInputParcel) { Log.d(Constants.TAG, "Starting staggered update"); // final int WEEK_IN_SECONDS = (int) TimeUnit.DAYS.toSeconds(7); + // we are limiting our randomness to ORBOT_CIRCUIT_TIMEOUT_SECONDS for now final int WEEK_IN_SECONDS = 0; + ImportOperation.KeyImportAccumulator accumulator = new ImportOperation.KeyImportAccumulator(keyList.size(), null); + + // so that the first key can be updated without waiting. This is so that there isn't a + // large gap between a "Start Orbot" notification and the next key update + boolean first = true; + for (ParcelableKeyRing keyRing : keyList) { int waitTime; int staggeredTime = new Random().nextInt(1 + 2 * (WEEK_IN_SECONDS / keyList.size())); - if (staggeredTime >= ORBOT_CIRCUIT_TIMEOUT) { + if (staggeredTime >= ORBOT_CIRCUIT_TIMEOUT_SECONDS) { waitTime = staggeredTime; } else { - waitTime = ORBOT_CIRCUIT_TIMEOUT + new Random().nextInt(ORBOT_CIRCUIT_TIMEOUT); + waitTime = ORBOT_CIRCUIT_TIMEOUT_SECONDS + + new Random().nextInt(1 + ORBOT_CIRCUIT_TIMEOUT_SECONDS); + } + + if (first) { + waitTime = 0; + first = false; } + Log.d(Constants.TAG, "Updating key with fingerprint " + keyRing.mExpectedFingerprint + " with a wait time of " + waitTime + "s"); try { @@ -362,13 +400,15 @@ public class KeyserverSyncAdapterService extends Service { ); ArrayList ignoreMasterKeyIds = new ArrayList<>(); - while (updatedKeysCursor.moveToNext()) { + while (updatedKeysCursor != null && updatedKeysCursor.moveToNext()) { long masterKeyId = updatedKeysCursor.getLong(INDEX_UPDATED_KEYS_MASTER_KEY_ID); Log.d(Constants.TAG, "Keyserver sync: Ignoring {" + masterKeyId + "} last updated at {" + updatedKeysCursor.getLong(INDEX_LAST_UPDATED) + "}s"); ignoreMasterKeyIds.add(masterKeyId); } - updatedKeysCursor.close(); + if (updatedKeysCursor != null) { + updatedKeysCursor.close(); + } // 2. Make a list of public keys which should be updated final int INDEX_MASTER_KEY_ID = 0; @@ -413,7 +453,7 @@ public class KeyserverSyncAdapterService extends Service { /** * will cancel an update already in progress. We send an Intent to cancel it instead of simply - * modifying a static variable sync the service is running in a process that is different from + * modifying a static variable since the service is running in a process that is different from * the default application process where the UI code runs. * * @param context used to send an Intent to the service requesting cancellation. @@ -491,6 +531,12 @@ public class KeyserverSyncAdapterService extends Service { } } + private void startServiceWithUpdateAll() { + Intent serviceIntent = new Intent(this, KeyserverSyncAdapterService.class); + serviceIntent.setAction(ACTION_UPDATE_ALL); + this.startService(serviceIntent); + } + // from de.azapps.mirakel.helper.Helpers from https://github.com/MirakelX/mirakel-android private Bitmap getBitmap(int resId, Context context) { int mLargeIconWidth = (int) context.getResources().getDimension( -- cgit v1.2.3