Attribute shared-IP DNS evidence by recency, not alphabetically

app/src/main/java/eu/faircode/netguard/DatabaseHelper.java

@@ -1164,14 +1164,28 @@ public Cursor getQAName(int uid, String ip, boolean alive) {
             if (readableDb == null)
                 readableDb = this.getReadableDatabase();
             SQLiteDatabase db = readableDb;
-            // There is a segmented index on resource
-            String query = "SELECT d.qname, d.aname, d.time, d.ttl";
-            query += " FROM dns AS d";
-            query += " WHERE d.resource = '" + ip.replace("'", "''") + "'";
-            if (alive)
-                query += " AND (d.time IS NULL OR d.time + d.ttl >= " + now + ")";
-            query += " GROUP BY d.qname"; // remove duplicates
-            query += " ORDER BY d.qname";
+            String escapedIp = ip.replace("'", "''");
+            String aliveFilter = alive
+                    ? " AND (%1$s.time IS NULL OR %1$s.time + %1$s.ttl >= " + now + ")"
+                    : "";
+            // There is a segmented index on resource. A shared IP can carry
+            // DNS evidence for several qnames; keep only the most recently
+            // observed row per qname (dedup) and order qnames by recency, so
+            // the freshest resolution — most likely tied to the connection
+            // that's actually being made now — is attributed first instead
+            // of an alphabetically-first but possibly stale one.
+            String query = "SELECT d.qname, d.aname, d.time, d.ttl" +
+                    " FROM dns AS d" +
+                    " WHERE d.resource = '" + escapedIp + "'" +
+                    String.format(aliveFilter, "d") +
+                    " AND d.ID = (" +
+                    "   SELECT d2.ID FROM dns AS d2" +
+                    "   WHERE d2.resource = d.resource AND d2.qname = d.qname" +
+                    String.format(aliveFilter, "d2") +
+                    "   ORDER BY d2.time DESC, d2.ID DESC" +
+                    "   LIMIT 1" +
+                    " )" +
+                    " ORDER BY d.time DESC, d.ID DESC";
             return db.rawQuery(query, new String[] {});
         } finally {
             lock.readLock().unlock();

app/src/test/java/eu/faircode/netguard/DatabaseHelperDnsAttributionTest.java

@@ -0,0 +1,71 @@
+package eu.faircode.netguard;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import android.database.Cursor;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+
+/**
+ * Covers getQAName's ordering: when a shared IP carries DNS evidence for
+ * several qnames (see issue #655), the most recently observed qname should
+ * be attributed first rather than the alphabetically-first one, and a qname
+ * with several observed rows (e.g. distinct CNAME targets) should collapse
+ * to its single freshest row.
+ */
+@RunWith(RobolectricTestRunner.class)
+public class DatabaseHelperDnsAttributionTest {
+
+    @Test
+    public void mostRecentlyObservedQnameIsReturnedFirst() {
+        DatabaseHelper dh = DatabaseHelper.getInstance(RuntimeEnvironment.getApplication());
+        dh.clearDns();
+
+        String ip = "203.0.113.10";
+        // Alphabetically "aaa..." would sort first, but "zzz..." was resolved later.
+        dh.insertDns(rr(1_000L, "aaa-old.example.com", "aaa-old.example.com", ip, 3600));
+        dh.insertDns(rr(2_000L, "zzz-new.example.com", "zzz-new.example.com", ip, 3600));
+
+        try (Cursor c = dh.getQAName(-1, ip, false)) {
+            assertEquals(2, c.getCount());
+            assertTrue(c.moveToFirst());
+            assertEquals("zzz-new.example.com", c.getString(c.getColumnIndexOrThrow("qname")));
+            assertTrue(c.moveToNext());
+            assertEquals("aaa-old.example.com", c.getString(c.getColumnIndexOrThrow("qname")));
+        }
+    }
+
+    @Test
+    public void repeatedObservationsOfSameQnameCollapseToFreshestRow() {
+        DatabaseHelper dh = DatabaseHelper.getInstance(RuntimeEnvironment.getApplication());
+        dh.clearDns();
+
+        String ip = "203.0.113.20";
+        // Same qname, two distinct CNAME targets observed at different times.
+        dh.insertDns(rr(1_000L, "tracker.example.com", "old-cname.example.com", ip, 3600));
+        dh.insertDns(rr(5_000L, "tracker.example.com", "new-cname.example.com", ip, 3600));
+
+        try (Cursor c = dh.getQAName(-1, ip, false)) {
+            assertEquals("duplicate rows for the same qname must collapse to one", 1, c.getCount());
+            assertTrue(c.moveToFirst());
+            assertEquals("tracker.example.com", c.getString(c.getColumnIndexOrThrow("qname")));
+            assertEquals("the freshest row's aname should win",
+                    "new-cname.example.com", c.getString(c.getColumnIndexOrThrow("aname")));
+            assertEquals(5_000L, c.getLong(c.getColumnIndexOrThrow("time")));
+        }
+    }
+
+    private static ResourceRecord rr(long time, String qname, String aname, String resource, int ttl) {
+        ResourceRecord rr = new ResourceRecord();
+        rr.Time = time;
+        rr.QName = qname;
+        rr.AName = aname;
+        rr.Resource = resource;
+        rr.TTL = ttl;
+        return rr;
+    }
+}