Merge pull request #1607 from pi-hole/tweak/query_details

Provide regex ID for API
pi-hole · Sep 15, 2023 · 392db95 · 392db95
2 parents 9079c4e + 4f1a9dd
commit 392db95
Show file tree

Hide file tree

Showing 9 changed files with 121 additions and 91 deletions.
diff --git a/src/api/docs/content/specs/queries.yaml b/src/api/docs/content/specs/queries.yaml
@@ -208,12 +208,10 @@ components:
                   time:
                     type: number
                     description: Time until the response was received (ms, negative if N/A)
-              ttl:
-                type: integer
-                description: Query's Time-To-Live (TTL) value (`-1` if N/A)
               regex_id:
                 type: integer
-                description: ID of blocking regex (`-1` if N/A)
+                description: ID of regex (`NULL` if N/A)
+                nullable: true
               upstream:
                 type: string
                 description: IP or name + port of upstream server
@@ -231,8 +229,7 @@ components:
               reply:
                 type: "IP"
                 time: 19
-              ttl: -1
-              regex_id: -1
+              regex_id: NULL
               upstream: "localhost#5353"
               dbid: 112421354
             - time: 1581907871.583821
@@ -247,8 +244,7 @@ components:
               reply:
                 type: "IP"
                 time: 12.3
-              ttl: 7
-              regex_id: -1
+              regex_id: NULL
               upstream: "localhost#5353"
               dbid: 112421355
         cursor:

diff --git a/src/api/queries.c b/src/api/queries.c
@@ -174,7 +174,7 @@ int api_queries_suggestions(struct ftl_conn *api)
 	JSON_SEND_OBJECT(json);
 }
 
-#define QUERYSTR "SELECT q.id,timestamp,q.type,status,d.domain,f.forward,additional_info,reply_type,reply_time,dnssec,c.ip,c.name,a.content" // ttl, regex_id
+#define QUERYSTR "SELECT q.id,timestamp,q.type,status,d.domain,f.forward,additional_info,reply_type,reply_time,dnssec,c.ip,c.name,a.content,regex_id"
 // JOIN: Only return rows where there is a match in BOTH tables
 // LEFT JOIN: Return all rows from the left table, and the matched rows from the right table
 #define JOINSTR "JOIN client_by_id c ON q.client = c.id JOIN domain_by_id d ON q.domain = d.id LEFT JOIN forward_by_id f ON q.forward = f.id LEFT JOIN addinfo_by_id a ON a.id = q.additional_info"
@@ -692,8 +692,11 @@ int api_queries(struct ftl_conn *api)
 			JSON_ADD_NULL_TO_OBJECT(client, "name");
 		JSON_ADD_ITEM_TO_OBJECT(item, "client", client);
 
-		JSON_ADD_NUMBER_TO_OBJECT(item, "ttl", 0); // sqlite3_column_int(read_stmt, 12));
-		JSON_ADD_NUMBER_TO_OBJECT(item, "regex_id", 0); // sqlite3_column_int(read_stmt, 13));
+		// Add regex_id if it exists
+		if(sqlite3_column_type(read_stmt, 13) == SQLITE_INTEGER)
+			JSON_ADD_NUMBER_TO_OBJECT(item, "regex_id", sqlite3_column_int(read_stmt, 13));
+		else
+			JSON_ADD_NULL_TO_OBJECT(item, "regex_id");
 
 		const unsigned char *cname = NULL;
 		switch(query.status)

diff --git a/src/database/common.c b/src/database/common.c
@@ -478,6 +478,21 @@ void db_init(void)
 		dbversion = db_get_int(db, DB_VERSION);
 	}
 
+	// Update to version 13 if lower
+	if(dbversion < 13)
+	{
+		// Update to version 13: Add additional column for regex ID
+		log_info("Updating long-term database to version 13");
+		if(!add_query_storage_column_regex_id(db))
+		{
+			log_info("Additional records not generated, database not available");
+			dbclose(&db);
+			return;
+		}
+		// Get updated version
+		dbversion = db_get_int(db, DB_VERSION);
+	}
+
 	lock_shm();
 	import_aliasclients(db);
 	unlock_shm();

diff --git a/src/database/query-table.c b/src/database/query-table.c
@@ -710,6 +710,38 @@ bool add_query_storage_columns(sqlite3 *db)
 	return true;
 }
 
