Refine SRV lookup; run resolving synchronized (no thread-interleave)srv

4 files changed, 26 insertions, 23 deletions

diff --git a/build.gradle b/build.gradle
index 36f2fdd..f0fd30e 100644
--- a/build.gradle
+++ b/build.gradle
@@ -43,6 +43,7 @@ dependencies {
     implementation 'com.googlecode.javacpp:javacpp:0.7'
     implementation 'org.jetbrains:annotations:18.0.0'
     implementation 'org.minidns:minidns-hla:0.3.4'
+    implementation 'com.google.guava:guava:28.2-android'
 }
 
 android {
diff --git a/src/main/AndroidManifest.xml b/src/main/AndroidManifest.xml
index 0605155..e1b87e8 100644
--- a/src/main/AndroidManifest.xml
+++ b/src/main/AndroidManifest.xml
@@ -21,6 +21,7 @@
     android:versionName="1.0">
 
     <uses-permission android:name="android.permission.INTERNET" />
+    <!-- Needed for ConnectivityManager, at least -->
     <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
 
     <application android:allowBackup="true">
diff --git a/src/main/java/se/lublin/humla/HumlaService.java b/src/main/java/se/lublin/humla/HumlaService.java
index f2667a4..63f578f 100644
--- a/src/main/java/se/lublin/humla/HumlaService.java
+++ b/src/main/java/se/lublin/humla/HumlaService.java
@@ -296,7 +296,7 @@ public class HumlaService extends Service implements IHumlaService, IHumlaSessio
 
             mCallbacks.onConnecting();
 
-            mConnection.connect(mServer.getResolvedHost(), mServer.getResolvedPort());
+            mConnection.connect(mServer.getSrvHost(), mServer.getSrvPort());
         } catch (HumlaException e) {
             e.printStackTrace();
             mCallbacks.onDisconnected(e);
diff --git a/src/main/java/se/lublin/humla/model/Server.java b/src/main/java/se/lublin/humla/model/Server.java
index b9b7faf..adde92a 100644
--- a/src/main/java/se/lublin/humla/model/Server.java
+++ b/src/main/java/se/lublin/humla/model/Server.java
@@ -20,7 +20,8 @@ package se.lublin.humla.model;
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.util.Log;
-import android.util.Patterns;
+
+import com.google.common.net.InetAddresses;
 
 import org.minidns.hla.ResolverApi;
 import org.minidns.hla.SrvResolverResult;
@@ -36,7 +37,6 @@ import java.util.concurrent.atomic.AtomicReference;
 import se.lublin.humla.Constants;
 
 public class Server implements Parcelable {
-
     private long mId;
     private String mName;
     private String mHost;
@@ -44,7 +44,6 @@ public class Server implements Parcelable {
     private String mUsername;
     private String mPassword;
 
-    // TODO SRV should we remove the cache when disconnecting? maybe
     private String mResolvedHost = null;
     private int mResolvedPort;
 
@@ -85,7 +84,7 @@ public class Server implements Parcelable {
         parcel.writeString(mPassword);
     }
 
-    public void readFromParcel(Parcel in) {
+    private void readFromParcel(Parcel in) {
         mId = in.readLong();
         mName = in.readString();
         mHost = in.readString();
@@ -162,37 +161,40 @@ public class Server implements Parcelable {
         return mId != -1;
     }
 
-    public String getResolvedHost() {
-        resolveSRV();
+    public String getSrvHost() {
+        srvResolve();
         return mResolvedHost;
     }
 
-    public int getResolvedPort() {
-        resolveSRV();
+    public int getSrvPort() {
+        srvResolve();
         return mResolvedPort;
     }
 
-    private void resolveSRV() {
+    private synchronized void srvResolve() {
         if (mResolvedHost != null) {
             return;
         }
-        mResolvedHost = mHost;
-        mResolvedPort = mPort;
-        if (mResolvedPort != 0) {
+        // if we have a port then don't bother with SRV
+        if (mPort != 0) {
+            mResolvedHost = mHost;
+            mResolvedPort = mPort;
             return;
         }
-        mResolvedPort = Constants.DEFAULT_PORT;
-        if (Patterns.IP_ADDRESS.matcher(mResolvedHost).matches()) {
+        if (InetAddresses.isInetAddress(mHost)) {
+            mResolvedHost = mHost;
+            mResolvedPort = Constants.DEFAULT_PORT;
             return;
         }
-        final AtomicReference<String> host = new AtomicReference<>(mResolvedHost);
-        final AtomicInteger port = new AtomicInteger(mResolvedPort);
+        // set to our fallback values in case of no SRV or resolve fail
+        final AtomicReference<String> srvHost = new AtomicReference<>(mHost);
+        final AtomicInteger srvPort = new AtomicInteger(Constants.DEFAULT_PORT);
         try {
             Thread t = new Thread(new Runnable() {
                 @Override
                 public void run() {
                     try {
-                        final String lookup = "_mumble._tcp." + host.get();
+                        final String lookup = "_mumble._tcp." + srvHost.get();
                         SrvResolverResult res = ResolverApi.INSTANCE.resolveSrv(lookup);
                         if (!res.wasSuccessful()) {
                             Log.d(Constants.TAG, "resolveSrv " + lookup + ": " + res.getResponseCode());
@@ -206,8 +208,8 @@ public class Server implements Parcelable {
                         List<SRV> srvs = SrvUtil.sortSrvRecords(answers);
                         for (SRV srv : srvs) {
                             Log.d(Constants.TAG, "resolved " + lookup + " SRV: " + srv.toString());
-                            host.set(srv.target.toString());
-                            port.set(srv.port);
+                            srvHost.set(srv.target.toString());
+                            srvPort.set(srv.port);
                             // TODO SRV just picking the first record.
                             return;
                         }
@@ -222,8 +224,7 @@ public class Server implements Parcelable {
         catch (Exception e) {
             Log.d(Constants.TAG, "resolveSRV() " + e);
         }
-        mResolvedHost = host.get();
-        mResolvedPort = port.get();
+        mResolvedHost = srvHost.get();
+        mResolvedPort = srvPort.get();
     }
-
 }