Use RPC error responses correctly.

5 files changed, 96 insertions, 26 deletions

diff --git a/docs/RPC/xmlrpc.php b/docs/RPC/xmlrpc.php
index 90f07a944..eaa22cec9 100644
--- a/docs/RPC/xmlrpc.php
+++ b/docs/RPC/xmlrpc.php
@@ -85,7 +85,7 @@ class AnopeXMLRPC
     {
         $ret = $this->run("checkAuthentication", [$account, $pass]);
 
-        if ($ret && $ret["result"] == "Success") {
+        if ($ret && array_key_exists("account", $ret)) {
             return $ret["account"];
         }
 
diff --git a/include/modules/rpc.h b/include/modules/rpc.h
index 7171b73a8..76ecf3542 100644
--- a/include/modules/rpc.h
+++ b/include/modules/rpc.h
@@ -13,16 +13,32 @@
 class RPCRequest final
 {
 private:
+	std::optional<std::pair<int64_t, Anope::string>> error;
 	std::map<Anope::string, Anope::string> replies;
 
 public:
 	Anope::string name;
 	Anope::string id;
 	std::deque<Anope::string> data;
-	HTTPReply &r;
+	HTTPReply &reply;
+
+	RPCRequest(HTTPReply &r)
+		: reply(r)
+	{
+	}
+
+	inline void Error(uint64_t errcode, const Anope::string &errstr)
+	{
+		this->error.emplace(errcode, errstr);
+	}
+
+	inline void Reply(const Anope::string &dname, const Anope::string &ddata)
+	{
+		this->replies.emplace(dname, ddata);
+	}
+
+	inline const auto &GetError() { return this->error; }
 
-	RPCRequest(HTTPReply &_r) : r(_r) { }
-	inline void Reply(const Anope::string &dname, const Anope::string &ddata) { this->replies.emplace(dname, ddata); }
 	inline const auto &GetReplies() { return this->replies; }
 };
 
diff --git a/modules/rpc/jsonrpc.cpp b/modules/rpc/jsonrpc.cpp
index 9be352571..92a4765e9 100644
--- a/modules/rpc/jsonrpc.cpp
+++ b/modules/rpc/jsonrpc.cpp
@@ -25,7 +25,7 @@ class MyJSONRPCServiceInterface final
 private:
 	std::deque<RPCEvent *> events;
 
-	void SendError(HTTPReply &reply, int64_t code, const char *message, const Anope::string &id)
+	void SendError(HTTPReply &reply, int64_t code, const Anope::string &message, const Anope::string &id)
 	{
 		Log(LOG_DEBUG) << "JSON-RPC error " << code << ": " << message;
 
@@ -37,7 +37,7 @@ private:
 
 		auto *error = yyjson_mut_obj(doc);
 		yyjson_mut_obj_add_sint(doc, error, "code", code);
-		yyjson_mut_obj_add_str(doc, error, "message", message);
+		yyjson_mut_obj_add_strn(doc, error, "message", message.c_str(), message.length());
 
 		yyjson_mut_obj_add_val(doc, root, "error", error);
 		yyjson_mut_obj_add_str(doc, root, "jsonrpc", "2.0");
@@ -122,6 +122,12 @@ public:
 			if (!e->Run(this, client, request))
 				return false;
 
+			else if (request.GetError())
+			{
+				SendError(reply, request.GetError()->first, request.GetError()->second, id);
+				return true;
+			}
+
 			else if (!request.GetReplies().empty())
 			{
 				this->Reply(request);
@@ -160,7 +166,7 @@ public:
 		auto *json = yyjson_mut_write(doc, YYJSON_WRITE_ALLOW_INVALID_UNICODE | YYJSON_WRITE_NEWLINE_AT_END, nullptr);
 		if (json)
 		{
-			request.r.Write(json);
+			request.reply.Write(json);
 			free(json);
 		}
 		yyjson_mut_doc_free(doc);
diff --git a/modules/rpc/rpc_main.cpp b/modules/rpc/rpc_main.cpp
index 9704950c5..ab48068c2 100644
--- a/modules/rpc/rpc_main.cpp
+++ b/modules/rpc/rpc_main.cpp
@@ -21,20 +21,26 @@ class RPCIdentifyRequest final
 	Reference<RPCServiceInterface> xinterface;
 
 public:
-	RPCIdentifyRequest(Module *m, RPCRequest &req, HTTPClient *c, RPCServiceInterface *iface, const Anope::string &acc, const Anope::string &pass) : IdentifyRequest(m, acc, pass), request(req), repl(request.r), client(c), xinterface(iface) { }
+	RPCIdentifyRequest(Module *m, RPCRequest &req, HTTPClient *c, RPCServiceInterface *iface, const Anope::string &acc, const Anope::string &pass)
+		: IdentifyRequest(m, acc, pass)
+		, request(req)
+		, repl(request.reply)
+		, client(c)
+		, xinterface(iface)
+	{
+	}
 
 	void OnSuccess() override
 	{
 		if (!xinterface || !client)
 			return;
 
-		request.r = this->repl;
+		request.reply = this->repl;
 
-		request.Reply("result", "Success");
 		request.Reply("account", GetAccount());
 
 		xinterface->Reply(request);
-		client->SendReply(&request.r);
+		client->SendReply(&request.reply);
 	}
 
 	void OnFail() override
@@ -42,12 +48,12 @@ public:
 		if (!xinterface || !client)
 			return;
 
-		request.r = this->repl;
+		request.reply = this->repl;
 
-		request.Reply("error", "Invalid password");
+		request.Error(-32000, "Invalid password");
 
 		xinterface->Reply(request);
-		client->SendReply(&request.r);
+		client->SendReply(&request.reply);
 	}
 };
 
@@ -83,16 +89,14 @@ private:
 		Anope::string command = request.data.size() > 2 ? request.data[2] : "";
 
 		if (service.empty() || user.empty() || command.empty())
-			request.Reply("error", "Invalid parameters");
+			request.Error(-32602, "Invalid parameters");
 		else
 		{
 			BotInfo *bi = BotInfo::Find(service, true);
 			if (!bi)
-				request.Reply("error", "Invalid service");
+				request.Error(-32000, "Invalid service");
 			else
 			{
-				request.Reply("result", "Success");
-
 				NickAlias *na = NickAlias::Find(user);
 
 				Anope::string out;
@@ -127,7 +131,7 @@ private:
 		Anope::string password = request.data.size() > 1 ? request.data[1] : "";
 
 		if (username.empty() || password.empty())
-			request.Reply("error", "Invalid parameters");
+			request.Error(-32602, "Invalid parameters");
 		else
 		{
 			auto *req = new RPCIdentifyRequest(me, request, client, iface, username, password);
@@ -273,8 +277,6 @@ private:
 			return;
 
 		u->SendMessage(bi, message);
-
-		request.Reply("result", "Success");
 	}
 };
 
diff --git a/modules/rpc/xmlrpc.cpp b/modules/rpc/xmlrpc.cpp
index e67d88556..2ecf00410 100644
--- a/modules/rpc/xmlrpc.cpp
+++ b/modules/rpc/xmlrpc.cpp
@@ -187,12 +187,58 @@ public:
 		if (!request.id.empty())
 			request.Reply("id", request.id);
 
-		Anope::string r = "<?xml version=\"1.0\" encoding=\"iso-8859-1\"?>\n<methodResponse>\n<params>\n<param>\n<value>\n<struct>\n";
-		for (const auto &[name, value] : request.GetReplies())
-			r += "<member>\n<name>" + this->Sanitize(name) + "</name>\n<value>\n<string>" + this->Sanitize(value) + "</string>\n</value>\n</member>\n";
-		r += "</struct>\n</value>\n</param>\n</params>\n</methodResponse>";
+		Anope::string xml =
+			"<?xml version=\"1.0\" encoding=\"iso-8859-1\"?>\n"
+			"<methodResponse>\n";
 
-		request.r.Write(r);
+		if (request.GetError())
+		{
+			xml +=
+				"<fault>\n"
+				"	<value>\n"
+				"		<struct>\n"
+				"			<member>\n"
+				"				<name>faultCode</name>\n"
+				"				<value>\n"
+				"					<int>" + Anope::ToString(request.GetError()->first) + "</int>\n"
+				"				</value>\n"
+				"			</member>\n"
+				"			<member>\n"
+				"				<name>faultString</name>\n"
+				"				<value>\n"
+				"					<string>" + this->Sanitize(request.GetError()->second) + "</string>\n"
+				"				</value>\n"
+				"			</member>\n"
+				"		</struct>\n"
+				"	</value>\n"
+				"</fault>\n";
+		}
+		else
+		{
+			xml +=
+				"<params>\n"
+				"	<param>\n"
+				"		<value>\n"
+				"			<struct>\n";
+			for (const auto &[name, value] : request.GetReplies())
+			{
+				xml +=
+					"<member>\n"
+					"	<name>" + this->Sanitize(name) + "</name>\n"
+					"	<value>\n"
+					"		<string>" + this->Sanitize(value) + "</string>\n"
+					"	</value>\n"
+					"</member>\n";
+			}
+			xml +=
+				"			</struct>\n"
+				"		</value>\n"
+				"	</param>\n"
+				"</params>\n";
+		}
+		xml += "</methodResponse>";
+
+		request.reply.Write(xml);
 	}
 };