Add error messages to Resolver::OnError()

Add exception handling to several places that use Resolver (it can throw)
Remove Resolver::ProcessResult(), its now handled within the bowels of dns.cpp


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

include/dns.h

@@ -24,6 +24,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 #include "base.h"
 
 typedef std::pair<int,std::string> DNSResult;
+typedef std::pair<unsigned char*, std::string> DNSInfo;
 
 /**
  * Error types that class Resolver can emit to its error method.
@@ -33,7 +34,8 @@ enum ResolverError
 	RESOLVER_NOERROR	=	0,
 	RESOLVER_NSDOWN		= 	1,
 	RESOLVER_NXDOMAIN	=	2,
-	RESOLVER_NOTREADY	=	3
+	RESOLVER_NOTREADY	=	3,
+	RESOLVER_BADIP		=	4
 };
 
 
@@ -45,8 +47,8 @@ enum ResolverError
 class DNS : public Extensible
 {
  public:
-	int dns_getip4(const char* name);
+	int dns_getip(const char* name);
-	int dns_getname4(const insp_inaddr* ip);
+	int dns_getname(const insp_inaddr* ip);
 	DNSResult dns_getresult();
 	DNS();
 	~DNS();
@@ -54,7 +56,7 @@ class DNS : public Extensible
 
 /**
  * The Resolver class is a high-level abstraction for resolving DNS entries.
- * It can do forward and reverse IPv4 lookups, and when IPv6 is supported, will
+ * It can do forward and reverse IPv4 lookups, and where IPv6 is supported, will
  * also be able to do those, transparent of protocols. Module developers must
  * extend this class via inheritence, and then insert a pointer to their derived
  * class into the core using Server::AddResolver(). Once you have done this,
@@ -111,19 +113,14 @@ class Resolver : public Extensible
 	 * When your lookup completes, this method will be called.
 	 * @param result The resulting DNS lookup, either an IP address or a hostname.
 	 */
-	virtual void OnLookupComplete(const std::string &result);
+	virtual void OnLookupComplete(const std::string &result) = 0;
 	/**
 	 * If an error occurs (such as NXDOMAIN, no domain name found) then this method
 	 * will be called.
 	 * @param e A ResolverError enum containing the error type which has occured.
+	 * @param errormessage The error text of the error that occured.
 	 */
-	virtual void OnError(ResolverError e);
+	virtual void OnError(ResolverError e, const std::string &errormessage);
-	/**
-	 * This method is called by the core when the object's file descriptor is ready
-	 * for reading, and will then dispatch a call to either OnLookupComplete or
-	 * OnError. You should never call this method yourself.
-	 */
-	bool ProcessResult(const std::string &result);
 	/**
 	 * Returns the id value of this class. This is primarily used by the core
 	 * to determine where in various tables to place a pointer to your class, but it

include/users.h

@@ -79,7 +79,7 @@ class UserResolver : public Resolver
 	UserResolver(userrec* user, std::string to_resolve, bool forward);
 
 	void OnLookupComplete(const std::string &result);
-	void OnError(ResolverError e);
+	void OnError(ResolverError e, const std::string &errormessage);
 };
 
 

src/dns.cpp

@@ -56,6 +56,11 @@ enum QueryType
 	DNS_QRY_PTR	= 12
 };
 
+enum QueryInfo
+{
+	ERROR_MASK	= 0x10000
+};
+
 enum QueryFlags
 {
 	FLAGS_MASK_RD		= 0x01,
@@ -120,8 +125,8 @@ class dns_connection
 		delete[] res;
 	}
 
-	unsigned char*	result_ready(dns_header &h, int length);
+	DNSInfo	result_ready(dns_header &h, int length);
-	int		send_requests(const dns_header *header, const int length, QueryType qt);
+	int	send_requests(const dns_header *header, const int length, QueryType qt);
 };
 
 /*
@@ -313,7 +318,7 @@ int dns_build_query_payload(const char * const name, const unsigned short rr, co
 	return payloadpos + 4;
 }
 
-int DNS::dns_getip4(const char *name)
+int DNS::dns_getip(const char *name)
 {
 	dns_header h;
 	int id;
@@ -331,7 +336,7 @@ int DNS::dns_getip4(const char *name)
 	return id;
 }
 
-int DNS::dns_getname4(const insp_inaddr *ip)
+int DNS::dns_getname(const insp_inaddr *ip)
 {
 #ifdef IPV6
 	return -1;
@@ -393,34 +398,35 @@ DNSResult DNS::dns_getresult()
 		/* We don't delete c here, because its done later when needed */
 		connections.erase(n_iter);
         }
-	unsigned char* data = req->result_ready(header, length);
+	DNSInfo data = req->result_ready(header, length);
 	std::string resultstr;
 
