From 9ee976823dfc18e7ba5cf885b92032504b060ed9 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 24 Apr 2018 03:31:10 +0200 Subject: [PATCH] Throw IllegalArgumentExceptions when arguments are bad This will make the two way data binding crash more, but it will improve the robustness of the config file parser, which deals with exceptions gracefully, and when we move to one way data binding, it will help with that too. Signed-off-by: Jason A. Donenfeld --- .../java/com/wireguard/config/Attribute.java | 2 +- .../java/com/wireguard/config/IPCidr.java | 10 ++++-- .../java/com/wireguard/config/Interface.java | 33 +++++++++---------- .../main/java/com/wireguard/config/Peer.java | 27 +++++++-------- 4 files changed, 35 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/com/wireguard/config/Attribute.java b/app/src/main/java/com/wireguard/config/Attribute.java index b574ecfa..c73e9f2e 100644 --- a/app/src/main/java/com/wireguard/config/Attribute.java +++ b/app/src/main/java/com/wireguard/config/Attribute.java @@ -51,7 +51,7 @@ enum Attribute { } public static String[] stringToList(final String string) { - return string.trim().split("\\s*,\\s*", -1); + return string.trim().split("\\s*,\\s*"); } public String composeWith(final Object value) { diff --git a/app/src/main/java/com/wireguard/config/IPCidr.java b/app/src/main/java/com/wireguard/config/IPCidr.java index adc778c0..245141ac 100644 --- a/app/src/main/java/com/wireguard/config/IPCidr.java +++ b/app/src/main/java/com/wireguard/config/IPCidr.java @@ -25,11 +25,11 @@ public class IPCidr implements Parcelable { } }; - public IPCidr(String in) throws UnknownHostException { + public IPCidr(String in) { parse(in); } - private void parse(String in) throws UnknownHostException { + private void parse(String in) { cidr = -1; int slash = in.lastIndexOf('/'); if (slash != -1 && slash < in.length() - 1) { @@ -39,7 +39,11 @@ public class IPCidr implements Parcelable { } catch (Exception e) { } } - address = InetAddress.getByName(in); + try { + address = InetAddress.getByName(in); + } catch (UnknownHostException e) { + throw new IllegalArgumentException(e); + } if ((address instanceof Inet6Address) && (cidr > 128 || cidr < 0)) cidr = 128; else if ((address instanceof Inet4Address) && (cidr > 32 || cidr < 0)) diff --git a/app/src/main/java/com/wireguard/config/Interface.java b/app/src/main/java/com/wireguard/config/Interface.java index b90d116f..3f775389 100644 --- a/app/src/main/java/com/wireguard/config/Interface.java +++ b/app/src/main/java/com/wireguard/config/Interface.java @@ -135,7 +135,7 @@ public class Interface extends BaseObservable implements Parcelable { return keypair != null ? keypair.getPublicKey() : null; } - public void parse(final String line) throws UnknownHostException { + public void parse(final String line) { final Attribute key = Attribute.match(line); if (key == Attribute.ADDRESS) addAddresses(key.parseList(line)); @@ -151,11 +151,11 @@ public class Interface extends BaseObservable implements Parcelable { throw new IllegalArgumentException(line); } - public void addAddresses(String[] addresses) throws UnknownHostException { + public void addAddresses(String[] addresses) { if (addresses != null && addresses.length > 0) { for (final String addr : addresses) { if (addr.isEmpty()) - throw new UnknownHostException("{empty}"); + throw new IllegalArgumentException("Address is empty"); this.addressList.add(new IPCidr(addr)); } } @@ -166,19 +166,19 @@ public class Interface extends BaseObservable implements Parcelable { public void setAddressString(final String addressString) { this.addressList.clear(); - try { - addAddresses(Attribute.stringToList(addressString)); - } catch (Exception e) { - this.addressList.clear(); - } + addAddresses(Attribute.stringToList(addressString)); } - public void addDnses(String[] dnses) throws UnknownHostException { + public void addDnses(String[] dnses) { if (dnses != null && dnses.length > 0) { for (final String dns : dnses) { if (dns.isEmpty()) - throw new UnknownHostException("{empty}"); - this.dnsList.add(InetAddress.getByName(dns)); + throw new IllegalArgumentException("DNS is empty"); + try { + this.dnsList.add(InetAddress.getByName(dns)); + } catch (UnknownHostException e) { + throw new IllegalArgumentException(e); + } } } notifyPropertyChanged(BR.dnses); @@ -187,12 +187,8 @@ public class Interface extends BaseObservable implements Parcelable { } public void setDnsString(final String dnsString) { - try { - this.dnsList.clear(); - addDnses(Attribute.stringToList(dnsString)); - } catch (Exception e) { - this.dnsList.clear(); - } + this.dnsList.clear(); + addDnses(Attribute.stringToList(dnsString)); } public void setListenPort(int listenPort) { @@ -228,8 +224,9 @@ public class Interface extends BaseObservable implements Parcelable { if (privateKey != null && privateKey.length() == KeyEncoding.KEY_LENGTH_BASE64) { try { keypair = new Keypair(privateKey); - } catch (final IllegalArgumentException ignored) { + } catch (final IllegalArgumentException e) { keypair = null; + throw e; } } else { keypair = null; diff --git a/app/src/main/java/com/wireguard/config/Peer.java b/app/src/main/java/com/wireguard/config/Peer.java index 58f2633f..c2b23e9f 100644 --- a/app/src/main/java/com/wireguard/config/Peer.java +++ b/app/src/main/java/com/wireguard/config/Peer.java @@ -125,7 +125,7 @@ public class Peer extends BaseObservable implements Parcelable { return publicKey; } - public void parse(final String line) throws UnknownHostException { + public void parse(final String line) { final Attribute key = Attribute.match(line); if (key == Attribute.ALLOWED_IPS) addAllowedIPs(key.parseList(line)); @@ -141,11 +141,11 @@ public class Peer extends BaseObservable implements Parcelable { throw new IllegalArgumentException(line); } - public void addAllowedIPs(String[] allowedIPs) throws UnknownHostException { + public void addAllowedIPs(String[] allowedIPs) { if (allowedIPs != null && allowedIPs.length > 0) { for (final String allowedIP : allowedIPs) { if (allowedIP.isEmpty()) - throw new UnknownHostException("{empty}"); + throw new IllegalArgumentException("AllowedIP is empty"); this.allowedIPsList.add(new IPCidr(allowedIP)); } } @@ -154,12 +154,8 @@ public class Peer extends BaseObservable implements Parcelable { } public void setAllowedIPsString(final String allowedIPsString) { - try { - this.allowedIPsList.clear(); - addAllowedIPs(Attribute.stringToList(allowedIPsString)); - } catch (Exception e) { - this.allowedIPsList.clear(); - } + this.allowedIPsList.clear(); + addAllowedIPs(Attribute.stringToList(allowedIPsString)); } public void setEndpoint(InetSocketAddress endpoint) { @@ -171,14 +167,15 @@ public class Peer extends BaseObservable implements Parcelable { public void setEndpointString(final String endpoint) { if (endpoint != null && !endpoint.isEmpty()) { InetSocketAddress constructedEndpoint; + if (endpoint.indexOf('/') != -1 || endpoint.indexOf('?') != -1 || endpoint.indexOf('#') != -1) + throw new IllegalArgumentException("Forbidden characters in endpoint"); + URI uri; try { - if (endpoint.indexOf('/') != -1 || endpoint.indexOf('?') != -1 || endpoint.indexOf('#') != -1) - throw new Exception(); - URI uri = new URI("wg://" + endpoint); - constructedEndpoint = InetSocketAddress.createUnresolved(uri.getHost(), uri.getPort()); - } catch (Exception e) { - return; /* XXX: Uh oh. */ + uri = new URI("wg://" + endpoint); + } catch (URISyntaxException e) { + throw new IllegalArgumentException(e); } + constructedEndpoint = InetSocketAddress.createUnresolved(uri.getHost(), uri.getPort()); setEndpoint(constructedEndpoint); } else setEndpoint(null);