Fix bug found in <connect> code, <connect:limit> was checked after finding a class that matched, not during - meaning that if they were locked out by <connect:limit>, they were given no second chance to be matched by a future <connect> (and that <connect> deny would not apply to them if necessary etc). Also tidy this up a *lot*, remove some of the nesting by (ab)using looping. This is a lot more understandable for me now.

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@9732 e03df62e-2008-0410-955e-edbf42e46eb7

1 files changed, 36 insertions, 23 deletions

diff --git a/src/users.cpp b/src/users.cpp
index b81cd6d91..fd99f12b7 100644
--- a/src/users.cpp
+++ b/src/users.cpp
@@ -1701,7 +1701,10 @@ ConnectClass* User::SetClass(const std::string &explicit_name)
 		{
 			ConnectClass* c = *i;
 
-			if (explicit_name == c->GetName() && !c->GetDisabled())
+			if (c->GetDisabled())
+				continue; // can't possibly match, removed from conf
+
+			if (explicit_name == c->GetName())
 			{
 				found = c;
 			}
@@ -1713,37 +1716,47 @@ ConnectClass* User::SetClass(const std::string &explicit_name)
 		{
 			ConnectClass* c = *i;
 
-			if (((match(this->GetIPString(),c->GetHost().c_str(),true)) || (match(this->host,c->GetHost().c_str()))))
+			/* check if host matches.. */
+			if (((!match(this->GetIPString(),c->GetHost().c_str(),true)) && (!match(this->host,c->GetHost().c_str()))))
 			{
-				if (c->GetPort())
-				{
-					if (this->GetPort() == c->GetPort() && !c->GetDisabled())
-					{
-						found = c;
-					}
-					else
-						continue;
-				}
-				else
+				continue;
+			}
+
+			/*
+			 * deny change if change will take class over the limit check it HERE, not after we found a matching class,
+			 * because we should attempt to find another class if this one doesn't match us. -- w00t
+			 */
+			if (c->limit && (c->RefCount + 1 >= c->limit))
+			{
+				ServerInstance->Logs->Log("USERS", DEBUG, "OOPS: Connect class limit (%lu) hit, denying", c->limit);
+				continue;
+			}
+
+			/* if it's disabled, we can't match this one. */
+			if (c->GetDisabled())
+				continue;
+
+			/* if it requires a port ... */
+			if (c->GetPort())
+			{
+				/* and our port doesn't match, fail. */
+				if (this->GetPort() != c->GetPort())
 				{
-					if (!c->GetDisabled())
-						found = c;
+					continue;
 				}
 			}
+
+			/* we match this class, BUT! we must keep checking in case a further class is type deny and also matches us. */
+			found = c;
 		}
 	}
 
-	/* ensure we don't fuck things up refcount wise, only remove them from a class if we find a new one :P */
+	/*
+	 * Okay, assuming we found a class that matches.. switch us into that class, keeping refcounts up to date.
+	 */
 	if (found)
 	{
-		/* deny change if change will take class over the limit */
-		if (found->limit && (found->RefCount + 1 >= found->limit))
-		{
-			ServerInstance->Logs->Log("USERS", DEBUG, "OOPS: Connect class limit (%lu) hit, denying", found->limit);
-			return this->MyClass;
-		}
-
-		/* should always be valid, but just in case .. */
+		/* only fiddle with refcounts if they are already in a class .. */
 		if (this->MyClass)
 		{
 			if (found == this->MyClass) // no point changing this shit :P