From 5406de35a39c935a19460da06bf3dcd3948a00d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?louiz=E2=80=99?= Date: Wed, 17 Aug 2016 20:44:00 +0200 Subject: On a client error, do not QUIT, just make the resource leave all channels This should fix #3205 --- src/bridge/bridge.cpp | 16 +++++++++++++++- src/bridge/bridge.hpp | 6 +++++- src/xmpp/biboumi_component.cpp | 17 +++++++++-------- tests/end_to_end/__main__.py | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/bridge/bridge.cpp b/src/bridge/bridge.cpp index 8323c77..f6fefd9 100644 --- a/src/bridge/bridge.cpp +++ b/src/bridge/bridge.cpp @@ -60,6 +60,20 @@ void Bridge::shutdown(const std::string& exit_message) } } +void Bridge::remove_resource(const std::string& resource, + const std::string& part_message) +{ + const auto resources_in_chan_copy = this->resources_in_chan; + for (const auto& chan_pair: resources_in_chan_copy) + { + const ChannelKey& channel_key = chan_pair.first; + const std::set& resources = chan_pair.second; + if (resources.count(resource)) + this->leave_irc_channel({std::get<0>(channel_key), std::get<1>(channel_key), {}}, + part_message, resource); + } +} + void Bridge::clean() { auto it = this->irc_clients.begin(); @@ -330,7 +344,7 @@ void Bridge::send_raw_message(const std::string& hostname, const std::string& bo irc->send_raw(body); } -void Bridge::leave_irc_channel(Iid&& iid, std::string&& status_message, const std::string& resource) +void Bridge::leave_irc_channel(Iid&& iid, const std::string& status_message, const std::string& resource) { IrcClient* irc = this->get_irc_client(iid.get_server()); const auto key = iid.to_tuple(); diff --git a/src/bridge/bridge.hpp b/src/bridge/bridge.hpp index a1ff3ad..bcad030 100644 --- a/src/bridge/bridge.hpp +++ b/src/bridge/bridge.hpp @@ -44,6 +44,10 @@ public: * QUIT all connected IRC servers. */ void shutdown(const std::string& exit_message); + /** + * PART the given resource from all the channels + */ + void remove_resource(const std::string& resource, const std::string& part_message); /** * Remove all inactive IrcClients */ @@ -70,7 +74,7 @@ public: void send_channel_message(const Iid& iid, const std::string& body); void send_private_message(const Iid& iid, const std::string& body, const std::string& type="PRIVMSG"); void send_raw_message(const std::string& hostname, const std::string& body); - void leave_irc_channel(Iid&& iid, std::string&& status_message, const std::string& resource); + void leave_irc_channel(Iid&& iid, const std::string& status_message, const std::string& resource); void send_irc_nick_change(const Iid& iid, const std::string& new_nick); void send_irc_kick(const Iid& iid, const std::string& target, const std::string& reason, const std::string& iq_id, const std::string& to_jid); diff --git a/src/xmpp/biboumi_component.cpp b/src/xmpp/biboumi_component.cpp index e7549df..cb9aac8 100644 --- a/src/xmpp/biboumi_component.cpp +++ b/src/xmpp/biboumi_component.cpp @@ -180,23 +180,24 @@ void BiboumiComponent::handle_presence(const Stanza& stanza) void BiboumiComponent::handle_message(const Stanza& stanza) { - std::string from = stanza.get_tag("from"); + std::string from_str = stanza.get_tag("from"); std::string id = stanza.get_tag("id"); std::string to_str = stanza.get_tag("to"); std::string type = stanza.get_tag("type"); - if (from.empty()) + if (from_str.empty()) return; if (type.empty()) type = "normal"; - Bridge* bridge = this->get_user_bridge(from); + Bridge* bridge = this->get_user_bridge(from_str); + Jid from(from_str); Jid to(to_str); Iid iid(to.local, bridge); std::string error_type("cancel"); std::string error_name("internal-server-error"); utils::ScopeGuard stanza_error([&](){ - this->send_stanza_error("message", from, to_str, id, + this->send_stanza_error("message", from_str, to_str, id, error_type, error_name, ""); }); const XmlNode* body = stanza.get_child("body", COMPONENT_NS); @@ -216,7 +217,7 @@ void BiboumiComponent::handle_message(const Stanza& stanza) { const XmlNode* error = stanza.get_child("error", COMPONENT_NS); // Only a set of errors are considered “fatal”. If we encounter one of - // them, we purge (we disconnect the user from all the IRC servers). + // them, we purge (we disconnect that resource from all the IRC servers) // We consider this to be true, unless the error condition is // specified and is not in the kickable_errors set bool kickable_error = true; @@ -227,7 +228,7 @@ void BiboumiComponent::handle_message(const Stanza& stanza) kickable_error = false; } if (kickable_error) - bridge->shutdown("Error from remote client"); + bridge->remove_resource(from.resource, "Error from remote client"); } else if (type == "chat") { @@ -268,10 +269,10 @@ void BiboumiComponent::handle_message(const Stanza& stanza) } else if (iid.type == Iid::Type::User) - this->send_invalid_user_error(to.local, from); + this->send_invalid_user_error(to.local, from_str); } catch (const IRCNotConnected& ex) { - this->send_stanza_error("message", from, to_str, id, + this->send_stanza_error("message", from_str, to_str, id, "cancel", "remote-server-not-found", "Not connected to IRC server "s + ex.hostname, true); diff --git a/tests/end_to_end/__main__.py b/tests/end_to_end/__main__.py index 4a3abfb..21718f6 100644 --- a/tests/end_to_end/__main__.py +++ b/tests/end_to_end/__main__.py @@ -1006,6 +1006,39 @@ if __name__ == '__main__': ""), partial(expect_stanza, "/message/body[text()='{nick_one} is already on channel #foo']") + ]), + Scenario("client_error", + [ + handshake_sequence(), + # First resource + partial(send_stanza, + ""), + connection_sequence("irc.localhost", '{jid_one}/{resource_one}'), + partial(expect_stanza, + "/message/body[text()='Mode #foo [+nt] by {irc_host_one}']"), + partial(expect_stanza, + ("/presence[@to='{jid_one}/{resource_one}'][@from='#foo%{irc_server_one}/{nick_one}']/muc_user:x/muc_user:item[@affiliation='admin'][@role='moderator']", + "/presence/muc_user:x/muc_user:status[@code='110']") + ), + partial(expect_stanza, "/message[@from='#foo%{irc_server_one}'][@type='groupchat']/subject[not(text())]"), + + # Second resource, same channel + partial(send_stanza, + ""), + partial(expect_stanza, + ("/presence[@to='{jid_one}/{resource_two}'][@from='#foo%{irc_server_one}/{nick_one}']/muc_user:x/muc_user:item[@affiliation='admin'][@role='moderator']", + "/presence/muc_user:x/muc_user:status[@code='110']") + ), + partial(expect_stanza, "/message[@from='#foo%{irc_server_one}'][@type='groupchat'][@to='{jid_one}/{resource_two}']/subject[not(text())]"), + + # Now the first resource has an error + partial(send_stanza, + ""), + # Receive a leave only to the leaving resource + partial(expect_stanza, + ("/presence[@type='unavailable'][@from='#foo%{irc_server_one}/{nick_one}'][@to='{jid_one}/{resource_one}']/muc_user:x/muc_user:status[@code='110']", + "/presence/status[text()='Biboumi note: 1 resources are still in this channel.']") + ), ]) ) -- cgit v1.2.3