Redo OnSetEndPoint logic to fix duplicate clones (#1549).

7 files changed, 47 insertions, 46 deletions

diff --git a/include/inspsocket.h b/include/inspsocket.h
index e432f9c16..69fdaac8b 100644
--- a/include/inspsocket.h
+++ b/include/inspsocket.h
@@ -312,8 +312,9 @@ class CoreExport StreamSocket : public EventHandler
 	/** Called when the endpoint addresses are changed.
 	 * @param local The new local endpoint.
 	 * @param remote The new remote endpoint.
+	 * @return true if the connection is still open, false if it has been closed
 	 */
-	virtual void OnSetEndPoint(const irc::sockets::sockaddrs& local, const irc::sockets::sockaddrs& remote) { }
+	virtual bool OnSetEndPoint(const irc::sockets::sockaddrs& local, const irc::sockets::sockaddrs& remote);
 
 	/** Send the given data out the socket, either now or when writes unblock
 	 */
diff --git a/include/users.h b/include/users.h
index 469c17124..eaf400c67 100644
--- a/include/users.h
+++ b/include/users.h
@@ -380,9 +380,9 @@ class CoreExport User : public Extensible
 	/** Sets the client IP for this user
 	 * @return true if the conversion was successful
 	 */
-	virtual bool SetClientIP(const std::string& address, bool recheck_eline = true);
+	virtual bool SetClientIP(const std::string& address);
 
-	virtual void SetClientIP(const irc::sockets::sockaddrs& sa, bool recheck_eline = true);
+	virtual void SetClientIP(const irc::sockets::sockaddrs& sa);
 
 	/** Constructor
 	 * @throw CoreException if the UID allocated to the user already exists
@@ -689,7 +689,7 @@ class CoreExport UserIOHandler : public StreamSocket
 	{
 	}
 	void OnDataReady() CXX11_OVERRIDE;
-	void OnSetEndPoint(const irc::sockets::sockaddrs& local, const irc::sockets::sockaddrs& remote) CXX11_OVERRIDE;
+	bool OnSetEndPoint(const irc::sockets::sockaddrs& local, const irc::sockets::sockaddrs& remote) CXX11_OVERRIDE;
 	void OnError(BufferedSocketError error) CXX11_OVERRIDE;
 
 	/** Adds to the user's write buffer.
@@ -820,9 +820,9 @@ class CoreExport LocalUser : public User, public insp::intrusive_list_node<Local
 	 */
 	void SetClass(const std::string &explicit_name = "");
 
-	bool SetClientIP(const std::string& address, bool recheck_eline = true) CXX11_OVERRIDE;
+	bool SetClientIP(const std::string& address) CXX11_OVERRIDE;
 
-	void SetClientIP(const irc::sockets::sockaddrs& sa, bool recheck_eline = true) CXX11_OVERRIDE;
+	void SetClientIP(const irc::sockets::sockaddrs& sa) CXX11_OVERRIDE;
 
 	/** Send a NOTICE message from the local server to the user.
 	 * The message will be sent even if the user is connected to a remote server.
diff --git a/src/coremods/core_xline/core_xline.cpp b/src/coremods/core_xline/core_xline.cpp
index aa19bad34..c53834fc0 100644
--- a/src/coremods/core_xline/core_xline.cpp
+++ b/src/coremods/core_xline/core_xline.cpp
@@ -64,6 +64,15 @@ class CoreModXLine : public Module
 	{
 	}
 
+	void OnSetUserIP(LocalUser* user) CXX11_OVERRIDE
+	{
+		if (user->quitting)
+			return;
+
+		user->exempt = (ServerInstance->XLines->MatchesLine("E", user) != NULL);
+		user->CheckLines(true);
+	}
+
 	ModResult OnUserPreNick(LocalUser* user, const std::string& newnick) CXX11_OVERRIDE
 	{
 		// Check Q-lines (for local nick changes only, remote servers have our Q-lines to enforce themselves)
diff --git a/src/inspsocket.cpp b/src/inspsocket.cpp
index 69c427212..ebbff448e 100644
--- a/src/inspsocket.cpp
+++ b/src/inspsocket.cpp
@@ -358,6 +358,11 @@ void StreamSocket::FlushSendQ(SendQueue& sq)
 		}
 }
 
+bool StreamSocket::OnSetEndPoint(const irc::sockets::sockaddrs& local, const irc::sockets::sockaddrs& remote)
+{
+	return false;
+}
+
 void StreamSocket::WriteData(const std::string &data)
 {
 	if (fd < 0)
diff --git a/src/modules/m_cgiirc.cpp b/src/modules/m_cgiirc.cpp
index 4a5a4fb03..df8201bde 100644
--- a/src/modules/m_cgiirc.cpp
+++ b/src/modules/m_cgiirc.cpp
@@ -34,29 +34,6 @@ enum
 	RPL_WHOISGATEWAY = 350
 };
 
-// We need this method up here so that it can be accessed from anywhere
-static void ChangeIP(LocalUser* user, const irc::sockets::sockaddrs& sa)
-{
-	// Set the users IP address and make sure they are in the right clone pool.
-	ServerInstance->Users->RemoveCloneCounts(user);
-	user->SetClientIP(sa);
-	ServerInstance->Users->AddClone(user);
-	if (user->quitting)
-		return;
-
-	// Recheck the connect class.
-	user->MyClass = NULL;
-	user->SetClass();
-	user->CheckClass();
-	if (user->quitting)
-		return;
-
-	// Check if this user matches any XLines.
-	user->CheckLines(true);
-	if (user->quitting)
-		return;
-}
-
 // Encapsulates information about an ident host.
 class IdentHost
 {
@@ -215,7 +192,7 @@ class CommandWebIRC : public SplitCommand
 
 			// Set the IP address sent via WEBIRC. We ignore the hostname and lookup
 			// instead do our own DNS lookups because of unreliable gateways.
-			ChangeIP(user, ipaddr);
+			user->SetClientIP(ipaddr);
 			return CMD_SUCCESS;
 		}
 
@@ -391,7 +368,7 @@ class ModuleCgiIRC
 				user->uuid.c_str(), user->GetIPString().c_str(), address.addr().c_str(), user->ident.c_str(), newident.c_str());
 
 			user->ChangeIdent(newident);
-			ChangeIP(user, address);
+			user->SetClientIP(address);
 			break; 
 		}
 		return MOD_RES_PASSTHRU;
diff --git a/src/modules/m_haproxy.cpp b/src/modules/m_haproxy.cpp
index f61a39fdd..ee9079cbf 100644
--- a/src/modules/m_haproxy.cpp
+++ b/src/modules/m_haproxy.cpp
@@ -261,7 +261,8 @@ class HAProxyHook : public IOHookMiddle
 						break;
 				}
 
-				sock->OnSetEndPoint(server, client);
+				if (!sock->OnSetEndPoint(server, client))
+					return -1;
 
 				// Parse any available TLVs.
 				while (tlv_index < address_length)
diff --git a/src/users.cpp b/src/users.cpp
index f11a7a380..a0b9c9d03 100644
--- a/src/users.cpp
+++ b/src/users.cpp
@@ -318,10 +318,11 @@ void UserIOHandler::AddWriteBuf(const std::string &data)
 	WriteData(data);
 }
 
-void UserIOHandler::OnSetEndPoint(const irc::sockets::sockaddrs& server, const irc::sockets::sockaddrs& client)
+bool UserIOHandler::OnSetEndPoint(const irc::sockets::sockaddrs& server, const irc::sockets::sockaddrs& client)
 {
 	memcpy(&user->server_sa, &server, sizeof(irc::sockets::sockaddrs));
 	user->SetClientIP(client);
+	return !user->quitting;
 }
 
 void UserIOHandler::OnError(BufferedSocketError)
@@ -723,17 +724,17 @@ irc::sockets::cidr_mask User::GetCIDRMask()
 	return irc::sockets::cidr_mask(client_sa, range);
 }
 
-bool User::SetClientIP(const std::string& address, bool recheck_eline)
+bool User::SetClientIP(const std::string& address)
 {
 	irc::sockets::sockaddrs sa;
 	if (!irc::sockets::aptosa(address, client_sa.port(), sa))
 		return false;
 
-	User::SetClientIP(sa, recheck_eline);
+	User::SetClientIP(sa);
 	return true;
 }
 
-void User::SetClientIP(const irc::sockets::sockaddrs& sa, bool recheck_eline)
+void User::SetClientIP(const irc::sockets::sockaddrs& sa)
 {
 	const std::string oldip(GetIPString());
 	memcpy(&client_sa, &sa, sizeof(irc::sockets::sockaddrs));
@@ -746,26 +747,33 @@ void User::SetClientIP(const irc::sockets::sockaddrs& sa, bool recheck_eline)
 		ChangeDisplayedHost(GetIPString());
 }
 
-bool LocalUser::SetClientIP(const std::string& address, bool recheck_eline)
+bool LocalUser::SetClientIP(const std::string& address)
 {
 	irc::sockets::sockaddrs sa;
 	if (!irc::sockets::aptosa(address, client_sa.port(), sa))
 		return false;
 
-	LocalUser::SetClientIP(sa, recheck_eline);
+	LocalUser::SetClientIP(sa);
 	return true;
 }
 
-void LocalUser::SetClientIP(const irc::sockets::sockaddrs& sa, bool recheck_eline)
+void LocalUser::SetClientIP(const irc::sockets::sockaddrs& sa)
 {
-	if (sa != client_sa)
-	{
-		User::SetClientIP(sa);
-		if (recheck_eline)
-			this->exempt = (ServerInstance->XLines->MatchesLine("E", this) != NULL);
+	if (sa == client_sa)
+		return;
 
-		FOREACH_MOD(OnSetUserIP, (this));
-	}
+	ServerInstance->Users->RemoveCloneCounts(this);
+
+	User::SetClientIP(sa);
+
+	FOREACH_MOD(OnSetUserIP, (this));
+
+	ServerInstance->Users->AddClone(this);
+
+	// Recheck the connect class.
+	this->MyClass = NULL;
+	this->SetClass();
+	this->CheckClass();
 }
 
 void LocalUser::Write(const ClientProtocol::SerializedMessage& text)