-	if (data == NULL)
+	if (data.first == NULL)
 	{
-		resultstr = "";
+		delete req;
+		return std::make_pair(this_id | ERROR_MASK, data.second);
 	}
 	else
 	{
 		if (req->type == DNS_QRY_A)
 		{
 			char formatted[16];
-			snprintf(formatted,16,"%u.%u.%u.%u",data[0],data[1],data[2],data[3]);
+			snprintf(formatted,16,"%u.%u.%u.%u",data.first[0],data.first[1],data.first[2],data.first[3]);
 			resultstr = formatted;
 		}
 		else
 		{
-			resultstr = std::string((const char*)data);
+			resultstr = std::string((const char*)data.first);
 		}
-	}
 
-	delete req;
+		delete req;
-	return std::make_pair(this_id,resultstr);
+		return std::make_pair(this_id,resultstr);
+	}
 }
 
 /** A result is ready, process it
  */
-unsigned char* dns_connection::result_ready(dns_header &header, int length)
+DNSInfo dns_connection::result_ready(dns_header &header, int length)
 {
 	int i = 0;
 	int q = 0;
@@ -429,25 +435,17 @@ unsigned char* dns_connection::result_ready(dns_header &header, int length)
  	unsigned short p;
 					
 	if (!(header.flags1 & FLAGS_MASK_QR))
-	{
+		return std::make_pair((unsigned char*)NULL,"Not a query result");
-		log(DEBUG,"DNS: didnt get a query result");
+
-		return NULL;
-	}
 	if (header.flags1 & FLAGS_MASK_OPCODE)
-	{
+		return std::make_pair((unsigned char*)NULL,"Unexpected value in DNS reply packet");
-		log(DEBUG,"DNS: got an OPCODE and didnt want one");
+
-		return NULL;
-	}
 	if (header.flags2 & FLAGS_MASK_RCODE)
-	{
+		return std::make_pair((unsigned char*)NULL,"Internal server error (SERVFAIL)");
-		log(DEBUG,"DNS lookup failed due to SERVFAIL");
+
-		return NULL;
-	}
 	if (header.ancount < 1)
-	{
+		return std::make_pair((unsigned char*)NULL,"No resource records returned");
-		log(DEBUG,"DNS: no answers!");
+
-		return NULL;
-	}
 	length -= 12;
 	while ((unsigned int)q < header.qdcount && i < length)
 	{
@@ -488,9 +486,8 @@ unsigned char* dns_connection::result_ready(dns_header &header, int length)
 			}
 		}
 		if (length - i < 10)
-		{
+			return std::make_pair((unsigned char*)NULL,"Incorrectly sized DNS reply");
-			return NULL;
+
-		}
 		dns_fill_rr(&rr,&header.payload[i]);
 		i += 10;
 		if (rr.type != this->type)
@@ -508,23 +505,23 @@ unsigned char* dns_connection::result_ready(dns_header &header, int length)
 		break;
 	}
 	if ((unsigned int)curanswer == header.ancount)
-		return NULL;
+		return std::make_pair((unsigned char*)NULL,"No valid answers");
+
 	if (i + rr.rdlength > (unsigned int)length)
-		return NULL;
+		return std::make_pair((unsigned char*)NULL,"Resource record larger than stated");
+
 	if (rr.rdlength > 1023)
-		return NULL;
+		return std::make_pair((unsigned char*)NULL,"Resource record too large");
 
 	switch (rr.type)
 	{
 		case DNS_QRY_PTR:
-			log(DEBUG,"DNS: got a result of type DNS_QRY_PTR");
 			o = 0;
 			q = 0;
 			while (q == 0 && i < length && o + 256 < 1023)
 			{
 				if (header.payload[i] > 63)
 				{
-					log(DEBUG,"DNS: h.payload[i] > 63");
 					memcpy(&p,&header.payload[i],2);
 					i = ntohs(p) - 0xC000 - 12;
 				}
@@ -548,7 +545,6 @@ unsigned char* dns_connection::result_ready(dns_header &header, int length)
 			res[o] = '\0';
 		break;
 		case DNS_QRY_A:
-			log(DEBUG,"DNS: got a result of type DNS_QRY_A");
 			memcpy(res,&header.payload[i],rr.rdlength);
 			res[rr.rdlength] = '\0';
 			break;
@@ -557,7 +553,7 @@ unsigned char* dns_connection::result_ready(dns_header &header, int length)
 			res[rr.rdlength] = '\0';
 			break;
 	}
-	return res;
+	return std::make_pair(res,"No error");;
 }
 
 DNS::DNS()
