From a828e83399a1ea29341e10d498002216be54ca8c Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Thu, 24 Aug 2017 22:13:46 -0500 Subject: [PATCH] Centralize/unify validation of configurations Signed-off-by: Jason A. Donenfeld --- .../java/com/wireguard/android/ConfigEditFragment.java | 7 +------ .../main/java/com/wireguard/android/VpnService.java | 10 +++++++++- app/src/main/java/com/wireguard/config/Config.java | 10 +--------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/wireguard/android/ConfigEditFragment.java b/app/src/main/java/com/wireguard/android/ConfigEditFragment.java index ebebef42..1edae9cc 100644 --- a/app/src/main/java/com/wireguard/android/ConfigEditFragment.java +++ b/app/src/main/java/com/wireguard/android/ConfigEditFragment.java @@ -113,18 +113,13 @@ public class ConfigEditFragment extends BaseConfigFragment { } private void saveConfig() { - final String errorMessage = localConfig.validate(); final VpnService service = VpnService.getInstance(); - if (errorMessage != null) { - Toast.makeText(getActivity(), errorMessage, Toast.LENGTH_SHORT).show(); - return; - } try { if (getCurrentConfig() != null) service.update(getCurrentConfig().getName(), localConfig); else service.add(localConfig); - } catch (final IllegalStateException e) { + } catch (final IllegalArgumentException | IllegalStateException e) { Toast.makeText(getActivity(), e.getMessage(), Toast.LENGTH_SHORT).show(); return; } diff --git a/app/src/main/java/com/wireguard/android/VpnService.java b/app/src/main/java/com/wireguard/android/VpnService.java index ad8eb608..0fd67136 100644 --- a/app/src/main/java/com/wireguard/android/VpnService.java +++ b/app/src/main/java/com/wireguard/android/VpnService.java @@ -13,6 +13,7 @@ import android.service.quicksettings.TileService; import android.util.Log; import com.wireguard.config.Config; +import com.wireguard.config.Peer; import java.io.File; import java.io.FileOutputStream; @@ -367,8 +368,15 @@ public class VpnService extends Service // When adding a config, "old file" and "new file" are the same thing. oldName = knownConfig != null ? knownConfig.getName() : newName; this.shouldConnect = shouldConnect; + if (newName == null || !Config.isNameValid(newName)) + throw new IllegalArgumentException("This configuration does not have a valid name"); if (isAddOrRename() && configurations.containsKey(newName)) - throw new IllegalStateException("Config " + newName + " already exists"); + throw new IllegalStateException("Configuration " + newName + " already exists"); + if (newConfig.getInterface().getPublicKey() == null) + throw new IllegalArgumentException("This configuration must have a valid keypair"); + for (final Peer peer : newConfig.getPeers()) + if (peer.getPublicKey() == null || peer.getPublicKey().isEmpty()) + throw new IllegalArgumentException("Each peer must have a valid public key"); } @Override diff --git a/app/src/main/java/com/wireguard/config/Config.java b/app/src/main/java/com/wireguard/config/Config.java index cb9b7ed2..2a282d09 100644 --- a/app/src/main/java/com/wireguard/config/Config.java +++ b/app/src/main/java/com/wireguard/config/Config.java @@ -40,7 +40,7 @@ public class Config extends BaseObservable public static final int NAME_MAX_LENGTH = 16; private static final Pattern PATTERN = Pattern.compile("^[a-zA-Z0-9_=+.-]{1,16}$"); - private static boolean isNameValid(final String name) { + public static boolean isNameValid(final String name) { return name.length() <= NAME_MAX_LENGTH && PATTERN.matcher(name).matches(); } @@ -180,14 +180,6 @@ public class Config extends BaseObservable return sb.toString(); } - public String validate() { - if (name == null || !isNameValid(name)) - return "This configuration does not have a valid name."; - if (iface.getPublicKey() == null) - return "This configuration does not have a valid keypair."; - return null; - } - @Override public void writeToParcel(final Parcel dest, final int flags) { dest.writeParcelable(iface, flags);