From 259df23ffb16efde4653a33557c95c3f045f15d4 Mon Sep 17 00:00:00 2001
From: Matthew Wild <mwild1@gmail.com>
Date: Wed, 23 Mar 2022 15:25:22 +0000
Subject: mod_s2s: Store real stanzas in session.sendq, rather than strings

This is the "right" thing to do. Strings were more memory-efficient, but
e.g. bypassed stanza filters at reconnection time. Also not being stanzas
prevents us from potential future work, such as merging sendq with mod_smacks.

Regarding performance: we should counter the probable negative effect of this
change with other positive changes that are desired anyway - e.g. a limit on
the size of the sendq, improved in-memory representation of stanzas, s2s
backoff (e.g. if a remote server is persistently unreachable, cache this
failure for a while and don't just keep forever queuing stanzas for it).
---
 plugins/mod_s2s.lua | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/plugins/mod_s2s.lua b/plugins/mod_s2s.lua
index 3ad0f521..b9cd5fcd 100644
--- a/plugins/mod_s2s.lua
+++ b/plugins/mod_s2s.lua
@@ -146,16 +146,14 @@ local function bounce_sendq(session, reason)
 	elseif type(reason) == "string" then
 		reason_text = reason;
 	end
-	for i, data in ipairs(sendq) do
-		local reply = data[2];
-		if reply and not(reply.attr.xmlns) and bouncy_stanzas[reply.name] then
-			reply.attr.type = "error";
-			reply:tag("error", {type = error_type, by = session.from_host})
-				:tag(condition, {xmlns = "urn:ietf:params:xml:ns:xmpp-stanzas"}):up();
-			if reason_text then
-				reply:tag("text", {xmlns = "urn:ietf:params:xml:ns:xmpp-stanzas"})
-					:text("Server-to-server connection failed: "..reason_text):up();
-			end
+	for i, stanza in ipairs(sendq) do
+		if not stanza.attr.xmlns and bouncy_stanzas[stanza.name] then
+			local reply = st.error_reply(
+				stanza,
+				error_type,
+				condition,
+				reason_text and ("Server-to-server connection failed: "..reason_text) or nil
+			);
 			core_process_stanza(dummy, reply);
 		end
 		sendq[i] = nil;
@@ -182,15 +180,11 @@ function route_to_existing_session(event)
 		(host.log or log)("debug", "trying to send over unauthed s2sout to "..to_host);
 
 		-- Queue stanza until we are able to send it
-		local queued_item = {
-			tostring(stanza),
-			stanza.attr.type ~= "error" and stanza.attr.type ~= "result" and st.reply(stanza);
-		};
 		if host.sendq then
-			t_insert(host.sendq, queued_item);
+			t_insert(host.sendq, st.clone(stanza));
 		else
 			-- luacheck: ignore 122
-			host.sendq = { queued_item };
+			host.sendq = { st.clone(stanza) };
 		end
 		host.log("debug", "stanza [%s] queued ", stanza.name);
 		return true;
@@ -215,7 +209,7 @@ function route_to_new_session(event)
 
 	-- Store in buffer
 	host_session.bounce_sendq = bounce_sendq;
-	host_session.sendq = { {tostring(stanza), stanza.attr.type ~= "error" and stanza.attr.type ~= "result" and st.reply(stanza)} };
+	host_session.sendq = { st.clone(stanza) };
 	log("debug", "stanza [%s] queued until connection complete", stanza.name);
 	-- FIXME Cleaner solution to passing extra data from resolvers to net.server
 	-- This mt-clone allows resolvers to add extra data, currently used for DANE TLSA records
@@ -324,8 +318,8 @@ function mark_connected(session)
 		if sendq then
 			session.log("debug", "sending %d queued stanzas across new outgoing connection to %s", #sendq, session.to_host);
 			local send = session.sends2s;
-			for i, data in ipairs(sendq) do
-				send(data[1]);
+			for i, stanza in ipairs(sendq) do
+				send(stanza);
 				sendq[i] = nil;
 			end
 			session.sendq = nil;
-- 
cgit v1.2.3