Merge pull request #1832 from pi-hole/tweak/limit_history_clients

Limit number of clients returned by /api/history/clients

src/api/api.h

@@ -24,6 +24,7 @@
 int api_handler(struct mg_connection *conn, void *ignored);
 
 // Statistic methods
+int __attribute__((pure)) cmpdesc(const void *a, const void *b);
 int api_stats_summary(struct ftl_conn *api);
 int api_stats_query_types(struct ftl_conn *api);
 int api_stats_upstreams(struct ftl_conn *api);

src/api/docs/content/specs/config.yaml

@@ -426,6 +426,8 @@ components:
                         type: string
                     maxHistory:
                       type: integer
+                    maxClients:
+                      type: integer
                     allow_destructive:
                       type: boolean
                     temp:
@@ -700,6 +702,7 @@ components:
               excludeClients: [ '1.2.3.4', 'localhost', 'fe80::345' ]
               excludeDomains: [ 'google.de', 'pi-hole.net' ]
               maxHistory: 86400
+              maxClients: 10
               allow_destructive: true
               temp:
                 limit: 60.0

src/api/docs/content/specs/history.yaml

@@ -62,7 +62,15 @@ components:
           - Metrics
         operationId: "get_client_metrics"
         description: |
-          Request data needed to generate the \"Client activity over last 24 hours\" graph
+          Request data needed to generate the \"Client activity over last 24 hours\" graph.
+          This endpoint returns the top N clients, sorted by total number of queries within 24 hours. If N is set to 0, all clients will be returned.
+          The client name is only available if the client's IP address can be resolved to a hostname.
+
+          The last client returned is a special client that contains the total number of queries that were not sent by any of the other shown clients , i.e. queries that were sent by clients that are not in the top N. This client is always present, even if it has 0 queries and can be identified by the special name "other clients" (mind the space in the hostname) and the IP address "0.0.0.0".
+
+          Note that, due to privacy settings, the returned data may also be empty.
+        parameters:
+          - $ref: 'history.yaml#/components/parameters/clients/N'
         responses:
           '200':
             description: OK
@@ -142,6 +150,38 @@ components:
     client_history:
       type: object
       properties:
+        clients:
+          type: array
+          description: Data array
+          items:
+            type: object
+            properties:
+              name:
+                type: string
+                nullable: true
+                description: Client name
+              ip:
+                type: string
+                description: Client IP address
+              total:
+                type: integer
+                description: Total number of queries
+          example:
+            - name: localhost
+              ip: "127.0.0.1"
+              total: 13428
+            - name: ip6-localnet
+              ip: "::1"
+              total: 2100
+            - name: null
+              ip: "192.168.1.1"
+              total: 254
+            - name: "pi.hole"
+              ip: "::"
+              total: 29
+            - name: "other clients"
+              ip: "0.0.0.0"
+              total: 14
         history:
           type: array
           description: Data array
@@ -162,28 +202,22 @@ components:
                 - 12
                 - 65
                 - 67
+                - 9
+                - 5
             - timestamp: 1511820500.583821
               data:
                 - 1
                 - 35
                 - 63
-        clients:
-          type: array
-          description: Data array
-          items:
-            type: object
-            properties:
-              name:
-                type: string
-                nullable: true
-                description: Client name
-              ip:
-                type: string
-                description: Client IP address
-          example:
-            - name: localhost
-              ip: "127.0.0.1"
-            - name: ip6-localnet
-              ip: "::1"
-            - name: null
-              ip: "192.168.1.1"
+                - 20
+                - 9
+  parameters:
+    clients:
+      N:
+        in: query
+        description: Maximum number of clients to return, setting this to 0 will return all clients
+        name: N
+        schema:
+          type: integer
+        required: false
+        example: 20

src/api/history.c

@@ -66,13 +66,38 @@ int api_history_clients(struct ftl_conn *api)
 		JSON_SEND_OBJECT_UNLOCK(json);
 	}
 
+	// Get number of clients to return´
+	unsigned int Nc = min(counters->clients, config.webserver.api.maxClients.v.u16);
+	if(api->request->query_string != NULL)
+	{
+		// Does the user request a non-default number of clients
+		get_uint_var(api->request->query_string, "N", &Nc);
+
+		// Limit the number of clients to return to the number of
+		// clients to avoid possible overflows for very large N
+		// Also allow N=0 to return all clients
+		if((int)Nc > counters->clients || Nc == 0)
+			Nc = counters->clients;
+	}
+
+	// Lock shared memory
 	lock_shm();
 
 	// Get clients which the user doesn't want to see
 	// if skipclient[i] == true then this client should be hidden from
 	// returned data. We initialize it with false
 	bool *skipclient = calloc(counters->clients, sizeof(bool));
