Add whitespace deletion in buildvarstring() so template can have a space.

Basically if a $variable is empty, and there is a space before it in the
template string then we delete that space.

May seem (or is) a bit over the top but this way the template stays clean,
and it may be used/useful in other places as well.

This is a behavior change, but I think we can live with it. One can opt-
out via BUILDVARSTRING_KEEP_SPACE_FOR_EMPTY_VAR.

include/struct.h

@@ -2781,6 +2781,7 @@ typedef enum JsonRpcError {
 #define BUILDVARSTRING_URLENCODE		0x1
 #define BUILDVARSTRING_XML			0x2
 #define BUILDVARSTRING_UNKNOWN_VAR_IS_EMPTY	0x4
+#define BUILDVARSTRING_KEEP_SPACE_FOR_EMPTY_VAR	0x8
 
 #endif /* __struct_include__ */
 

src/modules/quit.c

@@ -495,6 +495,7 @@ void _banned_client(Client *client, const char *bantype, const char *reason, con
 {
 	char buf[512];
 	char idbuf[64];
+	const char *banid;
 	char *fmt = global ? iConf.reject_message_gline : iConf.reject_message_kline;
 	const char *vars[7], *values[7];
 	MessageTag *mtags = NULL;
@@ -504,13 +505,18 @@ void _banned_client(Client *client, const char *bantype, const char *reason, con
 
 	RunHook(HOOKTYPE_BANNED_CLIENT, client, bantype, reason, global);
 
-	/* The " [ID: xxx]" fragment, empty when there is no id. Used both as the $banid
+	/* Create the tklid. We actually need two buffers:
-	 * reject-message variable and appended to the quit reason / real-quit-reason mtag.
+	 * 1) 'idbuf' is used in snprintf() in the quit reason and is " [ID: %s]" (or "")
+	 * 2) 'banid' is used by buildvarstring() and is just "[ID: %s]" (or "")
 	 */
 	if (!BadPtr(tklid))
+	{
 		snprintf(idbuf, sizeof(idbuf), " [ID: %s]", tklid);
-	else
+		banid = idbuf + 1;
+	} else {
 		idbuf[0] = '\0';
+		banid = idbuf;
+	}
 
 	/* This was: "You are not welcome on this %s. %s: %s. %s" but is now dynamic: */
 	vars[0] = "bantype";
@@ -524,7 +530,7 @@ void _banned_client(Client *client, const char *bantype, const char *reason, con
 	vars[4] = "ip";
 	values[4] = GetIP(client);
 	vars[5] = "banid";
-	values[5] = idbuf;
+	values[5] = banid;
 	vars[6] = NULL;
 	values[6] = NULL;
 	buildvarstring(fmt, buf, sizeof(buf), vars, values);

src/modules/spamreport.c

@@ -466,7 +466,7 @@ int _spamreport(Client *client, const char *ip, NameValuePrioList *details, cons
 		NameValuePrioList *list = NULL;
 		list = duplicate_nvplist(details);
 		add_nvplist(&list, -1, "ip", ip);
-		buildvarstring_nvp(s->url, urlbuf, sizeof(urlbuf), list, BUILDVARSTRING_URLENCODE|BUILDVARSTRING_UNKNOWN_VAR_IS_EMPTY);
+		buildvarstring_nvp(s->url, urlbuf, sizeof(urlbuf), list, BUILDVARSTRING_URLENCODE|BUILDVARSTRING_UNKNOWN_VAR_IS_EMPTY|BUILDVARSTRING_KEEP_SPACE_FOR_EMPTY_VAR);
 		url = urlbuf;
 		safe_free_nvplist(list);
 		if (s->http_method == HTTP_METHOD_POST)
@@ -491,7 +491,7 @@ int _spamreport(Client *client, const char *ip, NameValuePrioList *details, cons
 		         " <add ip='$ip' type='$type' comment='$comment'>\n"
 		         "</request>\n",
 		         find_nvplist(s->parameters, "staging") ? " staging='1'" : "");
-		buildvarstring_nvp(fmtstring, bodybuf, sizeof(bodybuf), list, BUILDVARSTRING_XML|BUILDVARSTRING_UNKNOWN_VAR_IS_EMPTY);
+		buildvarstring_nvp(fmtstring, bodybuf, sizeof(bodybuf), list, BUILDVARSTRING_XML|BUILDVARSTRING_UNKNOWN_VAR_IS_EMPTY|BUILDVARSTRING_KEEP_SPACE_FOR_EMPTY_VAR);
 		body = bodybuf;
 		safe_free_nvplist(list); // frees all the duplicated lists
 		add_nvplist(&headers, 0, "Content-Type", "text/xml");

src/support.c

@@ -1477,12 +1477,26 @@ void buildvarstring_nvp(const char *inbuf, char *outbuf, size_t len, NameValuePr
 						output = urlencode(output, outputbuf, sizeof(outputbuf));
 					if (flags & BUILDVARSTRING_XML)
 						output = xmlescape(output, outputbuf, sizeof(outputbuf));
+					if (!*output && !(flags & BUILDVARSTRING_KEEP_SPACE_FOR_EMPTY_VAR))
+					{
+						/* Empty value: eat one preceding space so "Something. $var"
+						 * becomes "Something." Opt out with KEEP_SPACE_FOR_EMPTY_VAR
+						 * (eg URL/XML). Only present-but-empty values are affected;
+						 * NULL values and unknown vars are left alone.
+						 */
+						if ((o > outbuf) && (o[-1] == ' '))
+						{
+							o--;
+							left++;
+						}
+					} else {
 						strlcpy(o, output, left);
 						left -= strlen(output); /* may become <0 */
 						if (left <= 0)
 							return; /* return - don't write \0 to 'o'. ensured by strlcpy already */
 						o += strlen(output); /* value entirely written */
 					}
+				}
 			} else
 			{
 				/* variable name does not exist */