From d1e85633fbe8d871355d2b9feb51e2c9983d8a21 Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Wed, 5 Sep 2018 20:17:14 -0500 Subject: Remodel the Model - The configuration and crypto model is now entirely independent of Android classes other than Nullable and TextUtils. - Model classes are immutable and use builders that enforce the appropriate optional/required attributes. - The Android config proxies (for Parcelable and databinding) are moved to the Android side of the codebase, and are designed to be safe for two-way databinding. This allows proper observability in TunnelDetailFragment. - Various robustness fixes and documentation updates to helper classes. Signed-off-by: Jason A. Donenfeld --- .../android/fragment/AppListDialogFragment.java | 17 +-- .../fragment/ConfigNamingDialogFragment.java | 7 +- .../android/fragment/TunnelDetailFragment.java | 9 +- .../android/fragment/TunnelEditorFragment.java | 115 ++++++--------------- .../android/fragment/TunnelListFragment.java | 14 +-- 5 files changed, 53 insertions(+), 109 deletions(-) (limited to 'app/src/main/java/com/wireguard/android/fragment') diff --git a/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java b/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java index 8bf5a22d..20633c3e 100644 --- a/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java +++ b/app/src/main/java/com/wireguard/android/fragment/AppListDialogFragment.java @@ -27,19 +27,21 @@ import com.wireguard.android.util.ObservableKeyedArrayList; import com.wireguard.android.util.ObservableKeyedList; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; +import java9.util.Comparators; + public class AppListDialogFragment extends DialogFragment { private static final String KEY_EXCLUDED_APPS = "excludedApps"; private final ObservableKeyedList appData = new ObservableKeyedArrayList<>(); - private List currentlyExcludedApps; + @Nullable private List currentlyExcludedApps; - public static AppListDialogFragment newInstance(final String[] excludedApps, final T target) { + public static + AppListDialogFragment newInstance(final ArrayList excludedApps, final T target) { final Bundle extras = new Bundle(); - extras.putStringArray(KEY_EXCLUDED_APPS, excludedApps); + extras.putStringArrayList(KEY_EXCLUDED_APPS, excludedApps); final AppListDialogFragment fragment = new AppListDialogFragment(); fragment.setTargetFragment(target, 0); fragment.setArguments(extras); @@ -64,7 +66,7 @@ public class AppListDialogFragment extends DialogFragment { appData.add(new ApplicationData(resolveInfo.loadIcon(pm), resolveInfo.loadLabel(pm).toString(), packageName, currentlyExcludedApps.contains(packageName))); } - Collections.sort(appData, (lhs, rhs) -> lhs.getName().toLowerCase().compareTo(rhs.getName().toLowerCase())); + Collections.sort(appData, Comparators.comparing(ApplicationData::getName, String.CASE_INSENSITIVE_ORDER)); return appData; }).whenComplete(((data, throwable) -> { if (data != null) { @@ -82,12 +84,11 @@ public class AppListDialogFragment extends DialogFragment { @Override public void onCreate(@Nullable final Bundle savedInstanceState) { super.onCreate(savedInstanceState); - - currentlyExcludedApps = Arrays.asList(getArguments().getStringArray(KEY_EXCLUDED_APPS)); + currentlyExcludedApps = getArguments().getStringArrayList(KEY_EXCLUDED_APPS); } @Override - public Dialog onCreateDialog(final Bundle savedInstanceState) { + public Dialog onCreateDialog(@Nullable final Bundle savedInstanceState) { final AlertDialog.Builder alertDialogBuilder = new AlertDialog.Builder(getActivity()); alertDialogBuilder.setTitle(R.string.excluded_applications); diff --git a/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java b/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java index 83799818..0931868e 100644 --- a/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java +++ b/app/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.java @@ -19,8 +19,11 @@ import com.wireguard.android.Application; import com.wireguard.android.R; import com.wireguard.android.databinding.ConfigNamingDialogFragmentBinding; import com.wireguard.config.Config; +import com.wireguard.config.ParseException; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Objects; public class ConfigNamingDialogFragment extends DialogFragment { @@ -63,8 +66,8 @@ public class ConfigNamingDialogFragment extends DialogFragment { super.onCreate(savedInstanceState); try { - config = Config.from(getArguments().getString(KEY_CONFIG_TEXT)); - } catch (final IOException exception) { + config = Config.parse(new ByteArrayInputStream(getArguments().getString(KEY_CONFIG_TEXT).getBytes(StandardCharsets.UTF_8))); + } catch (final IOException | ParseException exception) { throw new RuntimeException("Invalid config passed to " + getClass().getSimpleName(), exception); } } diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java index fcc601f3..b4e7202f 100644 --- a/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java +++ b/app/src/main/java/com/wireguard/android/fragment/TunnelDetailFragment.java @@ -16,7 +16,6 @@ import android.view.ViewGroup; import com.wireguard.android.R; import com.wireguard.android.databinding.TunnelDetailFragmentBinding; import com.wireguard.android.model.Tunnel; -import com.wireguard.config.Config; /** * Fragment that shows details about a specific tunnel. @@ -25,12 +24,6 @@ import com.wireguard.config.Config; public class TunnelDetailFragment extends BaseFragment { @Nullable private TunnelDetailFragmentBinding binding; - private void onConfigLoaded(final String name, final Config config) { - if (binding != null) { - binding.setConfig(new Config.Observable(config, name)); - } - } - @Override public void onCreate(@Nullable final Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -65,7 +58,7 @@ public class TunnelDetailFragment extends BaseFragment { if (newTunnel == null) binding.setConfig(null); else - newTunnel.getConfigAsync().thenAccept(a -> onConfigLoaded(newTunnel.getName(), a)); + newTunnel.getConfigAsync().thenAccept(binding::setConfig); } @Override diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java index 8f319e1e..f1250e64 100644 --- a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java +++ b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java @@ -7,7 +7,6 @@ package com.wireguard.android.fragment; import android.app.Activity; import android.content.Context; -import android.databinding.Observable; import android.databinding.ObservableList; import android.os.Bundle; import android.support.annotation.Nullable; @@ -24,19 +23,16 @@ import android.view.inputmethod.InputMethodManager; import android.widget.Toast; import com.wireguard.android.Application; -import com.wireguard.android.BR; import com.wireguard.android.R; import com.wireguard.android.databinding.TunnelEditorFragmentBinding; import com.wireguard.android.fragment.AppListDialogFragment.AppExclusionListener; import com.wireguard.android.model.Tunnel; import com.wireguard.android.model.TunnelManager; import com.wireguard.android.util.ExceptionLoggers; -import com.wireguard.config.Attribute; +import com.wireguard.android.viewmodel.ConfigProxy; import com.wireguard.config.Config; -import com.wireguard.config.Peer; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Objects; @@ -48,64 +44,13 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi private static final String KEY_LOCAL_CONFIG = "local_config"; private static final String KEY_ORIGINAL_NAME = "original_name"; private static final String TAG = "WireGuard/" + TunnelEditorFragment.class.getSimpleName(); - private final Collection breakObjectOrientedLayeringHandlerReceivers = new ArrayList<>(); - @Nullable private TunnelEditorFragmentBinding binding; - private final Observable.OnPropertyChangedCallback breakObjectOrientedLayeringHandler = new Observable.OnPropertyChangedCallback() { - @Override - public void onPropertyChanged(final Observable sender, final int propertyId) { - if (binding == null) - return; - final Config.Observable config = binding.getConfig(); - if (config == null) - return; - if (propertyId == BR.config) { - config.addOnPropertyChangedCallback(breakObjectOrientedLayeringHandler); - breakObjectOrientedLayeringHandlerReceivers.add(config); - config.getInterfaceSection().addOnPropertyChangedCallback(breakObjectOrientedLayeringHandler); - breakObjectOrientedLayeringHandlerReceivers.add(config.getInterfaceSection()); - config.getPeers().addOnListChangedCallback(breakObjectListOrientedLayeringHandler); - breakObjectOrientedLayeringHandlerReceivers.add(config.getPeers()); - } else if (propertyId == BR.dnses || propertyId == BR.peers) - ; - else - return; - final int numSiblings = config.getPeers().size() - 1; - for (final Peer.Observable peer : config.getPeers()) { - peer.setInterfaceDNSRoutes(config.getInterfaceSection().getDnses()); - peer.setNumSiblings(numSiblings); - } - } - }; - private final ObservableList.OnListChangedCallback> breakObjectListOrientedLayeringHandler = new ObservableList.OnListChangedCallback>() { - @Override - public void onChanged(final ObservableList sender) { - } - - @Override - public void onItemRangeChanged(final ObservableList sender, final int positionStart, final int itemCount) { - } - - @Override - public void onItemRangeInserted(final ObservableList sender, final int positionStart, final int itemCount) { - if (binding != null) - breakObjectOrientedLayeringHandler.onPropertyChanged(binding.getConfig(), BR.peers); - } - @Override - public void onItemRangeMoved(final ObservableList sender, final int fromPosition, final int toPosition, final int itemCount) { - } - - @Override - public void onItemRangeRemoved(final ObservableList sender, final int positionStart, final int itemCount) { - if (binding != null) - breakObjectOrientedLayeringHandler.onPropertyChanged(binding.getConfig(), BR.peers); - } - }; + @Nullable private TunnelEditorFragmentBinding binding; @Nullable private Tunnel tunnel; - private void onConfigLoaded(final String name, final Config config) { + private void onConfigLoaded(final Config config) { if (binding != null) { - binding.setConfig(new Config.Observable(config, name)); + binding.setConfig(new ConfigProxy(config)); } } @@ -143,29 +88,23 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi @Nullable final Bundle savedInstanceState) { super.onCreateView(inflater, container, savedInstanceState); binding = TunnelEditorFragmentBinding.inflate(inflater, container, false); - binding.addOnPropertyChangedCallback(breakObjectOrientedLayeringHandler); - breakObjectOrientedLayeringHandlerReceivers.add(binding); binding.executePendingBindings(); return binding.getRoot(); } - @SuppressWarnings("unchecked") @Override public void onDestroyView() { binding = null; - for (final Object o : breakObjectOrientedLayeringHandlerReceivers) { - if (o instanceof Observable) - ((Observable) o).removeOnPropertyChangedCallback(breakObjectOrientedLayeringHandler); - else if (o instanceof ObservableList) - ((ObservableList) o).removeOnListChangedCallback(breakObjectListOrientedLayeringHandler); - } super.onDestroyView(); } @Override public void onExcludedAppsSelected(final List excludedApps) { Objects.requireNonNull(binding, "Tried to set excluded apps while no view was loaded"); - binding.getConfig().getInterfaceSection().setExcludedApplications(Attribute.iterableToString(excludedApps)); + final ObservableList excludedApplications = + binding.getConfig().getInterface().getExcludedApplications(); + excludedApplications.clear(); + excludedApplications.addAll(excludedApps); } private void onFinished() { @@ -195,25 +134,27 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi public boolean onOptionsItemSelected(final MenuItem item) { switch (item.getItemId()) { case R.id.menu_action_save: - final Config newConfig = new Config(); + if (binding == null) + return false; + final Config newConfig; try { - binding.getConfig().commitData(newConfig); + newConfig = binding.getConfig().resolve(); } catch (final Exception e) { final String error = ExceptionLoggers.unwrapMessage(e); - final String tunnelName = tunnel == null ? binding.getConfig().getName() : tunnel.getName(); + final String tunnelName = tunnel == null ? binding.getName() : tunnel.getName(); final String message = getString(R.string.config_save_error, tunnelName, error); Log.e(TAG, message, e); Snackbar.make(binding.mainContainer, error, Snackbar.LENGTH_LONG).show(); return false; } if (tunnel == null) { - Log.d(TAG, "Attempting to create new tunnel " + binding.getConfig().getName()); + Log.d(TAG, "Attempting to create new tunnel " + binding.getName()); final TunnelManager manager = Application.getTunnelManager(); - manager.create(binding.getConfig().getName(), newConfig) + manager.create(binding.getName(), newConfig) .whenComplete(this::onTunnelCreated); - } else if (!tunnel.getName().equals(binding.getConfig().getName())) { - Log.d(TAG, "Attempting to rename tunnel to " + binding.getConfig().getName()); - tunnel.setName(binding.getConfig().getName()) + } else if (!tunnel.getName().equals(binding.getName())) { + Log.d(TAG, "Attempting to rename tunnel to " + binding.getName()); + tunnel.setName(binding.getName()) .whenComplete((a, b) -> onTunnelRenamed(tunnel, newConfig, b)); } else { Log.d(TAG, "Attempting to save config of " + tunnel.getName()); @@ -229,7 +170,7 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi public void onRequestSetExcludedApplications(@SuppressWarnings("unused") final View view) { final FragmentManager fragmentManager = getFragmentManager(); if (fragmentManager != null && binding != null) { - final String[] excludedApps = Attribute.stringToList(binding.getConfig().getInterfaceSection().getExcludedApplications()); + final ArrayList excludedApps = new ArrayList<>(binding.getConfig().getInterface().getExcludedApplications()); final AppListDialogFragment fragment = AppListDialogFragment.newInstance(excludedApps, this); fragment.show(fragmentManager, null); } @@ -237,19 +178,25 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi @Override public void onSaveInstanceState(final Bundle outState) { - outState.putParcelable(KEY_LOCAL_CONFIG, binding.getConfig()); + if (binding != null) + outState.putParcelable(KEY_LOCAL_CONFIG, binding.getConfig()); outState.putString(KEY_ORIGINAL_NAME, tunnel == null ? null : tunnel.getName()); super.onSaveInstanceState(outState); } @Override - public void onSelectedTunnelChanged(@Nullable final Tunnel oldTunnel, @Nullable final Tunnel newTunnel) { + public void onSelectedTunnelChanged(@Nullable final Tunnel oldTunnel, + @Nullable final Tunnel newTunnel) { tunnel = newTunnel; if (binding == null) return; - binding.setConfig(new Config.Observable(null, null)); - if (tunnel != null) - tunnel.getConfigAsync().thenAccept(a -> onConfigLoaded(tunnel.getName(), a)); + binding.setConfig(new ConfigProxy()); + if (tunnel != null) { + binding.setName(tunnel.getName()); + tunnel.getConfigAsync().thenAccept(this::onConfigLoaded); + } else { + binding.setName(""); + } } private void onTunnelCreated(final Tunnel newTunnel, @Nullable final Throwable throwable) { @@ -301,7 +248,7 @@ public class TunnelEditorFragment extends BaseFragment implements AppExclusionLi onSelectedTunnelChanged(null, getSelectedTunnel()); } else { tunnel = getSelectedTunnel(); - final Config.Observable config = savedInstanceState.getParcelable(KEY_LOCAL_CONFIG); + final ConfigProxy config = savedInstanceState.getParcelable(KEY_LOCAL_CONFIG); final String originalName = savedInstanceState.getString(KEY_ORIGINAL_NAME); if (tunnel != null && !tunnel.getName().equals(originalName)) onSelectedTunnelChanged(null, tunnel); diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java index 7509e40c..783c0a29 100644 --- a/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java +++ b/app/src/main/java/com/wireguard/android/fragment/TunnelListFragment.java @@ -41,9 +41,10 @@ import com.wireguard.android.util.ExceptionLoggers; import com.wireguard.android.widget.MultiselectableRelativeLayout; import com.wireguard.android.widget.fab.FloatingActionsMenuRecyclerViewScrollListener; import com.wireguard.config.Config; +import com.wireguard.config.ParseException; -import java.io.BufferedReader; -import java.io.InputStreamReader; +import java.io.ByteArrayInputStream; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; @@ -79,13 +80,13 @@ public class TunnelListFragment extends BaseFragment { private void importTunnel(@NonNull final String configText) { try { // Ensure the config text is parseable before proceeding… - Config.from(configText); + Config.parse(new ByteArrayInputStream(configText.getBytes(StandardCharsets.UTF_8))); // Config text is valid, now create the tunnel… final FragmentManager fragmentManager = getFragmentManager(); if (fragmentManager != null) ConfigNamingDialogFragment.newInstance(configText).show(fragmentManager, null); - } catch (final Exception exception) { + } catch (final IllegalArgumentException | IOException | ParseException exception) { onTunnelImportFinished(Collections.emptyList(), Collections.singletonList(exception)); } } @@ -122,7 +123,6 @@ public class TunnelListFragment extends BaseFragment { if (isZip) { try (ZipInputStream zip = new ZipInputStream(contentResolver.openInputStream(uri))) { - BufferedReader reader = new BufferedReader(new InputStreamReader(zip, StandardCharsets.UTF_8)); ZipEntry entry; while ((entry = zip.getNextEntry()) != null) { if (entry.isDirectory()) @@ -140,7 +140,7 @@ public class TunnelListFragment extends BaseFragment { continue; Config config = null; try { - config = Config.from(reader); + config = Config.parse(zip); } catch (Exception e) { throwables.add(e); } @@ -150,7 +150,7 @@ public class TunnelListFragment extends BaseFragment { } } else { futureTunnels.add(Application.getTunnelManager().create(name, - Config.from(contentResolver.openInputStream(uri))).toCompletableFuture()); + Config.parse(contentResolver.openInputStream(uri))).toCompletableFuture()); } if (futureTunnels.isEmpty()) { -- cgit v1.2.3