+bool add_query_storage_column_regex_id(sqlite3 *db)
+{
+	// Start transaction of database update
+	SQL_bool(db, "BEGIN TRANSACTION");
+
+	// Add additional column to the query_storage table
+	SQL_bool(db, "ALTER TABLE query_storage ADD COLUMN regex_id INTEGER");
+
+	// Update VIEW queries
+	SQL_bool(db, "DROP VIEW queries");
+	SQL_bool(db, "CREATE VIEW queries AS "
+	                     "SELECT id, timestamp, type, status, "
+	                       "CASE typeof(domain) WHEN 'integer' THEN (SELECT domain FROM domain_by_id d WHERE d.id = q.domain) ELSE domain END domain,"
+	                       "CASE typeof(client) WHEN 'integer' THEN (SELECT ip FROM client_by_id c WHERE c.id = q.client) ELSE client END client,"
+	                       "CASE typeof(forward) WHEN 'integer' THEN (SELECT forward FROM forward_by_id f WHERE f.id = q.forward) ELSE forward END forward,"
+	                       "CASE typeof(additional_info) WHEN 'integer' THEN (SELECT content FROM addinfo_by_id a WHERE a.id = q.additional_info) ELSE additional_info END additional_info, "
+	                       "reply_type, reply_time, dnssec, regex_id "
+	                       "FROM query_storage q");
+
+	// Update database version to 13
+	if(!db_set_FTL_property(db, DB_VERSION, 13))
+	{
+		log_err("add_query_storage_column_regex_id(): Failed to update database version!");
+		return false;
+	}
+
+	// Finish transaction
+	SQL_bool(db, "COMMIT");
+
+	return true;
+}
+
 bool optimize_queries_table(sqlite3 *db)
 {
 	// Start transaction of database update
@@ -1208,7 +1240,8 @@ bool queries_to_database(void)
 	                                 "(SELECT id FROM addinfo_by_id WHERE type = ?9 AND content = ?10),"
 	                                 "?11," \
 	                                 "?12," \
-	                                 "?13)", -1, SQLITE_PREPARE_PERSISTENT, &query_stmt, NULL);
+	                                 "?13," \
+	                                 "?14)", -1, SQLITE_PREPARE_PERSISTENT, &query_stmt, NULL);
 	if( rc != SQLITE_OK )
 	{
 		log_err("queries_to_database(query_storage) - SQL error step: %s", sqlite3_errstr(rc));
@@ -1380,10 +1413,14 @@ bool queries_to_database(void)
 			sqlite3_bind_null(query_stmt, 8);
 		}
 
+		// Get cache entry for this query
+		const int cacheID = findCacheID(query->domainID, query->clientID, query->type, false);
+		DNSCacheData *cache = cacheID < 0 ? NULL : getDNSCache(cacheID, true);
+
 		// ADDITIONAL_INFO
 		if(query->status == QUERY_GRAVITY_CNAME ||
-		query->status == QUERY_REGEX_CNAME ||
-		query->status == QUERY_DENYLIST_CNAME)
+		   query->status == QUERY_REGEX_CNAME ||
+		   query->status == QUERY_DENYLIST_CNAME)
 		{
 			// Save domain blocked during deep CNAME inspection
 			const char *cname = getCNAMEDomainString(query);
@@ -1403,30 +1440,23 @@ bool queries_to_database(void)
 				break;
 			}
 		}
