Fix s2s labeled-response batches

Previously, batches would end prematurely if they traveled through an
intermediate server before reaching their destination because the
intermediate server would send ENCAP ACK multiple times, causing the
pending server count to go down way too fast. Solve this by introducing
two things:

- Check that the direction of the incoming command matches the direction
  we'd be sending the ACK to (avoids sending additional ACKs for replies
  from remote servers that we pass along)
- Add a mask parameter to the tracking metadata for remote responses and
  only send an ACK if we match the mask. this way things like remote
  WHOIS don't generate ACKs from intermediate servers

Author: skizzerz

include/response.h

@@ -42,6 +42,8 @@ struct ResponseInfo
 	bool sent;
 	/* while this is >0, outgoing messages will not be tagged with label/batch even if they otherwise would */
 	int skip_tags;
+	/* for remote batches, the server mask dictating which servers we are expecting responses from */
+	char mask[HOSTLEN + 1];
 };
 
 /* state of an outgoing labeled-response */
@@ -57,9 +59,11 @@ void begin_local_response_batch(void);
 
 /* send a labeled-response BATCH to the incoming client for replies that will be generated at least partially
  * by remote servers. server_count must be the number of servers expected to send responses back.
- * If a batch is already opened, adds server_count to the number of servers we're expecting responses from.
+ * mask must be a string (potentially containing * and ? wildcards) that matches servers we need responses from.
+ * If a local batch is already opened, transforms it into a remote batch.
+ * Calling on an existing remote batch is an error.
  */
-void begin_remote_response_batch(int server_count);
+void begin_remote_response_batch(int server_count, const char *mask);
 
 /* Get details for a labeled-response batch with the specified batch ID */
 struct ResponseInfo *get_remote_response_batch(const char *batch);

ircd/response.c

@@ -25,6 +25,7 @@
 #include "client.h"
 #include "response.h"
 #include "send.h"
+#include "s_assert.h"
 #include "s_serv.h"
 
 /* number of seconds we'll wait for remote servers to finish sending their responses
@@ -93,7 +94,7 @@ begin_local_response_batch(void)
 }
 
 void
-begin_remote_response_batch(int server_count)
+begin_remote_response_batch(int server_count, const char *mask)
 {
 	if (outgoing_response_info == NULL || !MyConnect(outgoing_response_info->source_p))
 		return;
@@ -103,6 +104,7 @@ begin_remote_response_batch(int server_count)
 		/* creating a new remote response batch */
 		generate_batch_id(outgoing_response_info->batch, sizeof(outgoing_response_info->batch));
 		outgoing_response_info->remote_response = server_count;
+		rb_strlcpy(outgoing_response_info->mask, mask, sizeof(outgoing_response_info->mask));
 		outgoing_response_info->expires = rb_current_time() + RESPONSE_EXPIRY;
 		rb_dlinkAddAlloc(outgoing_response_info, &outgoing_response_info->source_p->localClient->pending_remote_responses);
 		rb_dictionary_add(pending_responses, outgoing_response_info->batch, outgoing_response_info);
@@ -111,16 +113,14 @@ begin_remote_response_batch(int server_count)
 	}
 	else
 	{
-		/* increase the number of servers we're expecting responses from */
-		outgoing_response_info->remote_response += server_count;
+		/* can only upgrade local batches to remote */
+		s_assert(outgoing_response_info->remote_response == 0);
 
-		/* if we're upgrading a local batch to a remote batch, add it to tracking places */
-		if (outgoing_response_info->expires == 0)
-		{
-			outgoing_response_info->expires = rb_current_time() + RESPONSE_EXPIRY;
-			rb_dlinkAddAlloc(outgoing_response_info, &outgoing_response_info->source_p->localClient->pending_remote_responses);
-			rb_dictionary_add(pending_responses, outgoing_response_info->batch, outgoing_response_info);
-		}
+		outgoing_response_info->remote_response += server_count;
+		rb_strlcpy(outgoing_response_info->mask, mask, sizeof(outgoing_response_info->mask));
+		outgoing_response_info->expires = rb_current_time() + RESPONSE_EXPIRY;
+		rb_dlinkAddAlloc(outgoing_response_info, &outgoing_response_info->source_p->localClient->pending_remote_responses);
+		rb_dictionary_add(pending_responses, outgoing_response_info->batch, outgoing_response_info);
 	}
 }
 