+	int *temparray = calloc(2*counters->clients, sizeof(int));
+	if(skipclient == NULL || temparray == NULL)
+	{
+		unlock_shm();
+		return send_json_error(api, 500, "internal_error",
+		                       "Failed to allocate memory for client history", NULL);
+	}
 
+	// Check if the user wants to exclude any clients, this code path is
+	// only taken if the user has configured the web interface to exclude
+	// clients (it will most often be skipped)
 	unsigned int excludeClients = cJSON_GetArraySize(config.webserver.api.excludeClients.v.json);
 	if(excludeClients > 0)
 	{
@@ -93,42 +118,72 @@ int api_history_clients(struct ftl_conn *api)
 		}
 	}
 
-	// Also skip clients included in others (in alias-clients)
+	// Skip clients included in others (in alias-clients)
 	for(int clientID = 0; clientID < counters->clients; clientID++)
 	{
 		// Get client pointer
 		const clientsData* client = getClient(clientID, true);
 		if(client == NULL)
 			continue;
+
+		// Check if this client should be skipped
 		if(!client->flags.aliasclient && client->aliasclient_id > -1)
 			skipclient[clientID] = true;
 	}
 
+	// Get MAX_CLIENTS clients with the highest number of queries
+	for(int clientID = 0; clientID < counters->clients; clientID++)
+	{
+		// Get client pointer
+		const clientsData* client = getClient(clientID, true);
+
+		// Skip invalid clients
+		if(client == NULL)
+			continue;
+
+		// Store clientID and number of queries in temporary array
+		temparray[2*clientID + 0] = clientID;
+		temparray[2*clientID + 1] = client->count;
+	}
+
+	// Sort temporary array
+	qsort(temparray, counters->clients, sizeof(int[2]), cmpdesc);
+
 	// Main return loop
 	cJSON *history = JSON_NEW_ARRAY();
+	int others_total = 0;
 	for(unsigned int slot = 0; slot < OVERTIME_SLOTS; slot++)
 	{
 		cJSON *item = JSON_NEW_OBJECT();
 		JSON_ADD_NUMBER_TO_OBJECT(item, "timestamp", overTime[slot].timestamp);
 
 		// Loop over clients to generate output to be sent to the client
 		cJSON *data = JSON_NEW_ARRAY();
-		for(int clientID = 0; clientID < counters->clients; clientID++)
+		int others = 0;
+		for(int id = 0; id < counters->clients; id++)
 		{
-			if(skipclient[clientID])
-				continue;
-
 			// Get client pointer
+			const int clientID = temparray[2*id + 0];
 			const clientsData* client = getClient(clientID, true);
 
-			// Skip invalid clients and also those managed by alias clients
-			if(client == NULL || client->aliasclient_id >= 0)
+			// Skip invalid (recycled) clients
+			if(client == NULL)
 				continue;
 
-			const int thisclient = client->overTime[slot];
+			// Skip clients which should be hidden and add them to the "others" counter.
+			// Also skip clients when we reached the maximum number of clients to return
+			if(skipclient[clientID] || id >= (int)Nc)
+			{
+				others += client->overTime[slot];
+				continue;
+			}
 
-			JSON_ADD_NUMBER_TO_ARRAY(data, thisclient);
+			JSON_ADD_NUMBER_TO_ARRAY(data, client->overTime[slot]);
 		}
+		// Add others as last element in the array
+		others_total += others;
+		JSON_ADD_NUMBER_TO_ARRAY(data, others);
+
 		JSON_ADD_ITEM_TO_OBJECT(item, "data", data);
 		JSON_ADD_ITEM_TO_ARRAY(history, item);
 	}
@@ -137,25 +192,40 @@ int api_history_clients(struct ftl_conn *api)
 
 	// Loop over clients to generate output to be sent to the client
 	cJSON *clients = JSON_NEW_ARRAY();