-		else if(query->status == QUERY_REGEX)
+		else if(cache != NULL && query->status == QUERY_REGEX)
 		{
 			// Restore regex ID if applicable
-			const int cacheID = findCacheID(query->domainID, query->clientID, query->type, false);
-			DNSCacheData *cache = getDNSCache(cacheID, true);
-			if(cache != NULL)
+			sqlite3_bind_int(query_stmt, 9, ADDINFO_REGEX_ID);
+			sqlite3_bind_int(query_stmt, 10, cache->domainlist_id);
+
+			// Execute prepared addinfo statement and check if successful
+			sqlite3_bind_int(addinfo_stmt, 1, ADDINFO_REGEX_ID);
+			sqlite3_bind_int(addinfo_stmt, 2, cache->domainlist_id);
+			rc = sqlite3_step(addinfo_stmt);
+			sqlite3_clear_bindings(addinfo_stmt);
+			sqlite3_reset(addinfo_stmt);
+			if(rc != SQLITE_DONE)
 			{
-				sqlite3_bind_int(query_stmt, 9, ADDINFO_REGEX_ID);
-				sqlite3_bind_int(query_stmt, 10, cache->domainlist_id);
-
-				// Execute prepared addinfo statement and check if successful
-				sqlite3_bind_int(addinfo_stmt, 1, ADDINFO_REGEX_ID);
-				sqlite3_bind_int(addinfo_stmt, 2, cache->domainlist_id);
-				rc = sqlite3_step(addinfo_stmt);
-				sqlite3_clear_bindings(addinfo_stmt);
-				sqlite3_reset(addinfo_stmt);
-				if(rc != SQLITE_DONE)
-				{
-					log_err("Encountered error while trying to store addinfo");
-					break;
-				}
+				log_err("Encountered error while trying to store addinfo");
+				break;
 			}
-			else
-				sqlite3_bind_null(query_stmt, 9);
 		}
 		else
 		{
@@ -1449,25 +1479,13 @@ bool queries_to_database(void)
 		// DNSSEC
 		sqlite3_bind_int(query_stmt, 13, query->dnssec);
 
-	/*	// TTL
-		sqlite3_bind_int(query_stmt, 14, query->ttl);
-
 		// REGEX_ID
-		if(query->status == QUERY_REGEX)
-		{
-			// Restore regex ID if applicable
-			const int cacheID = findCacheID(query->domainID, query->clientID, query->type);
-			DNSCacheData *cache = getDNSCache(cacheID, true);
-			if(cache != NULL)
-				sqlite3_bind_int(query_stmt, 15, cache->deny_regex_id);
-			else
-				sqlite3_bind_null(query_stmt, 15);
-		}
+		if(cache != NULL && cache->domainlist_id > -1)
+			sqlite3_bind_int(query_stmt, 14, cache->domainlist_id);
 		else
-		{
-			sqlite3_bind_null(query_stmt, 15);
-		}
-	*/
+			// Not applicable, setting NULL
+			sqlite3_bind_null(query_stmt, 14);
+
 		// Step and check if successful
 		rc = sqlite3_step(query_stmt);
 		sqlite3_clear_bindings(query_stmt);

diff --git a/src/database/query-table.h b/src/database/query-table.h
@@ -23,7 +23,7 @@
                                                        "client TEXT NOT NULL, " \
                                                        "forward TEXT );"
 
-#define CREATE_QUERY_STORAGE_TABLE_V12 "CREATE TABLE query_storage ( id INTEGER PRIMARY KEY AUTOINCREMENT, " \
+#define CREATE_QUERY_STORAGE_TABLE_V13 "CREATE TABLE query_storage ( id INTEGER PRIMARY KEY AUTOINCREMENT, " \
                                                               "timestamp INTEGER NOT NULL, " \
                                                               "type INTEGER NOT NULL, " \
                                                               "status INTEGER NOT NULL, " \
@@ -33,9 +33,10 @@
                                                               "additional_info INTEGER, " \
                                                               "reply_type INTEGER, " \
                                                               "reply_time REAL, " \
-                                                              "dnssec INTEGER );"
+                                                              "dnssec INTEGER, " \
+                                                              "regex_id INTEGER );"
 
-#define CREATE_QUERIES_VIEW_V12 "CREATE VIEW queries AS " \
+#define CREATE_QUERIES_VIEW_V13 "CREATE VIEW queries AS " \
                                     "SELECT id, timestamp, type, status, " \
                                       "CASE typeof(domain) " \
                                         "WHEN 'integer' THEN (SELECT domain FROM domain_by_id d WHERE d.id = q.domain) ELSE domain END domain," \
@@ -45,7 +46,7 @@
                                         "WHEN 'integer' THEN (SELECT forward FROM forward_by_id f WHERE f.id = q.forward) ELSE forward END forward," \
                                       "CASE typeof(additional_info) "\
                                         "WHEN 'integer' THEN (SELECT content FROM addinfo_by_id a WHERE a.id = q.additional_info) ELSE additional_info END additional_info, " \
-                                      "reply_type, reply_time, dnssec FROM query_storage q"
+                                      "reply_type, reply_time, dnssec, regex_id FROM query_storage q"
 
 // Version 1
 #define CREATE_QUERIES_TIMESTAMP_INDEX		"CREATE INDEX idx_queries_timestamp ON queries (timestamp);"
@@ -76,12 +77,12 @@
 
 #ifdef QUERY_TABLE_PRIVATE
 const char *table_creation[] = {
-	CREATE_QUERY_STORAGE_TABLE_V12,
+	CREATE_QUERY_STORAGE_TABLE_V13,
 	CREATE_DOMAINS_BY_ID,
 	CREATE_CLIENTS_BY_ID,
 	CREATE_FORWARD_BY_ID,
 	CREATE_ADDINFO_BY_ID,
-	CREATE_QUERIES_VIEW_V12,
+	CREATE_QUERIES_VIEW_V13,
 };
 const char *index_creation[] = {
 	CREATE_QUERY_STORAGE_ID_INDEX,
@@ -124,5 +125,6 @@ bool queries_to_database(void);
 bool optimize_queries_table(sqlite3 *db);
 bool create_addinfo_table(sqlite3 *db);
 bool add_query_storage_columns(sqlite3 *db);
+bool add_query_storage_column_regex_id(sqlite3 *db);
 
 #endif //QUERY_TABLE_PRIVATE_H
diff --git a/src/datastructure.h b/src/datastructure.h
@@ -33,7 +33,6 @@ typedef struct {
 	int id; // the ID is a (signed) int in dnsmasq, so no need for a long int here
 	int CNAME_domainID; // only valid if query has a CNAME blocking status
 	int ede;
-	unsigned int ttl;
 	double response;
 	double timestamp;
 	int64_t db;

diff --git a/src/dnsmasq/rfc1035.c b/src/dnsmasq/rfc1035.c
@@ -1975,7 +1975,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 				(crecp->flags & F_HOSTS) &&
 				!is_same_net(crecp->addr.addr4, local_addr, local_netmask))
 			      continue;
-			    
+
 			    ans = 1;
 			    if (!dryrun)
 			      {
@@ -1994,7 +1994,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
 				    break;
 				  }
 				// **********************************************************************************
-				
+
 				if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, 
 							crec_ttl(crecp, now), NULL, type, C_IN, 
 							type == T_A ? "4" : "6", &crecp->addr))