@@ -573,7 +569,7 @@ Resolver::Resolver(const std::string &source, bool forward) : input(source), fwd
 	if (forward)
 	{
 		log(DEBUG,"Resolver: Forward lookup on %s",source.c_str());
-		this->myid = Res->dns_getip4(source.c_str());
+		this->myid = Res->dns_getip(source.c_str());
 	}
 	else
 	{
@@ -582,13 +578,19 @@ Resolver::Resolver(const std::string &source, bool forward) : input(source), fwd
 	        if (insp_aton(source.c_str(), &binip) > 0)
 		{
 			/* Valid ip address */
-	        	this->myid = Res->dns_getname4(&binip);
+	        	this->myid = Res->dns_getname(&binip);
+		}
+		else
+		{
+			this->OnError(RESOLVER_BADIP, "Bad IP address for reverse lookup");
+			throw ModuleException("Resolver: Bad IP address");
+			return;
 		}
 	}
 	if (this->myid == -1)
 	{
 		log(DEBUG,"Resolver::Resolver: Could not get an id!");
-		this->OnError(RESOLVER_NSDOWN);
+		this->OnError(RESOLVER_NSDOWN, "Nameserver is down");
 		throw ModuleException("Resolver: Couldnt get an id to make a request");
 		/* We shouldnt get here really */
 		return;
@@ -597,11 +599,11 @@ Resolver::Resolver(const std::string &source, bool forward) : input(source), fwd
 	log(DEBUG,"Resolver::Resolver: this->myid=%d",this->myid);
 }
 
-void Resolver::OnLookupComplete(const std::string &result)
+//void Resolver::OnLookupComplete(const std::string &result)
-{
+//{
-}
+//}
 
-void Resolver::OnError(ResolverError e)
+void Resolver::OnError(ResolverError e, const std::string &errormessage)
 {
 }
 
@@ -615,25 +617,6 @@ int Resolver::GetId()
 	return this->myid;
 }
 
-bool Resolver::ProcessResult(const std::string &result)
-{
-	log(DEBUG,"Resolver::ProcessResult");
-
-	if (!result.length())
-	{
-		log(DEBUG,"Resolver::OnError(RESOLVER_NXDOMAIN)");
-		this->OnError(RESOLVER_NXDOMAIN);
-		return false;
-	}
-	else
-	{
-
-		log(DEBUG,"Resolver::OnLookupComplete(%s)",result.c_str());
-		this->OnLookupComplete(result);
-		return true;
-	}
-}
-
 void dns_deal_with_classes(int fd)
 {
 	log(DEBUG,"dns_deal_with_classes(%d)",fd);
@@ -642,12 +625,27 @@ void dns_deal_with_classes(int fd)
 		DNSResult res = Res->dns_getresult();
 		if (res.first != -1)
 		{
-			log(DEBUG,"Result available, id=%d",res.first);
+			if (res.first & ERROR_MASK)
-			if (dns_classes[res.first])
 			{
-				dns_classes[res.first]->ProcessResult(res.second);
+				res.first -= ERROR_MASK;
-				delete dns_classes[res.first];
+
-				dns_classes[res.first] = NULL;
+				log(DEBUG,"Error available, id=%d",res.first);
+				if (dns_classes[res.first])
+				{
+					dns_classes[res.first]->OnError(RESOLVER_NXDOMAIN, res.second);
+					delete dns_classes[res.first];
+					dns_classes[res.first] = NULL;
+				}
+			}
+			else
+			{
+				log(DEBUG,"Result available, id=%d",res.first);
+				if (dns_classes[res.first])
+				{
+					dns_classes[res.first]->OnLookupComplete(res.second);
+					delete dns_classes[res.first];
+					dns_classes[res.first] = NULL;
+				}
 			}
 		}
 	}

src/modules/m_cgiirc.cpp

@@ -72,7 +72,7 @@ class CGIResolver : public Resolver
 		}
 	}
 
-	virtual void OnError(ResolverError e)
+	virtual void OnError(ResolverError e, const std::string &errormessage)
 	{
 		if ((them) && (them == fd_ref_table[theirfd]))
 		{

src/modules/m_spanningtree.cpp

@@ -3092,10 +3092,10 @@ class ServernameResolver : public Resolver
 		}
 	}
 
-	void OnError(ResolverError e)
+	void OnError(ResolverError e, const std::string &errormessage)
 	{
 		/* Ooops! */
-		WriteOpers("*** CONNECT: Error connecting \002%s\002: Unable to resolve hostname.",MyLink.Name.c_str());
+		WriteOpers("*** CONNECT: Error connecting \002%s\002: Unable to resolve hostname - %s",MyLink.Name.c_str(),errormessage.c_str());
 	}
 };
 
@@ -3114,9 +3114,9 @@ class SecurityIPResolver : public Resolver
 		ValidIPs.push_back(result);
 	}
 