-	for(int clientID = 0; clientID < counters->clients; clientID++)
+	for(int id = 0; id < counters->clients; id++)
 	{
-		if(skipclient[clientID])
-			continue;
-
 		// Get client pointer
+		const int clientID = temparray[2*id + 0];
 		const clientsData* client = getClient(clientID, true);
+
+		// Skip invalid (recycled) clients
 		if(client == NULL)
 			continue;
 
+		// Skip clients which should be hidden. Also skip clients when
+		// we reached the maximum number of clients to return
+		if(skipclient[clientID] || id >= (int)Nc)
+			continue;
+
+		// Get client name and IP address
 		const char *client_ip = getstr(client->ippos);
 		const char *client_name = client->namepos != 0 ? getstr(client->namepos) : NULL;
 
+		// Create JSON object for this client
 		cJSON *item = JSON_NEW_OBJECT();
 		JSON_REF_STR_IN_OBJECT(item, "name", client_name);
 		JSON_REF_STR_IN_OBJECT(item, "ip", client_ip);
+		JSON_ADD_NUMBER_TO_OBJECT(item, "total", client->count);
 		JSON_ADD_ITEM_TO_ARRAY(clients, item);
 	}
 
+	// Add "others" client
+	cJSON *item = JSON_NEW_OBJECT();
+	JSON_REF_STR_IN_OBJECT(item, "name", "other clients");
+	JSON_REF_STR_IN_OBJECT(item, "ip", "0.0.0.0");
+	JSON_ADD_NUMBER_TO_OBJECT(item, "total", others_total);
+	JSON_ADD_ITEM_TO_ARRAY(clients, item);
+
 	// Unlock already here to avoid keeping the lock during JSON generation
 	// This is safe because we don't access any shared memory after this
 	// point and all strings in the JSON are references to idempotent shared
@@ -164,6 +234,7 @@ int api_history_clients(struct ftl_conn *api)
 
 	// Free memory
 	free(skipclient);
+	free(temparray);
 
 	JSON_ADD_ITEM_TO_OBJECT(json, "clients", clients);
 	JSON_SEND_OBJECT(json);

src/api/stats.c

@@ -42,7 +42,7 @@ static int __attribute__((pure)) cmpasc(const void *a, const void *b)
 } */
 
 // qsort subroutine, sort DESC
-static int __attribute__((pure)) cmpdesc(const void *a, const void *b)
+int __attribute__((pure)) cmpdesc(const void *a, const void *b)
 {
 	const int *elem1 = (int*)a;
 	const int *elem2 = (int*)b;
@@ -163,6 +163,7 @@ int api_stats_top_domains(struct ftl_conn *api)
 		return 0;
 	}
 
+
 	bool blocked = false; // Can be overwritten by query string
 	int count = 10;
 	// /api/stats/top_domains?blocked=true

src/config/config.c

@@ -972,6 +972,11 @@ void initConfig(struct config *conf)
 	conf->webserver.api.maxHistory.t = CONF_UINT;
 	conf->webserver.api.maxHistory.d.ui = MAXLOGAGE*3600;
 
+	conf->webserver.api.maxClients.k = "webserver.api.maxClients";
+	conf->webserver.api.maxClients.h = "Up to how many clients should be returned in the activity graph endpoint (/api/history/clients)?\n This setting can be overwritten at run-time using the parameter N";
+	conf->webserver.api.maxClients.t = CONF_UINT16;
+	conf->webserver.api.maxClients.d.u16 = 10;
+
 	conf->webserver.api.allow_destructive.k = "webserver.api.allow_destructive";
 	conf->webserver.api.allow_destructive.h = "Allow destructive API calls (e.g. deleting all queries, powering off the system, ...)";
 	conf->webserver.api.allow_destructive.t = CONF_BOOL;

src/config/config.h

@@ -245,6 +245,7 @@ struct config {
 			struct conf_item excludeClients;
 			struct conf_item excludeDomains;
 			struct conf_item maxHistory;
+			struct conf_item maxClients;
 			struct conf_item allow_destructive;
 			struct {
 				struct conf_item limit;

src/datastructure.c

@@ -314,7 +314,7 @@ int _findClientID(const char *clientIP, const bool count, const bool aliasclient
 	// Set all MAC address bytes to zero
 	client->hwlen = -1;
 	memset(client->hwaddr, 0, sizeof(client->hwaddr));
-	// This may be a alias-client, the ID is set elsewhere
+	// This may be an alias-client, the ID is set elsewhere
 	client->flags.aliasclient = aliasclient;
 	client->aliasclient_id = -1;
 

test/pihole.toml

@@ -693,6 +693,11 @@
     # 86400)
     maxHistory = 86400
 
+    # Up to how many clients should be returned in the activity graph endpoint
+    # (/api/history/clients)?
+    # This setting can be overwritten at run-time using the parameter N
+    maxClients = 10
+
     # Allow destructive API calls (e.g. deleting all queries, powering off the system, ...)
     allow_destructive = true