From 5fce6528905e8f6d035b8e70e95f0e93329b5665 Mon Sep 17 00:00:00 2001 From: Miguel Portilla Date: Wed, 16 Dec 2015 12:27:41 -0500 Subject: [PATCH] Improve sub and unsub errors (RIPD-702) --- src/ripple/protocol/ErrorCodes.h | 1 + src/ripple/protocol/impl/ErrorCodes.cpp | 1 + src/ripple/rpc/handlers/Subscribe.cpp | 173 ++++++++++-------------- src/ripple/rpc/handlers/Unsubscribe.cpp | 117 ++++++++-------- 4 files changed, 130 insertions(+), 162 deletions(-) diff --git a/src/ripple/protocol/ErrorCodes.h b/src/ripple/protocol/ErrorCodes.h index c93330277a..0034cb81b5 100644 --- a/src/ripple/protocol/ErrorCodes.h +++ b/src/ripple/protocol/ErrorCodes.h @@ -110,6 +110,7 @@ enum error_code_i rpcSRC_AMT_MALFORMED, rpcSRC_CUR_MALFORMED, rpcSRC_ISR_MALFORMED, + rpcSTREAM_MALFORMED, rpcATX_DEPRECATED, // Internal error (should never happen) diff --git a/src/ripple/protocol/impl/ErrorCodes.cpp b/src/ripple/protocol/impl/ErrorCodes.cpp index 7714bfef97..154ab165ef 100644 --- a/src/ripple/protocol/impl/ErrorCodes.cpp +++ b/src/ripple/protocol/impl/ErrorCodes.cpp @@ -110,6 +110,7 @@ public: add (rpcSRC_ISR_MALFORMED, "srcIsrMalformed", "Source issuer is malformed."); add (rpcSRC_MISSING, "srcMissing", "Source is missing."); add (rpcSRC_UNCLAIMED, "srcUnclaimed", "Source account is not claimed."); + add (rpcSTREAM_MALFORMED, "malformedStream", "Stream malformed."); add (rpcTOO_BUSY, "tooBusy", "The server is too busy to help you now."); add (rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found."); add (rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method."); diff --git a/src/ripple/rpc/handlers/Subscribe.cpp b/src/ripple/rpc/handlers/Subscribe.cpp index 0d92aa5d07..60dcfa735d 100644 --- a/src/ripple/rpc/handlers/Subscribe.cpp +++ b/src/ripple/rpc/handlers/Subscribe.cpp @@ -39,21 +39,19 @@ Json::Value doSubscribe (RPC::Context& context) InfoSub::pointer ispSub; Json::Value jvResult (Json::objectValue); - if (!context.infoSub && !context.params.isMember (jss::url)) + if (! context.infoSub && ! context.params.isMember(jss::url)) { // Must be a JSON-RPC call. - JLOG (context.j.info) - << "doSubscribe: RPC subscribe requires a url"; - + JLOG(context.j.info) << "doSubscribe: RPC subscribe requires a url"; return rpcError (rpcINVALID_PARAMS); } - if (context.params.isMember (jss::url)) + if (context.params.isMember(jss::url)) { if (context.role != Role::ADMIN) - return rpcError (rpcNO_PERMISSION); + return rpcError(rpcNO_PERMISSION); - std::string strUrl = context.params[jss::url].asString (); + std::string strUrl = context.params[jss::url].asString (); std::string strUsername = context.params.isMember (jss::url_username) ? context.params[jss::url_username].asString () : ""; std::string strPassword = context.params.isMember (jss::url_password) ? @@ -67,9 +65,8 @@ Json::Value doSubscribe (RPC::Context& context) if (context.params.isMember (jss::password)) strPassword = context.params[jss::password].asString (); - ispSub = context.netOps.findRpcSub (strUrl); - - if (!ispSub) + ispSub = context.netOps.findRpcSub(strUrl); + if (! ispSub) { JLOG (context.j.debug) << "doSubscribe: building: " << strUrl; @@ -103,118 +100,86 @@ Json::Value doSubscribe (RPC::Context& context) ispSub = context.infoSub; } - if (!context.params.isMember (jss::streams)) + if (context.params.isMember (jss::streams)) { - } - else if (!context.params[jss::streams].isArray ()) - { - JLOG (context.j.info) - << "doSubscribe: streams requires an array."; - - return rpcError (rpcINVALID_PARAMS); - } - else - { - for (auto& it: context.params[jss::streams]) + if (! context.params[jss::streams].isArray ()) { - if (it.isString ()) - { - std::string streamName = it.asString (); + JLOG (context.j.info) + << "doSubscribe: streams requires an array."; + return rpcError (rpcINVALID_PARAMS); + } - if (streamName == "server") - { - context.netOps.subServer (ispSub, jvResult, - context.role == Role::ADMIN); - } - else if (streamName == "ledger") - { - context.netOps.subLedger (ispSub, jvResult); - } - else if (streamName == "transactions") - { - context.netOps.subTransactions (ispSub); - } - else if (streamName == "transactions_proposed" - || streamName == "rt_transactions") // DEPRECATED - { - context.netOps.subRTTransactions (ispSub); - } - else if (streamName == "validations") - { - context.netOps.subValidations (ispSub); - } - else if (streamName == "peer_status") - { - if (context.role != Role::ADMIN) - jvResult[jss::error] = "noPermission"; - else - context.netOps.subPeerStatus (ispSub); - } - else - { - jvResult[jss::error] = "unknownStream"; - } + for (auto const& it: context.params[jss::streams]) + { + if (! it.isString()) + return rpcError(rpcSTREAM_MALFORMED); + + std::string streamName = it.asString (); + if (streamName == "server") + { + context.netOps.subServer (ispSub, jvResult, + context.role == Role::ADMIN); + } + else if (streamName == "ledger") + { + context.netOps.subLedger (ispSub, jvResult); + } + else if (streamName == "transactions") + { + context.netOps.subTransactions (ispSub); + } + else if (streamName == "transactions_proposed" || + streamName == "rt_transactions") // DEPRECATED + { + context.netOps.subRTTransactions (ispSub); + } + else if (streamName == "validations") + { + context.netOps.subValidations (ispSub); + } + else if (streamName == "peer_status") + { + if (context.role != Role::ADMIN) + return rpcError(rpcNO_PERMISSION); + context.netOps.subPeerStatus (ispSub); } else { - jvResult[jss::error] = "malformedStream"; + return rpcError(rpcSTREAM_MALFORMED); } } } - auto strAccountsProposed = - context.params.isMember (jss::accounts_proposed) - ? jss::accounts_proposed : jss::rt_accounts; // DEPRECATED + auto accountsProposed = context.params.isMember(jss::accounts_proposed) + ? jss::accounts_proposed : jss::rt_accounts; // DEPRECATED + if (context.params.isMember(accountsProposed)) + { + if (! context.params[accountsProposed].isArray()) + return rpcError(rpcINVALID_PARAMS); - if (!context.params.isMember (strAccountsProposed)) - { - } - else if (!context.params[strAccountsProposed].isArray ()) - { - return rpcError (rpcINVALID_PARAMS); - } - else - { - auto ids = RPC::parseAccountIds (context.params[strAccountsProposed]); - - if (ids.empty ()) - jvResult[jss::error] = "malformedAccount"; - else - context.netOps.subAccount (ispSub, ids, true); + auto ids = RPC::parseAccountIds(context.params[accountsProposed]); + if (ids.empty()) + return rpcError(rpcACT_MALFORMED); + context.netOps.subAccount(ispSub, ids, true); } - if (!context.params.isMember (jss::accounts)) + if (context.params.isMember(jss::accounts)) { - } - else if (!context.params[jss::accounts].isArray ()) - { - return rpcError (rpcINVALID_PARAMS); - } - else - { - auto ids = RPC::parseAccountIds (context.params[jss::accounts]); + if (! context.params[jss::accounts].isArray()) + return rpcError(rpcINVALID_PARAMS); - if (ids.empty ()) - { - jvResult[jss::error] = "malformedAccount"; - } - else - { - context.netOps.subAccount (ispSub, ids, false); - JLOG (context.j.debug) - << "doSubscribe: accounts: " << ids.size (); - } + auto ids = RPC::parseAccountIds(context.params[jss::accounts]); + if (ids.empty()) + return rpcError(rpcACT_MALFORMED); + context.netOps.subAccount(ispSub, ids, false); + JLOG(context.j.debug) << "doSubscribe: accounts: " << ids.size(); } - if (!context.params.isMember (jss::books)) - { - } - else if (!context.params[jss::books].isArray ()) - { - return rpcError (rpcINVALID_PARAMS); - } - else + if (context.params.isMember(jss::books)) { + if (! context.params[jss::books].isArray()) + return rpcError (rpcINVALID_PARAMS); + for (auto& j: context.params[jss::books]) { if (!j.isObject () diff --git a/src/ripple/rpc/handlers/Unsubscribe.cpp b/src/ripple/rpc/handlers/Unsubscribe.cpp index ee4d6fdcf2..2e3f894b37 100644 --- a/src/ripple/rpc/handlers/Unsubscribe.cpp +++ b/src/ripple/rpc/handlers/Unsubscribe.cpp @@ -37,98 +37,99 @@ Json::Value doUnsubscribe (RPC::Context& context) InfoSub::pointer ispSub; Json::Value jvResult (Json::objectValue); - if (!context.infoSub && !context.params.isMember (jss::url)) + if (! context.infoSub && ! context.params.isMember(jss::url)) { // Must be a JSON-RPC call. - return rpcError (rpcINVALID_PARAMS); + return rpcError(rpcINVALID_PARAMS); } - if (context.params.isMember (jss::url)) + if (context.params.isMember(jss::url)) { if (context.role != Role::ADMIN) - return rpcError (rpcNO_PERMISSION); + return rpcError(rpcNO_PERMISSION); - std::string strUrl = context.params[jss::url].asString (); - ispSub = context.netOps.findRpcSub (strUrl); - - if (!ispSub) + std::string strUrl = context.params[jss::url].asString (); + ispSub = context.netOps.findRpcSub (strUrl); + if (! ispSub) return jvResult; } else { - ispSub = context.infoSub; + ispSub = context.infoSub; } if (context.params.isMember (jss::streams)) { + if (! context.params[jss::streams].isArray ()) + return rpcError (rpcINVALID_PARAMS); + for (auto& it: context.params[jss::streams]) { - if (it.isString ()) + if (! it.isString()) + return rpcError(rpcSTREAM_MALFORMED); + + std::string streamName = it.asString (); + if (streamName == "server") { - std::string streamName = it.asString (); - - if (streamName == "server") - context.netOps.unsubServer (ispSub->getSeq ()); - - else if (streamName == "ledger") - context.netOps.unsubLedger (ispSub->getSeq ()); - - else if (streamName == "transactions") - context.netOps.unsubTransactions (ispSub->getSeq ()); - - else if (streamName == "transactions_proposed" - || streamName == "rt_transactions") // DEPRECATED - context.netOps.unsubRTTransactions (ispSub->getSeq ()); - - else if (streamName == "validations") - context.netOps.unsubValidations (ispSub->getSeq ()); - - else if (streamName == "peer_status") - context.netOps.unsubPeerStatus (ispSub->getSeq ()); - - else - jvResult[jss::error] = "Unknown stream: " + streamName; + context.netOps.unsubServer (ispSub->getSeq ()); + } + else if (streamName == "ledger") + { + context.netOps.unsubLedger (ispSub->getSeq ()); + } + else if (streamName == "transactions") + { + context.netOps.unsubTransactions (ispSub->getSeq ()); + } + else if (streamName == "transactions_proposed" + || streamName == "rt_transactions") // DEPRECATED + { + context.netOps.unsubRTTransactions (ispSub->getSeq ()); + } + else if (streamName == "validations") + { + context.netOps.unsubValidations (ispSub->getSeq ()); + } + else if (streamName == "peer_status") + { + context.netOps.unsubPeerStatus (ispSub->getSeq ()); } else { - jvResult[jss::error] = "malformedSteam"; + return rpcError(rpcSTREAM_MALFORMED); } } } - if (context.params.isMember (jss::accounts_proposed) - || context.params.isMember (jss::rt_accounts)) + auto accountsProposed = context.params.isMember(jss::accounts_proposed) + ? jss::accounts_proposed : jss::rt_accounts; // DEPRECATED + if (context.params.isMember(accountsProposed)) { - auto accounts = RPC::parseAccountIds ( - context.params.isMember (jss::accounts_proposed) - ? context.params[jss::accounts_proposed] - : context.params[jss::rt_accounts]); // DEPRECATED + if (! context.params[accountsProposed].isArray()) + return rpcError(rpcINVALID_PARAMS); - if (accounts.empty ()) - jvResult[jss::error] = "malformedAccount"; - else - context.netOps.unsubAccount (ispSub, accounts, true); + auto ids = RPC::parseAccountIds(context.params[accountsProposed]); + if (ids.empty()) + return rpcError(rpcACT_MALFORMED); + context.netOps.unsubAccount(ispSub, ids, true); } - if (context.params.isMember (jss::accounts)) + if (context.params.isMember(jss::accounts)) { - auto accounts = RPC::parseAccountIds (context.params[jss::accounts]); + if (! context.params[jss::accounts].isArray()) + return rpcError(rpcINVALID_PARAMS); - if (accounts.empty ()) - jvResult[jss::error] = "malformedAccount"; - else - context.netOps.unsubAccount (ispSub, accounts, false); + auto ids = RPC::parseAccountIds(context.params[jss::accounts]); + if (ids.empty()) + return rpcError(rpcACT_MALFORMED); + context.netOps.unsubAccount(ispSub, ids, false); } - if (!context.params.isMember (jss::books)) - { - } - else if (!context.params[jss::books].isArray ()) - { - return rpcError (rpcINVALID_PARAMS); - } - else + if (context.params.isMember(jss::books)) { + if (! context.params[jss::books].isArray()) + return rpcError(rpcINVALID_PARAMS); + for (auto& jv: context.params[jss::books]) { if (!jv.isObject ()