From 2694ffa2dc2058239783aee47458f9c572b0d714 Mon Sep 17 00:00:00 2001 From: y5nw <37980625+y5nw@users.noreply.github.com> Date: Fri, 22 Mar 2024 21:59:04 +0100 Subject: [PATCH] Cleanly handle player mailling lists (#140) * Fix server hanging for certain recipient names * Disallow recursive maillist inclusion * Disallow sending to empty recipient * Complement testcases --- api.lua | 2 ++ api.spec.lua | 54 +++++++++++++++++++++++++++++++++++----------------- storage.lua | 51 ++++++++++++++++++++++++++++--------------------- 3 files changed, 68 insertions(+), 39 deletions(-) diff --git a/api.lua b/api.lua index ff4625b..541063f 100644 --- a/api.lua +++ b/api.lua @@ -51,6 +51,8 @@ function mail.send(m) table.insert(undeliverable_reason, reason) end return false, table.concat(undeliverable_reason, "\n") + elseif not next(recipients) then + return false, S("You did not specify any valid recipient.") end local extra = {} diff --git a/api.spec.lua b/api.spec.lua index 5c82898..56fc676 100644 --- a/api.spec.lua +++ b/api.spec.lua @@ -7,6 +7,12 @@ mail.register_recipient_handler(function(_, name) return false, "It works (?)" end end) +mail.update_maillist("player1", { + owner = "player1", + name = "recursive", + desc = "", + players = {"@recursive", "player1"}, +}, "recursive") local received_count = {} mail.register_on_player_receive(function(player) @@ -27,30 +33,44 @@ local function assert_inbox_count(player_name, count) assert(player_received == count, ("incorrect receive count: %d expected, got %d"):format(count, player_received)) end +local function assert_send(expected_success, ...) + local success, err = mail.send(...) + if expected_success then + assert(success, ("expected mail to be sent, got error message: %s"):format(err)) + assert(not err, ("unexpected message after sending mail: %s"):format(err)) + else + assert(not success, "expected mail to be rejected, mail was sent") + assert(type(err) == "string", ("expected error message, got datum of type %s"):format(type(err))) + end +end + mtt.register("send mail", function(callback) - -- send a mail to a list - local success, err = mail.send({from = "player1", to = "list/test", subject = "something", body = "blah"}) - assert(success) - assert(not err) - assert_inbox_count("player2", 1) - assert_inbox_count("player1", 0) + -- local maillists + assert_send(true, {from = "player1", to = "@recursive", subject = "hello recursion", body = "blah"}) + assert_inbox_count("player1", 1) assert(sent_count == 1) - -- send a second mail to the list and also the sender - success, err = mail.send({from = "player1", to = "list/test, alias/player1", subject = "something", body = "blah"}) - assert(success) - assert(not err) - assert_inbox_count("player2", 2) + -- do not allow empty recipients + assert_send(false, {from = "player1", to = "@doesnotexist", subject = "should not be sent", body = "blah"}) + assert(sent_count == 1) + + -- send a mail to a list + assert_send(true, {from = "player1", to = "list/test", subject = "something", body = "blah"}) + assert_inbox_count("player2", 1) assert_inbox_count("player1", 1) assert(sent_count == 2) - -- send a mail to list/reject - the mail should be rejected - success, err = mail.send({from = "player1", to = "list/reject", subject = "something", body = "NO"}) - assert(not success) - assert(type(err) == "string") + -- send a second mail to the list and also the sender + assert_send(true, {from = "player1", to = "list/test, alias/player1", subject = "something", body = "blah"}) assert_inbox_count("player2", 2) - assert_inbox_count("player1", 1) - assert(sent_count == 2) + assert_inbox_count("player1", 2) + assert(sent_count == 3) + + -- send a mail to list/reject - the mail should be rejected + assert_send(false, {from = "player1", to = "list/reject", subject = "something", body = "NO"}) + assert_inbox_count("player2", 2) + assert_inbox_count("player1", 2) + assert(sent_count == 3) callback() end) diff --git a/storage.lua b/storage.lua index 0348f35..e9e5e77 100644 --- a/storage.lua +++ b/storage.lua @@ -404,30 +404,37 @@ function mail.delete_maillist(playername, listname) end end -function mail.extractMaillists(receivers_string, maillists_owner) - local receivers = mail.parse_player_list(receivers_string) -- extracted receivers - - -- extract players from mailing lists - while string.find(receivers_string, "@") do - local globalReceivers = mail.parse_player_list(receivers_string) -- receivers including maillists - receivers = {} - for _, receiver in ipairs(globalReceivers) do - local receiverInfo = receiver:split("@") -- @maillist - if receiverInfo[1] and receiver == "@" .. receiverInfo[1] then - local maillist = mail.get_maillist_by_name(maillists_owner, receiverInfo[1]) - if maillist then - for _, playername in ipairs(maillist.players) do - table.insert(receivers, playername) - end - end - else -- in case of player - table.insert(receivers, receiver) - end - end - receivers_string = mail.concat_player_list(receivers) +local function extract_maillists_main(receivers, maillists_owner, expanded_receivers, seen) + if type(receivers) == "string" then + receivers = mail.parse_player_list(receivers) end - return receivers + for _, receiver in pairs(receivers) do + if seen[receiver] then + -- Do not add/expand this receiver as it is already seen + minetest.log("verbose", ("mail: ignoring duplicate receiver %q during maillist expansion"):format(receiver)) + elseif string.find(receiver, "^@") then + seen[receiver] = true + local listname = string.sub(receiver, 2) + local maillist = mail.get_maillist_by_name(maillists_owner, listname) + if maillist then + minetest.log("verbose", ("mail: expanding maillist %q"):format(listname)) + for _, entry in ipairs(maillist.players) do + extract_maillists_main(entry, maillists_owner, expanded_receivers, seen) + end + end + else + seen[receiver] = true + minetest.log("verbose", ("mail: adding %q to receiver list during maillist expansion"):format(receiver)) + table.insert(expanded_receivers, receiver) + end + end +end + +function mail.extractMaillists(receivers, maillists_owner) + local expanded_receivers = {} + extract_maillists_main(receivers, maillists_owner, expanded_receivers, {}) + return expanded_receivers end function mail.get_setting_default_value(key)