@@ -167,7 +167,7 @@ count_match_servs(struct Client *one, const char *mask, uint64_t cap, uint64_t n
 	RB_DLINK_FOREACH(ptr, global_serv_list.head)
 	{
 		struct Client *target_p = ptr->data;
-		if (target_p == &me)
+		if (IsMe(target_p))
 			continue;
 
 		if (target_p->from == one->from)

ircd/s_serv.c

@@ -246,7 +246,7 @@ hunt_server(struct Client *client_p, struct Client *source_p,
 		old = parv[server];
 		parv[server] = get_id(target_p, target_p);
 
-		begin_remote_response_batch(1);
+		begin_remote_response_batch(1, target_p->servptr->name);
 		sendto_one(target_p, command, get_id(source_p, target_p),
 			   parv[1], parv[2], parv[3], parv[4], parv[5], parv[6], parv[7], parv[8]);
 		parv[server] = old;

modules/cap_labeled_response.c

@@ -99,17 +99,25 @@ cap_labeled_response_incoming(void *data_)
 	}
 	else if (IsServer(data->client) && !strcmp(serv_response_tag, data->key) && !EmptyString(data->value))
 	{
-		/* tag value should be UID,batch_id */
-		char uid[IDLEN] = { 0 };
-		const char *batch_id = strchr(data->value, ',');
-		if (batch_id == NULL)
+		/* tag value should be UID,batch_id,mask */
+		char *buf = rb_strdup(data->value);
+		char *p;
+		const char *uid = rb_strtok_r(buf, ",", &p);
+		const char *batch_id = rb_strtok_r(NULL, ",", &p);
+		const char *mask = rb_strtok_r(NULL, ",", &p);
+
+		if (uid == NULL || batch_id == NULL || mask == NULL)
+		{
+			rb_free(buf);
 			return;
+		}
 
-		++batch_id;
-		memcpy(uid, data->value, sizeof(uid) - 1);
 		struct Client *client = find_id(uid);
 		if (client == NULL)
+		{
+			rb_free(buf);
 			return;
+		}
 
 		if (MyConnect(client))
 		{
@@ -120,15 +128,17 @@ cap_labeled_response_incoming(void *data_)
 				SetClientCap(outgoing_response_info->source_p, CLICAP_RECEIVE_LABEL);
 			}
 		}
-		else
+		else if (data->client == client->from && match(mask, me.name))
 		{
 			outgoing_response_info = rb_malloc(sizeof(struct ResponseInfo));
 			outgoing_response_info->source_p = client;
 			outgoing_response_info->client_p = client->from;
 			rb_strlcpy(outgoing_response_info->batch, batch_id, sizeof(outgoing_response_info->batch));
+			rb_strlcpy(outgoing_response_info->mask, mask, sizeof(outgoing_response_info->mask));
 			outgoing_response_info->sent = true;
 		}
 
+		rb_free(buf);
 		data->approved = MESSAGE_TAG_ALLOW;
 		data->capmask = CLICAP_SERVONLY;
 	}
@@ -174,8 +184,8 @@ cap_labeled_response_process(void *data_)
 	}
 	else if (outgoing_response_info != NULL && outgoing_response_info->remote_response > 0)
 	{
-		snprintf(response_tag_buf, sizeof(response_tag_buf), "%s,%s",
-			outgoing_response_info->source_p->id, outgoing_response_info->batch);
+		snprintf(response_tag_buf, sizeof(response_tag_buf), "%s,%s,%s",
+			outgoing_response_info->source_p->id, outgoing_response_info->batch, outgoing_response_info->mask);
 		msgbuf_append_tag(msgbuf, serv_response_tag, response_tag_buf, CLICAP_SERVONLY);
 	}
 }

modules/core/m_message.c

@@ -1074,7 +1074,7 @@ msg_client(enum message_type msgtype,
 	}
 	else
 	{
-		begin_remote_response_batch(1);
+		begin_remote_response_batch(1, target_p->servptr->name);
 		sendto_anywhere_tags(target_p, source_p, cmdname[msgtype],
 			msgtype == MESSAGE_TYPE_TAGMSG ? CAP_STAG : NOCAPS,
 			NOCAPS, msgbuf_p->n_tags, msgbuf_p->tags,
@@ -1094,7 +1094,7 @@ msg_client_targeted(enum message_type msgtype,
 {
 	s_assert(!IsMe(target_p) && IsServer(target_p));
 
-	begin_remote_response_batch(1);
+	begin_remote_response_batch(1, target_p->servptr->name);
 	sendto_one_tags(target_p,
 		msgtype == MESSAGE_TYPE_TAGMSG ? CAP_STAG : NOCAPS,
 		NOCAPS, msgbuf_p->n_tags, msgbuf_p->tags,

modules/core/m_modules.c

@@ -185,7 +185,7 @@ mo_modlist(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *sour
 
 	if(parc > 2)
 	{
-		begin_remote_response_batch(count_match_servs(source_p, parv[2], CAP_ENCAP, NOCAPS));
+		begin_remote_response_batch(count_match_servs(source_p, parv[2], CAP_ENCAP, NOCAPS), parv[2]);
 		sendto_match_servs(source_p, parv[2], CAP_ENCAP, NOCAPS,
 				"ENCAP %s MODLIST %s", parv[2], parv[1]);
 		if(match(parv[2], me.name) == 0)

modules/m_etrace.c

@@ -129,7 +129,7 @@ mo_etrace(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *sourc
 			{
 				if (!MyClient(target_p))
 				{
-					begin_remote_response_batch(1);
+					begin_remote_response_batch(1, target_p->servptr->name);
 					sendto_one(target_p, ":%s ENCAP %s ETRACE %s",
 						get_id(source_p, target_p),
 						target_p->servptr->name,

modules/m_grant.c

@@ -58,7 +58,7 @@ mo_grant(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source
 	}
 	else
 	{
-		begin_remote_response_batch(1);
+		begin_remote_response_batch(1, target_p->servptr->name);
 		sendto_one(target_p, ":%s ENCAP %s GRANT %s %s",
 				get_id(source_p, target_p), target_p->servptr->name,
 				get_id(target_p, target_p), parv[2]);

modules/m_list.c

@@ -310,7 +310,7 @@ mo_list(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source_
 	}
 
 	/* mark it as a "remote" response so we don't terminate the batch at the end of the command handler */
-	begin_remote_response_batch(1);
+	begin_remote_response_batch(1, "");
 	params->response_info = outgoing_response_info;
 
 	safelist_client_instantiate(source_p, params);

modules/m_privs.c

@@ -182,7 +182,7 @@ mo_privs(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source
 	}
 	else
 	{
-		begin_remote_response_batch(1);
+		begin_remote_response_batch(1, server_p->name);
 		sendto_one(server_p, ":%s ENCAP %s PRIVS %s",
 				get_id(source_p, server_p),
 				server_p->name,