summaryrefslogtreecommitdiffhomepage
path: root/app/src/main/java/com/wireguard
diff options
context:
space:
mode:
authorSamuel Holland <samuel@sholland.org>2017-08-01 01:38:39 -0500
committerSamuel Holland <samuel@sholland.org>2017-08-01 01:38:39 -0500
commit2d747aab64f36872ae3074ee4b91a4ed1e4d07e4 (patch)
treec9fe552530eb3e7d3a98bae8fa2c0f019c458eca /app/src/main/java/com/wireguard
parent92d9b8a5fd28a9661c2cca8e359076539bf016a3 (diff)
ProfileService: Rework profile updating
This should help discourage editing a Profile without making a copy first, since the caller has to keep track of the old Profile as well. ProfileAdder has been updated to prevent adding duplicate profiles. It checks once in the constructor, so the caller can catch the exception and pass the error back to the UI. It checks again in the worker thread to prevent any race from happening if a profile is added twice quickly. Either the file exists, or it doesn't. Additionally, this change solves the race condition when the old profile is removed before it is updated; previously this would lead to the profile being re-added. Now, ProfileRemover will fail and the profile will stay removed. Finally, updating a profile's name should now work correctly. There were previously multiple bugs with that (the old profile wasn't removed, the new one could duplicate a name, the new one could overwrite some random other one, etc.).
Diffstat (limited to 'app/src/main/java/com/wireguard')
-rw-r--r--app/src/main/java/com/wireguard/android/ProfileService.java80
-rw-r--r--app/src/main/java/com/wireguard/android/ProfileServiceInterface.java5
2 files changed, 29 insertions, 56 deletions
diff --git a/app/src/main/java/com/wireguard/android/ProfileService.java b/app/src/main/java/com/wireguard/android/ProfileService.java
index 5bc0593c..aca47c1c 100644
--- a/app/src/main/java/com/wireguard/android/ProfileService.java
+++ b/app/src/main/java/com/wireguard/android/ProfileService.java
@@ -54,10 +54,15 @@ public class ProfileService extends Service {
private class ProfileAdder extends AsyncTask<Void, Void, Boolean> {
private final Profile profile;
+ private final boolean shouldConnect;
- private ProfileAdder(Profile profile) {
+ private ProfileAdder(Profile profile, boolean shouldConnect) {
super();
+ for (Profile p : profiles)
+ if (p.getName().equals(profile.getName()))
+ throw new IllegalStateException("Profile already exists: " + profile.getName());
this.profile = profile;
+ this.shouldConnect = shouldConnect;
}
@Override
@@ -65,6 +70,8 @@ public class ProfileService extends Service {
Log.i(TAG, "Adding profile " + profile.getName());
try {
final String configFile = profile.getName() + ".conf";
+ if (new File(getFilesDir(), configFile).exists())
+ throw new IOException("Refusing to overwrite existing profile configuration");
final FileOutputStream stream = openFileOutput(configFile, MODE_PRIVATE);
stream.write(profile.toString().getBytes(StandardCharsets.UTF_8));
stream.close();
@@ -81,6 +88,8 @@ public class ProfileService extends Service {
return;
profile.setIsConnected(false);
profiles.add(profile);
+ if (shouldConnect)
+ new ProfileConnecter(profile).execute();
}
}
@@ -173,10 +182,14 @@ public class ProfileService extends Service {
private class ProfileRemover extends AsyncTask<Void, Void, Boolean> {
private final Profile profile;
+ private final Profile replaceWith;
+ private final boolean shouldConnect;
- private ProfileRemover(Profile profile) {
+ private ProfileRemover(Profile profile, Profile replaceWith, Boolean shouldConnect) {
super();
this.profile = profile;
+ this.replaceWith = replaceWith;
+ this.shouldConnect = shouldConnect != null ? shouldConnect : false;
}
@Override
@@ -196,46 +209,8 @@ public class ProfileService extends Service {
if (!result)
return;
profiles.remove(profile);
- }
- }
-
- private class ProfileUpdater extends AsyncTask<Void, Void, Boolean> {
- private final Profile profile, newProfile;
- private final boolean wasConnected;
-
- private ProfileUpdater(Profile profile, Profile newProfile, boolean wasConnected) {
- super();
- this.profile = profile;
- this.newProfile = newProfile;
- this.wasConnected = wasConnected;
- }
-
- @Override
- protected Boolean doInBackground(Void... voids) {
- Log.i(TAG, "Updating profile " + profile.getName());
- if (!newProfile.getName().equals(profile.getName()))
- throw new IllegalStateException("Profile name mismatch: " + profile.getName());
- try {
- final String configFile = profile.getName() + ".conf";
- final FileOutputStream stream = openFileOutput(configFile, MODE_PRIVATE);
- stream.write(newProfile.toString().getBytes(StandardCharsets.UTF_8));
- stream.close();
- return true;
- } catch (IOException e) {
- Log.e(TAG, "Could not update profile " + profile.getName(), e);
- return false;
- }
- }
-
- @Override
- protected void onPostExecute(Boolean result) {
- if (!result)
- return;
- // FIXME: This is also why the list of profiles should be a map.
- final int index = profiles.indexOf(profile);
- profiles.set(index, newProfile);
- if (wasConnected)
- new ProfileConnecter(newProfile).execute();
+ if (replaceWith != null)
+ new ProfileAdder(replaceWith, shouldConnect).execute();
}
}
@@ -276,23 +251,20 @@ public class ProfileService extends Service {
return;
if (profile.getIsConnected())
new ProfileDisconnecter(profile).execute();
- new ProfileRemover(profile).execute();
+ new ProfileRemover(profile, null, null).execute();
}
@Override
- public void saveProfile(Profile newProfile) {
- // FIXME: This is why the list of profiles should be a map.
- Profile profile = null;
- for (Profile p : profiles)
- if (p.getName().equals(newProfile.getName()))
- profile = p;
- if (profile != null) {
- final boolean wasConnected = profile.getIsConnected();
+ public void saveProfile(Profile oldProfile, Profile newProfile) {
+ if (oldProfile != null) {
+ if (!profiles.contains(oldProfile))
+ return;
+ final boolean wasConnected = oldProfile.getIsConnected();
if (wasConnected)
- new ProfileDisconnecter(profile).execute();
- new ProfileUpdater(profile, newProfile, wasConnected).execute();
+ new ProfileDisconnecter(oldProfile).execute();
+ new ProfileRemover(oldProfile, newProfile, wasConnected).execute();
} else {
- new ProfileAdder(newProfile).execute();
+ new ProfileAdder(newProfile, false).execute();
}
}
}
diff --git a/app/src/main/java/com/wireguard/android/ProfileServiceInterface.java b/app/src/main/java/com/wireguard/android/ProfileServiceInterface.java
index 98024afb..272059bc 100644
--- a/app/src/main/java/com/wireguard/android/ProfileServiceInterface.java
+++ b/app/src/main/java/com/wireguard/android/ProfileServiceInterface.java
@@ -57,13 +57,14 @@ public interface ProfileServiceInterface {
void removeProfile(Profile profile);
/**
- * Adds the profile if it does not yet exist, or replaces an existing profile of the same name.
+ * Replace the given profile, or add a new profile if oldProfile is null.
* If the profile exists and is currently connected, it will be disconnected before the
* replacement, and the service will attempt to reconnect it afterward. If the profile is new,
* it will be set to the disconnected state. If successful, configuration for this profile will
* be saved to persistent storage.
*
+ * @param oldProfile The existing profile to replace, or null to add the new profile.
* @param newProfile The profile to add, or a copy of the profile to replace.
*/
- void saveProfile(Profile newProfile);
+ void saveProfile(Profile oldProfile, Profile newProfile);
}