-	void OnError(ResolverError e)
+	void OnError(ResolverError e, const std::string &errormessage)
 	{
-		log(DEBUG,"Could not resolve IP associated with Link '%s'!",MyLink.Name.c_str());
+		log(DEBUG,"Could not resolve IP associated with Link '%s': %s",MyLink.Name.c_str(),errormessage.c_str());
 	}
 };
 
@@ -3347,8 +3347,15 @@ void ReadConfiguration(bool rebind)
 			insp_inaddr binip;
 			if (insp_aton(L.IPAddr.c_str(), &binip) < 1)
 			{
-				SecurityIPResolver* sr = new SecurityIPResolver(L.IPAddr, L);
+				try
-				Srv->AddResolver(sr);
+				{
+					SecurityIPResolver* sr = new SecurityIPResolver(L.IPAddr, L);
+					Srv->AddResolver(sr);
+				}
+				catch (ModuleException& e)
+				{
+					log(DEBUG,"Error in resolver: %s",e.GetReason());
+				}
 			}
 
 			LinkBlocks.push_back(L);
@@ -3752,8 +3759,15 @@ class ModuleSpanningTree : public Module
 					}
 					else
 					{
-						ServernameResolver* snr = new ServernameResolver(x->IPAddr, *x);
+						try
-						Srv->AddResolver(snr);
+						{
+							ServernameResolver* snr = new ServernameResolver(x->IPAddr, *x);
+							Srv->AddResolver(snr);
+						}
+						catch (ModuleException& e)
+						{
+							log(DEBUG,"Error in resolver: %s",e.GetReason());
+						}
 					}
 
 				}
@@ -3826,8 +3840,15 @@ class ModuleSpanningTree : public Module
 					}
 					else
 					{
-						ServernameResolver* snr = new ServernameResolver(x->IPAddr, *x);
+						try
-						Srv->AddResolver(snr);
+						{
+							ServernameResolver* snr = new ServernameResolver(x->IPAddr, *x);
+							Srv->AddResolver(snr);
+						}
+						catch (ModuleException& e)
+						{
+							log(DEBUG,"Error in resolver: %s",e.GetReason());
+						}
 					}
 					return 1;
 				}

src/modules/m_testcommand.cpp

@@ -35,9 +35,9 @@ class MyResolver : public Resolver
 		log(DEBUG,"*** RESOLVER COMPLETED LOOKUP, IP IS: '%s'",result.c_str());
 	}
 
-	virtual void OnError(ResolverError e)
+	virtual void OnError(ResolverError e, const std::string &errormessage)
 	{
-		log(DEBUG,"*** RESOLVER GOT ERROR: %d",e);
+		log(DEBUG,"*** RESOLVER GOT ERROR: %d: %s",e,errormessage.c_str());
 	}
 };
 

src/users.cpp

@@ -137,8 +137,15 @@ bool userrec::ProcessNoticeMasks(const char *sm)
 void userrec::StartDNSLookup()
 {
 	log(DEBUG,"Commencing reverse lookup");
-	res_reverse = new UserResolver(this, insp_ntoa(this->ip4), false);
+	try
-	MyServer->AddResolver(res_reverse);
+	{
+		res_reverse = new UserResolver(this, insp_ntoa(this->ip4), false);
+		MyServer->AddResolver(res_reverse);
+	}
+	catch (ModuleException& e)
+	{
+		log(DEBUG,"Error in resolver: %s",e.GetReason());
+	}
 }
 
 UserResolver::UserResolver(userrec* user, std::string to_resolve, bool forward) : Resolver(to_resolve, forward), bound_user(user)
@@ -152,8 +159,15 @@ void UserResolver::OnLookupComplete(const std::string &result)
 	{
 		log(DEBUG,"Commencing forward lookup");
 		this->bound_user->stored_host = result;
-		bound_user->res_forward = new UserResolver(this->bound_user, result, true);
+		try
-		MyServer->AddResolver(bound_user->res_forward);
+		{
+			bound_user->res_forward = new UserResolver(this->bound_user, result, true);
+			MyServer->AddResolver(bound_user->res_forward);
+		}
+		catch (ModuleException& e)
+		{
+			log(DEBUG,"Error in resolver: %s",e.GetReason());
+		}
 	}
 	else if ((this->fwd) && (fd_ref_table[this->bound_fd] == this->bound_user))
 	{
@@ -180,7 +194,7 @@ void UserResolver::OnLookupComplete(const std::string &result)
 	}
 }
 
-void UserResolver::OnError(ResolverError e)
+void UserResolver::OnError(ResolverError e, const std::string &errormessage)
 {
 	if (fd_ref_table[this->bound_fd] == this->bound_user)
 	{