Align the UI's shared-IP view with the runtime blocking decision (#759)

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

@@ -1189,17 +1189,26 @@ public String getQName(int uid, String ip) {
         }
     }
 
-    public Cursor getQAName(int uid, String ip, boolean alive) {
+    /**
+     * DNS evidence for an IP, freshest qname first, one row per qname.
+     *
+     * <p>Expired rows are always excluded. Both callers — the runtime
+     * blocking decision in {@code blockKnownTracker()} and the UI
+     * classification in {@code ServiceSinkhole.log()} — must answer "is this
+     * IP shared, and whose is it?" from the same row set. When the UI alone
+     * saw the full history (see issue #759), it drew the shared-IP marker and
+     * the ALLOWED/BLOCKED text from evidence the blocker had already
+     * discarded, so the log contradicted what actually happened.
+     */
+    public Cursor getQAName(int uid, String ip) {
         long now = new Date().getTime();
         lock.readLock().lock();
         try {
             if (readableDb == null)
                 readableDb = this.getReadableDatabase();
             SQLiteDatabase db = readableDb;
             String escapedIp = ip.replace("'", "''");
-            String aliveFilter = alive
-                    ? " AND (d.time IS NULL OR d.time + d.ttl >= " + now + ")"
-                    : "";
+            String aliveFilter = " AND (d.time IS NULL OR d.time + d.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

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

@@ -999,7 +999,7 @@ private void log(Packet packet, int connection, boolean interactive) {
 
             int uncertain = DatabaseHelper.ACCESS_UNCERTAIN_NONE;
             boolean isTracker = false;
-            try (Cursor lookup = dh.getQAName(packet.uid, packet.daddr, false)) {
+            try (Cursor lookup = dh.getQAName(packet.uid, packet.daddr)) {
                 uncertain = (lookup != null
                         && lookup.getCount() > 1) ? DatabaseHelper.ACCESS_UNCERTAIN_SHARED_IP
                                 : DatabaseHelper.ACCESS_UNCERTAIN_NONE;
@@ -2697,7 +2697,7 @@ private boolean blockKnownTracker(String daddr, int uid) {
                 boolean sawFirstRow = false;
                 boolean sawTrackerEvidence = false;
                 boolean sawNonTrackerEvidence = false;
-                try (Cursor lookup = dh.getQAName(uid, daddr, true)) {
+                try (Cursor lookup = dh.getQAName(uid, daddr)) {
                     // Loop through all fresh DNS candidates for this IP and only fail closed
                     // when ambiguous tracker blocking is enabled or the evidence is tracker-only.
                     if (lookup != null) {

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

@@ -20,17 +20,19 @@
 @RunWith(RobolectricTestRunner.class)
 public class DatabaseHelperDnsAttributionTest {
 
+    private static final long NOW = System.currentTimeMillis();
+
     @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));
+        dh.insertDns(rr(NOW - 2_000L, "aaa-old.example.com", "aaa-old.example.com", ip, 3600));
+        dh.insertDns(rr(NOW - 1_000L, "zzz-new.example.com", "zzz-new.example.com", ip, 3600));
 
-        try (Cursor c = dh.getQAName(-1, ip, false)) {
+        try (Cursor c = dh.getQAName(-1, ip)) {
             assertEquals(2, c.getCount());
             assertTrue(c.moveToFirst());
             assertEquals("zzz-new.example.com", c.getString(c.getColumnIndexOrThrow("qname")));
@@ -46,16 +48,16 @@ public void repeatedObservationsOfSameQnameCollapseToFreshestRow() {
 
         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));
+        dh.insertDns(rr(NOW - 5_000L, "tracker.example.com", "old-cname.example.com", ip, 3600));
+        dh.insertDns(rr(NOW - 1_000L, "tracker.example.com", "new-cname.example.com", ip, 3600));
 
-        try (Cursor c = dh.getQAName(-1, ip, false)) {
+        try (Cursor c = dh.getQAName(-1, ip)) {
             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")));
+            assertEquals(NOW - 1_000L, c.getLong(c.getColumnIndexOrThrow("time")));
         }
     }
 
@@ -65,15 +67,15 @@ public void caseVariantQnamesShareOneRowAndDoNotLookUncertain() {
         dh.clearDns();
 
         String ip = "203.0.113.25";
-        dh.insertDns(rr(1_000L, "Graph.Facebook.Com", "Alias.Example.Com", ip, 3600));
-        dh.insertDns(rr(5_000L, "graph.facebook.com", "alias.example.com", ip, 3600));
+        dh.insertDns(rr(NOW - 5_000L, "Graph.Facebook.Com", "Alias.Example.Com", ip, 3600));
+        dh.insertDns(rr(NOW - 1_000L, "graph.facebook.com", "alias.example.com", ip, 3600));
 
-        try (Cursor c = dh.getQAName(-1, ip, false)) {
+        try (Cursor c = dh.getQAName(-1, ip)) {
             assertEquals("case variants must share one stored qname", 1, c.getCount());
             assertTrue(c.moveToFirst());
             assertEquals("graph.facebook.com", c.getString(c.getColumnIndexOrThrow("qname")));
             assertEquals("alias.example.com", c.getString(c.getColumnIndexOrThrow("aname")));
-            assertEquals(5_000L, c.getLong(c.getColumnIndexOrThrow("time")));
+            assertEquals(NOW - 1_000L, c.getLong(c.getColumnIndexOrThrow("time")));
         }
     }
 
@@ -84,7 +86,7 @@ public void aliveFilterAppliesBeforeDedup() {
 
         String ip = "203.0.113.30";
         // The freshest observation has already expired; an older one is still
-        // alive. With alive=true the expired row must not shadow the alive one.
+        // alive. The expired row must not shadow the alive one.
         // Rows are inserted directly because insertDns() clamps the TTL to the
         // "ttl" preference floor, which would keep the fresh row alive.
         long now = System.currentTimeMillis();
@@ -97,13 +99,45 @@ public void aliveFilterAppliesBeforeDedup() {
                         + (now - 5_000) + ", 't2.example.com', 'expired-cname.example.com', '"
                         + ip + "', 1000)");
 
-        try (Cursor c = dh.getQAName(-1, ip, true)) {
+        try (Cursor c = dh.getQAName(-1, ip)) {
             assertEquals(1, c.getCount());
             assertTrue(c.moveToFirst());
             assertEquals("alive-cname.example.com", c.getString(c.getColumnIndexOrThrow("aname")));
         }
     }
 
+    /**
+     * Issue #759: an expired qname must not make an IP look shared. The UI
+     * derived its shared-IP marker from a row set that still contained
+     * expired evidence while the blocker had already dropped it, so the log
+     * flagged an ambiguity the blocking decision never saw.
+     */
+    @Test
+    public void expiredQnameDoesNotMakeIpLookShared() {
+        DatabaseHelper dh = DatabaseHelper.getInstance(RuntimeEnvironment.getApplication());
+        dh.clearDns();
+
+        String ip = "203.0.113.40";
+        // Two different qnames on one IP, but only one is still alive. Rows are
+        // inserted directly because insertDns() clamps the TTL to the "ttl"
+        // preference floor, which would keep the expired one alive.
+        long now = System.currentTimeMillis();
+        dh.getWritableDatabase().execSQL(
+                "INSERT INTO dns (time, qname, aname, resource, ttl) VALUES ("
+                        + (now - 10_000) + ", 'alive.example.com', 'alive.example.com', '"
+                        + ip + "', 3600000)");
+        dh.getWritableDatabase().execSQL(
+                "INSERT INTO dns (time, qname, aname, resource, ttl) VALUES ("
+                        + (now - 5_000) + ", 'expired.example.com', 'expired.example.com', '"
+                        + ip + "', 1000)");
+
+        try (Cursor c = dh.getQAName(-1, ip)) {
+            assertEquals("an expired qname must not count towards shared-IP", 1, c.getCount());
+            assertTrue(c.moveToFirst());
+            assertEquals("alive.example.com", c.getString(c.getColumnIndexOrThrow("qname")));
+        }
+    }
+
     private static ResourceRecord rr(long time, String qname, String aname, String resource, int ttl) {
         ResourceRecord rr = new ResourceRecord();
         rr.Time = time;