From e986a294ec3e328bd48bb4c423fe835d1aa2dc69 Mon Sep 17 00:00:00 2001
From: cyBerta <cyberta@riseup.net>
Date: Fri, 24 Jul 2020 16:30:02 +0200
Subject: fix bug in gateway selection - always iterate through all gateways
 and count those supporting the requested transport in order to get the
 nClosest gateway; adding more tests

---
 .../se/leap/bitmaskclient/eip/GatewaysManager.java | 21 +++++++++-----
 .../bitmaskclient/eip/GatewaysManagerTest.java     | 33 ++++++++++++++++++++--
 .../ptdemo_three_mixed_gateways.geoip.json         | 12 ++++++++
 3 files changed, 57 insertions(+), 9 deletions(-)
 create mode 100644 app/src/test/resources/ptdemo_three_mixed_gateways.geoip.json

diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java b/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
index aa10ffae..f3773758 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
@@ -81,22 +81,29 @@ public class GatewaysManager {
         List<Gateway> list = new ArrayList<>(gateways.values());
         GatewaySelector gatewaySelector = new GatewaySelector(list);
         Gateway gateway;
-        while ((gateway = gatewaySelector.select(nClosest)) != null) {
+        int found  = 0;
+        int i = 0;
+        while ((gateway = gatewaySelector.select(i)) != null) {
             if (gateway.getProfile(transportType) != null) {
-                return gateway;
+                if (found == nClosest) {
+                    return gateway;
+                }
+                found++;
             }
-            nClosest++;
+            i++;
         }
         return null;
     }
 
     private Gateway getGatewayFromPresortedList(int nClosest, Connection.TransportType transportType) {
-        while (nClosest < presortedList.size()) {
-            Gateway gateway = presortedList.get(nClosest);
+        int found = 0;
+        for (Gateway gateway : presortedList) {
             if (gateway.getProfile(transportType) != null) {
-                return gateway;
+                if (found == nClosest) {
+                    return gateway;
+                }
+                found++;
             }
-            nClosest++;
         }
         return null;
     }
diff --git a/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java b/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java
index dd5eb00c..5c5ebbfe 100644
--- a/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java
+++ b/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java
@@ -161,8 +161,6 @@ public class GatewaysManagerTest {
     @Test
     public void TestSelectN_selectFirstObfs4Connection_returnThirdGateway() throws JSONException, ConfigParser.ConfigParseError, IOException {
         Provider provider = getProvider(null, null, null, null, null, null, "ptdemo_two_openvpn_one_pt_gateways.json", null);
-        JSONObject eipServiceJson = provider.getEipServiceJson();
-        JSONObject gateway3 = eipServiceJson.getJSONArray(GATEWAYS).getJSONObject(2);
 
         MockHelper.mockProviderObserver(provider);
         mockStatic(PreferenceHelper.class);
@@ -172,6 +170,37 @@ public class GatewaysManagerTest {
         assertEquals("37.12.247.10", gatewaysManager.select(0).getRemoteIP());
     }
 
+    @Test
+    public void testSelectN_selectFromPresortedGateways_returnsGatewaysInPresortedOrder() throws JSONException {
+        Provider provider = getProvider(null, null, null, null, null, null, "ptdemo_three_mixed_gateways.json", "ptdemo_three_mixed_gateways.geoip.json");
+
+        MockHelper.mockProviderObserver(provider);
+        //use openvpn, not pluggable transports
+        mockStatic(PreferenceHelper.class);
+        when(PreferenceHelper.getUsePluggableTransports(any(Context.class))).thenReturn(false);
+        GatewaysManager gatewaysManager = new GatewaysManager(mockContext);
+
+        assertEquals("manila.bitmask.net", gatewaysManager.select(0).getHost());
+        assertEquals("moscow.bitmask.net", gatewaysManager.select(1).getHost());
+        assertEquals("pt.demo.bitmask.net", gatewaysManager.select(2).getHost());
+    }
+
+    @Test
+    public void testSelectN_selectObfs4FromPresortedGateways_returnsObfs4GatewaysInPresortedOrder() throws JSONException {
+        Provider provider = getProvider(null, null, null, null, null, null, "ptdemo_three_mixed_gateways.json", "ptdemo_three_mixed_gateways.geoip.json");
+
+        MockHelper.mockProviderObserver(provider);
+        //use openvpn, not pluggable transports
+        mockStatic(PreferenceHelper.class);
+        when(PreferenceHelper.getUsePluggableTransports(any(Context.class))).thenReturn(true);
+        GatewaysManager gatewaysManager = new GatewaysManager(mockContext);
+
+        assertEquals("moscow.bitmask.net", gatewaysManager.select(0).getHost());
+        assertEquals("pt.demo.bitmask.net", gatewaysManager.select(1).getHost());
+        assertNull(gatewaysManager.select(2));
+    }
+
+
     private String getJsonStringFor(String filename) throws IOException {
         return TestSetupHelper.getInputAsString(getClass().getClassLoader().getResourceAsStream(filename));
     }
diff --git a/app/src/test/resources/ptdemo_three_mixed_gateways.geoip.json b/app/src/test/resources/ptdemo_three_mixed_gateways.geoip.json
new file mode 100644
index 00000000..a72a85f5
--- /dev/null
+++ b/app/src/test/resources/ptdemo_three_mixed_gateways.geoip.json
@@ -0,0 +1,12 @@
+{
+  "ip":"51.158.144.32",
+  "cc":"FR",
+  "city":"Paris",
+  "lat":48.8628,
+  "lon":2.3292,
+  "gateways":[
+    "manila.bitmask.net",
+    "moscow.bitmask.net",
+    "pt.demo.bitmask.net"
+  ]
+}
\ No newline at end of file
-- 
cgit v1.2.3