From 78076a69038be72102bcf90076f69f79ce78987c Mon Sep 17 00:00:00 2001 From: drlongle Date: Wed, 17 May 2023 02:22:10 +0200 Subject: [PATCH] fix!: Prevent API from accepting seed or public key for account (#4404) The API would allow seeds (and public keys) to be used in place of accounts at several locations in the API. For example, when calling account_info, you could pass `"account": "foo"`. The string "foo" is treated like a seed, so the method returns `actNotFound` (instead of `actMalformed`, as most developers would expect). In the early days, this was a convenience to make testing easier. However, it allows for poor security practices, so it is no longer a good idea. Allowing a secret or passphrase is now considered a bug. Previously, it was controlled by the `strict` option on some methods. With this commit, since the API does not interpret `account` as `seed`, the option `strict` is no longer needed and is removed. Removing this behavior from the API is a [breaking change](https://xrpl.org/request-formatting.html#breaking-changes). One could argue that it shouldn't be done without bumping the API version; however, in this instance, there is no evidence that anyone is using the API in the "legacy" way. Furthermore, it is a potential security hole, as it allows users to send secrets to places where they are not needed, where they could end up in logs, error messages, etc. There's no reason to take such a risk with a seed/secret, since only the public address is needed. Resolves: #3329, #3330, #4337 BREAKING CHANGE: Remove non-strict account parsing (#3330) --- src/ripple/app/main/Main.cpp | 10 +- src/ripple/net/impl/RPCCall.cpp | 26 +-- src/ripple/rpc/handlers/AccountChannels.cpp | 34 ++- .../rpc/handlers/AccountCurrenciesHandler.cpp | 13 +- src/ripple/rpc/handlers/AccountInfo.cpp | 18 +- src/ripple/rpc/handlers/AccountLines.cpp | 53 ++--- src/ripple/rpc/handlers/AccountObjects.cpp | 30 +-- src/ripple/rpc/handlers/AccountOffers.cpp | 13 +- src/ripple/rpc/handlers/DepositAuthorized.cpp | 24 +- src/ripple/rpc/handlers/GatewayBalances.cpp | 28 +-- src/ripple/rpc/handlers/NoRippleCheck.cpp | 14 +- src/ripple/rpc/handlers/OwnerInfo.cpp | 20 +- src/test/rpc/AccountCurrencies_test.cpp | 15 +- src/test/rpc/AccountInfo_test.cpp | 17 +- src/test/rpc/AccountLinesRPC_test.cpp | 8 +- src/test/rpc/AccountObjects_test.cpp | 2 +- src/test/rpc/AccountOffers_test.cpp | 8 +- src/test/rpc/NoRippleCheck_test.cpp | 4 +- src/test/rpc/OwnerInfo_test.cpp | 10 +- src/test/rpc/RPCCall_test.cpp | 207 +++++------------- 20 files changed, 203 insertions(+), 351 deletions(-) diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index f25b83fd51..6693ac0f7b 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -125,14 +125,12 @@ printHelp(const po::options_description& desc) << systemName() << "d [options] \n" << desc << std::endl << "Commands: \n" - " account_currencies [] [strict]\n" - " account_info ||| [] " - "[strict]\n" + " account_currencies []\n" + " account_info | []\n" " account_lines |\"\" []\n" " account_channels |\"\" []\n" - " account_objects [] [strict]\n" - " account_offers | [] " - "[strict]\n" + " account_objects []\n" + " account_offers | []\n" " account_tx accountID [ledger_index_min [ledger_index_max " "[limit " "]]] [binary]\n" diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index b5a167f76b..26e56b690f 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -775,11 +775,9 @@ private: return jvRequest; } - // owner_info | [strict] - // owner_info || [] [strict] - // account_info | [strict] - // account_info || [] [strict] - // account_offers | [] [strict] + // owner_info + // account_info [] + // account_offers [] Json::Value parseAccountItems(Json::Value const& jvParams) { @@ -895,10 +893,7 @@ private: // Parameters 0 and 1 are accounts if (i < 2) { - if (parseBase58( - TokenType::AccountPublic, strParam) || - parseBase58(strParam) || - parseGenericSeed(strParam)) + if (parseBase58(strParam)) { jvRequest[accFields[i]] = std::move(strParam); } @@ -924,16 +919,8 @@ private: { std::string strIdent = jvParams[0u].asString(); unsigned int iCursor = jvParams.size(); - bool bStrict = false; - if (iCursor >= 2 && jvParams[iCursor - 1] == jss::strict) - { - bStrict = true; - --iCursor; - } - - if (!parseBase58(TokenType::AccountPublic, strIdent) && - !parseBase58(strIdent) && !parseGenericSeed(strIdent)) + if (!parseBase58(strIdent)) return rpcError(rpcACT_MALFORMED); // Get info on account. @@ -941,9 +928,6 @@ private: jvRequest[jss::account] = strIdent; - if (bStrict) - jvRequest[jss::strict] = 1; - if (iCursor == 2 && !jvParseLedger(jvRequest, jvParams[1u].asString())) return rpcError(rpcLGR_IDX_MALFORMED); diff --git a/src/ripple/rpc/handlers/AccountChannels.cpp b/src/ripple/rpc/handlers/AccountChannels.cpp index 9e5c9ca2c4..8f39bef164 100644 --- a/src/ripple/rpc/handlers/AccountChannels.cpp +++ b/src/ripple/rpc/handlers/AccountChannels.cpp @@ -58,7 +58,7 @@ addChannel(Json::Value& jsonLines, SLE const& line) } // { -// account: | +// account: // ledger_hash : // ledger_index : // limit: integer // optional @@ -76,11 +76,12 @@ doAccountChannels(RPC::JsonContext& context) if (!ledger) return result; - std::string strIdent(params[jss::account].asString()); - AccountID accountID; - - if (auto const err = RPC::accountFromString(accountID, strIdent)) - return err; + auto id = parseBase58(params[jss::account].asString()); + if (!id) + { + return rpcError(rpcACT_MALFORMED); + } + AccountID const accountID{std::move(id.value())}; if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); @@ -88,14 +89,12 @@ doAccountChannels(RPC::JsonContext& context) std::string strDst; if (params.isMember(jss::destination_account)) strDst = params[jss::destination_account].asString(); - auto hasDst = !strDst.empty(); - AccountID raDstAccount; - if (hasDst) - { - if (auto const err = RPC::accountFromString(raDstAccount, strDst)) - return err; - } + auto const raDstAccount = [&]() -> std::optional { + return strDst.empty() ? std::nullopt : parseBase58(strDst); + }(); + if (!strDst.empty() && !raDstAccount) + return rpcError(rpcACT_MALFORMED); unsigned int limit; if (auto err = readLimitField(limit, RPC::Tuning::accountChannels, context)) @@ -109,10 +108,9 @@ doAccountChannels(RPC::JsonContext& context) { std::vector> items; AccountID const& accountID; - bool hasDst; - AccountID const& raDstAccount; + std::optional const& raDstAccount; }; - VisitData visitData = {{}, accountID, hasDst, raDstAccount}; + VisitData visitData = {{}, accountID, raDstAccount}; visitData.items.reserve(limit); uint256 startAfter = beast::zero; std::uint64_t startHint = 0; @@ -180,8 +178,8 @@ doAccountChannels(RPC::JsonContext& context) if (count <= limit && sleCur->getType() == ltPAYCHAN && (*sleCur)[sfAccount] == accountID && - (!visitData.hasDst || - visitData.raDstAccount == (*sleCur)[sfDestination])) + (!visitData.raDstAccount || + *visitData.raDstAccount == (*sleCur)[sfDestination])) { visitData.items.emplace_back(sleCur); } diff --git a/src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp b/src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp index d735e5976f..64956a7d0a 100644 --- a/src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp +++ b/src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp @@ -46,13 +46,14 @@ doAccountCurrencies(RPC::JsonContext& context) params.isMember(jss::account) ? params[jss::account].asString() : params[jss::ident].asString()); - bool const bStrict = - params.isMember(jss::strict) && params[jss::strict].asBool(); - // Get info on account. - AccountID accountID; // out param - if (auto jvAccepted = RPC::accountFromString(accountID, strIdent, bStrict)) - return jvAccepted; + auto id = parseBase58(strIdent); + if (!id) + { + RPC::inject_error(rpcACT_MALFORMED, result); + return result; + } + auto const accountID{std::move(id.value())}; if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); diff --git a/src/ripple/rpc/handlers/AccountInfo.cpp b/src/ripple/rpc/handlers/AccountInfo.cpp index ef130ef185..3af70324bc 100644 --- a/src/ripple/rpc/handlers/AccountInfo.cpp +++ b/src/ripple/rpc/handlers/AccountInfo.cpp @@ -34,8 +34,6 @@ namespace ripple { // { // account: , -// strict: // optional (default false) -// // if true only allow public keys and addresses. // ledger_hash : // ledger_index : // signer_lists : // optional (default false) @@ -67,15 +65,14 @@ doAccountInfo(RPC::JsonContext& context) if (!ledger) return result; - bool bStrict = params.isMember(jss::strict) && params[jss::strict].asBool(); - AccountID accountID; - // Get info on account. - - auto jvAccepted = RPC::accountFromString(accountID, strIdent, bStrict); - - if (jvAccepted) - return jvAccepted; + auto id = parseBase58(strIdent); + if (!id) + { + RPC::inject_error(rpcACT_MALFORMED, result); + return result; + } + auto const accountID{std::move(id.value())}; static constexpr std:: array, 9> @@ -113,6 +110,7 @@ doAccountInfo(RPC::JsonContext& context) return result; } + Json::Value jvAccepted(Json::objectValue); RPC::injectSLE(jvAccepted, *sleAccepted); result[jss::account_data] = jvAccepted; diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index adba2acaa7..f30a5d4b0a 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -30,17 +30,6 @@ namespace ripple { -struct VisitData -{ - std::vector items; - AccountID const& accountID; - bool hasPeer; - AccountID const& raPeerAccount; - - bool ignoreDefault; - uint32_t foundCount; -}; - void addLine(Json::Value& jsonLines, RPCTrustLine const& line) { @@ -76,7 +65,7 @@ addLine(Json::Value& jsonLines, RPCTrustLine const& line) } // { -// account: | +// account: // ledger_hash : // ledger_index : // limit: integer // optional @@ -96,15 +85,13 @@ doAccountLines(RPC::JsonContext& context) if (!ledger) return result; - std::string strIdent(params[jss::account].asString()); - AccountID accountID; - - if (auto jv = RPC::accountFromString(accountID, strIdent)) + auto id = parseBase58(params[jss::account].asString()); + if (!id) { - for (auto it = jv.begin(); it != jv.end(); ++it) - result[it.memberName()] = *it; + RPC::inject_error(rpcACT_MALFORMED, result); return result; } + auto const accountID{std::move(id.value())}; if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); @@ -112,17 +99,14 @@ doAccountLines(RPC::JsonContext& context) std::string strPeer; if (params.isMember(jss::peer)) strPeer = params[jss::peer].asString(); - auto hasPeer = !strPeer.empty(); - AccountID raPeerAccount; - if (hasPeer) + auto const raPeerAccount = [&]() -> std::optional { + return strPeer.empty() ? std::nullopt : parseBase58(strPeer); + }(); + if (!strPeer.empty() && !raPeerAccount) { - if (auto jv = RPC::accountFromString(raPeerAccount, strPeer)) - { - for (auto it = jv.begin(); it != jv.end(); ++it) - result[it.memberName()] = *it; - return result; - } + RPC::inject_error(rpcACT_MALFORMED, result); + return result; } unsigned int limit; @@ -138,8 +122,15 @@ doAccountLines(RPC::JsonContext& context) params[jss::ignore_default].asBool(); Json::Value& jsonLines(result[jss::lines] = Json::arrayValue); - VisitData visitData = { - {}, accountID, hasPeer, raPeerAccount, ignoreDefault, 0}; + struct VisitData + { + std::vector items; + AccountID const& accountID; + std::optional const& raPeerAccount; + bool ignoreDefault; + uint32_t foundCount; + }; + VisitData visitData = {{}, accountID, raPeerAccount, ignoreDefault, 0}; uint256 startAfter = beast::zero; std::uint64_t startHint = 0; @@ -227,8 +218,8 @@ doAccountLines(RPC::JsonContext& context) RPCTrustLine::makeItem(visitData.accountID, sleCur); if (line && - (!visitData.hasPeer || - visitData.raPeerAccount == + (!visitData.raPeerAccount || + *visitData.raPeerAccount == line->getAccountIDPeer())) { visitData.items.emplace_back(*line); diff --git a/src/ripple/rpc/handlers/AccountObjects.cpp b/src/ripple/rpc/handlers/AccountObjects.cpp index 687b871797..e8304c670d 100644 --- a/src/ripple/rpc/handlers/AccountObjects.cpp +++ b/src/ripple/rpc/handlers/AccountObjects.cpp @@ -39,7 +39,7 @@ namespace ripple { /** General RPC command that can retrieve objects in the account root. { - account: | + account: ledger_hash: // optional ledger_index: // optional type: // optional, defaults to all account objects types @@ -60,17 +60,13 @@ doAccountNFTs(RPC::JsonContext& context) if (ledger == nullptr) return result; - AccountID accountID; + auto id = parseBase58(params[jss::account].asString()); + if (!id) { - auto const strIdent = params[jss::account].asString(); - if (auto jv = RPC::accountFromString(accountID, strIdent)) - { - for (auto it = jv.begin(); it != jv.end(); ++it) - result[it.memberName()] = *it; - - return result; - } + RPC::inject_error(rpcACT_MALFORMED, result); + return result; } + auto const accountID{std::move(id.value())}; if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); @@ -177,17 +173,13 @@ doAccountObjects(RPC::JsonContext& context) if (ledger == nullptr) return result; - AccountID accountID; + auto const id = parseBase58(params[jss::account].asString()); + if (!id) { - auto const strIdent = params[jss::account].asString(); - if (auto jv = RPC::accountFromString(accountID, strIdent)) - { - for (auto it = jv.begin(); it != jv.end(); ++it) - result[it.memberName()] = *it; - - return result; - } + RPC::inject_error(rpcACT_MALFORMED, result); + return result; } + auto const accountID{std::move(id.value())}; if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); diff --git a/src/ripple/rpc/handlers/AccountOffers.cpp b/src/ripple/rpc/handlers/AccountOffers.cpp index 409d071fb0..1afd273255 100644 --- a/src/ripple/rpc/handlers/AccountOffers.cpp +++ b/src/ripple/rpc/handlers/AccountOffers.cpp @@ -47,7 +47,7 @@ appendOfferJson(std::shared_ptr const& offer, Json::Value& offers) }; // { -// account: | +// account: // ledger_hash : // ledger_index : // limit: integer // optional @@ -65,16 +65,13 @@ doAccountOffers(RPC::JsonContext& context) if (!ledger) return result; - std::string strIdent(params[jss::account].asString()); - AccountID accountID; - - if (auto jv = RPC::accountFromString(accountID, strIdent)) + auto id = parseBase58(params[jss::account].asString()); + if (!id) { - for (auto it = jv.begin(); it != jv.end(); ++it) - result[it.memberName()] = (*it); - + RPC::inject_error(rpcACT_MALFORMED, result); return result; } + auto const accountID{std::move(id.value())}; // Get info on account. result[jss::account] = toBase58(accountID); diff --git a/src/ripple/rpc/handlers/DepositAuthorized.cpp b/src/ripple/rpc/handlers/DepositAuthorized.cpp index a74db92437..a5c9c9a21f 100644 --- a/src/ripple/rpc/handlers/DepositAuthorized.cpp +++ b/src/ripple/rpc/handlers/DepositAuthorized.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include #include @@ -46,13 +47,10 @@ doDepositAuthorized(RPC::JsonContext& context) rpcINVALID_PARAMS, RPC::expected_field_message(jss::source_account, "a string")); - AccountID srcAcct; - { - Json::Value const jvAccepted = RPC::accountFromString( - srcAcct, params[jss::source_account].asString(), true); - if (jvAccepted) - return jvAccepted; - } + auto srcID = parseBase58(params[jss::source_account].asString()); + if (!srcID) + return rpcError(rpcACT_MALFORMED); + auto const srcAcct{std::move(srcID.value())}; // Validate destination_account. if (!params.isMember(jss::destination_account)) @@ -62,13 +60,11 @@ doDepositAuthorized(RPC::JsonContext& context) rpcINVALID_PARAMS, RPC::expected_field_message(jss::destination_account, "a string")); - AccountID dstAcct; - { - Json::Value const jvAccepted = RPC::accountFromString( - dstAcct, params[jss::destination_account].asString(), true); - if (jvAccepted) - return jvAccepted; - } + auto dstID = + parseBase58(params[jss::destination_account].asString()); + if (!dstID) + return rpcError(rpcACT_MALFORMED); + auto const dstAcct{std::move(dstID.value())}; // Validate ledger. std::shared_ptr ledger; diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 3a422c6e96..77cec496ed 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -68,16 +69,11 @@ doGatewayBalances(RPC::JsonContext& context) params.isMember(jss::account) ? params[jss::account].asString() : params[jss::ident].asString()); - bool const bStrict = - params.isMember(jss::strict) && params[jss::strict].asBool(); - // Get info on account. - AccountID accountID; - auto jvAccepted = RPC::accountFromString(accountID, strIdent, bStrict); - - if (jvAccepted) - return jvAccepted; - + auto id = parseBase58(strIdent); + if (!id) + return rpcError(rpcACT_MALFORMED); + auto const accountID{std::move(id.value())}; context.loadType = Resource::feeHighBurdenRPC; result[jss::account] = toBase58(accountID); @@ -90,19 +86,9 @@ doGatewayBalances(RPC::JsonContext& context) auto addHotWallet = [&hotWallets](Json::Value const& j) { if (j.isString()) { - auto const pk = parseBase58( - TokenType::AccountPublic, j.asString()); - if (pk) + if (auto id = parseBase58(j.asString()); id) { - hotWallets.insert(calcAccountID(*pk)); - return true; - } - - auto const id = parseBase58(j.asString()); - - if (id) - { - hotWallets.insert(*id); + hotWallets.insert(std::move(id.value())); return true; } } diff --git a/src/ripple/rpc/handlers/NoRippleCheck.cpp b/src/ripple/rpc/handlers/NoRippleCheck.cpp index 18156ea424..20137c985c 100644 --- a/src/ripple/rpc/handlers/NoRippleCheck.cpp +++ b/src/ripple/rpc/handlers/NoRippleCheck.cpp @@ -50,7 +50,7 @@ fillTransaction( } // { -// account: | +// account: // ledger_hash : // ledger_index : // limit: integer // optional, number of problems @@ -92,17 +92,13 @@ doNoRippleCheck(RPC::JsonContext& context) Json::Value& jvTransactions = transactions ? (result[jss::transactions] = Json::arrayValue) : dummy; - std::string strIdent(params[jss::account].asString()); - AccountID accountID; - - if (auto jv = RPC::accountFromString(accountID, strIdent)) + auto id = parseBase58(params[jss::account].asString()); + if (!id) { - for (auto it(jv.begin()); it != jv.end(); ++it) - result[it.memberName()] = *it; - + RPC::inject_error(rpcACT_MALFORMED, result); return result; } - + auto const accountID{std::move(id.value())}; auto const sle = ledger->read(keylet::account(accountID)); if (!sle) return rpcError(rpcACT_NOT_FOUND); diff --git a/src/ripple/rpc/handlers/OwnerInfo.cpp b/src/ripple/rpc/handlers/OwnerInfo.cpp index b336107ec2..2bd9f258da 100644 --- a/src/ripple/rpc/handlers/OwnerInfo.cpp +++ b/src/ripple/rpc/handlers/OwnerInfo.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -45,21 +46,16 @@ doOwnerInfo(RPC::JsonContext& context) Json::Value ret; // Get info on account. - auto const& closedLedger = context.ledgerMaster.getClosedLedger(); - AccountID accountID; - auto jAccepted = RPC::accountFromString(accountID, strIdent); - - ret[jss::accepted] = !jAccepted - ? context.netOps.getOwnerInfo(closedLedger, accountID) - : jAccepted; + std::optional const accountID = parseBase58(strIdent); + ret[jss::accepted] = accountID.has_value() + ? context.netOps.getOwnerInfo(closedLedger, accountID.value()) + : rpcError(rpcACT_MALFORMED); auto const& currentLedger = context.ledgerMaster.getCurrentLedger(); - auto jCurrent = RPC::accountFromString(accountID, strIdent); - - ret[jss::current] = !jCurrent - ? context.netOps.getOwnerInfo(currentLedger, accountID) - : jCurrent; + ret[jss::current] = accountID.has_value() + ? context.netOps.getOwnerInfo(currentLedger, *accountID) + : rpcError(rpcACT_MALFORMED); return ret; } diff --git a/src/test/rpc/AccountCurrencies_test.cpp b/src/test/rpc/AccountCurrencies_test.cpp index ac4adcf516..c3e46a3e66 100644 --- a/src/test/rpc/AccountCurrencies_test.cpp +++ b/src/test/rpc/AccountCurrencies_test.cpp @@ -56,11 +56,22 @@ class AccountCurrencies_test : public beast::unit_test::suite result[jss::error_message] == "Missing field 'account'."); } - { // strict mode, invalid bitcoin token + { Json::Value params; params[jss::account] = "llIIOO"; // these are invalid in bitcoin alphabet - params[jss::strict] = true; + auto const result = env.rpc( + "json", + "account_currencies", + boost::lexical_cast(params))[jss::result]; + BEAST_EXPECT(result[jss::error] == "actMalformed"); + BEAST_EXPECT(result[jss::error_message] == "Account malformed."); + } + + { + // Cannot use a seed as account + Json::Value params; + params[jss::account] = "Bob"; auto const result = env.rpc( "json", "account_currencies", diff --git a/src/test/rpc/AccountInfo_test.cpp b/src/test/rpc/AccountInfo_test.cpp index 0cda0632ed..6ec4740bac 100644 --- a/src/test/rpc/AccountInfo_test.cpp +++ b/src/test/rpc/AccountInfo_test.cpp @@ -53,7 +53,9 @@ public: "{\"account\": " "\"n94JNrQYkDrpt62bbSR7nVEhdyAvcJXRAsjEkFYyqRkh9SUTYEqV\"}"); BEAST_EXPECT( - info[jss::result][jss::error_message] == "Disallowed seed."); + info[jss::result][jss::error_code] == rpcACT_MALFORMED); + BEAST_EXPECT( + info[jss::result][jss::error_message] == "Account malformed."); } { // account_info with an account that's not in the ledger. @@ -61,10 +63,21 @@ public: auto const info = env.rpc( "json", "account_info", - std::string("{ ") + "\"account\": \"" + bogie.human() + "\"}"); + R"({ "account": ")" + bogie.human() + R"("})"); + BEAST_EXPECT( + info[jss::result][jss::error_code] == rpcACT_NOT_FOUND); BEAST_EXPECT( info[jss::result][jss::error_message] == "Account not found."); } + { + // Cannot use a seed as account + auto const info = + env.rpc("json", "account_info", R"({"account": "foo"})"); + BEAST_EXPECT( + info[jss::result][jss::error_code] == rpcACT_MALFORMED); + BEAST_EXPECT( + info[jss::result][jss::error_message] == "Account malformed."); + } } // Test the "signer_lists" argument in account_info. diff --git a/src/test/rpc/AccountLinesRPC_test.cpp b/src/test/rpc/AccountLinesRPC_test.cpp index 1b099f7b7b..04688156d1 100644 --- a/src/test/rpc/AccountLinesRPC_test.cpp +++ b/src/test/rpc/AccountLinesRPC_test.cpp @@ -53,7 +53,7 @@ public: R"("n9MJkEKHDhy5eTLuHUQeAAjo382frHNbFK4C8hcwN4nwM2SrLdBj"})"); BEAST_EXPECT( lines[jss::result][jss::error_message] == - RPC::make_error(rpcBAD_SEED)[jss::error_message]); + RPC::make_error(rpcACT_MALFORMED)[jss::error_message]); } Account const alice{"alice"}; { @@ -239,7 +239,7 @@ public: R"("n9MJkEKHDhy5eTLuHUQeAAjo382frHNbFK4C8hcwN4nwM2SrLdBj"})"); BEAST_EXPECT( lines[jss::result][jss::error_message] == - RPC::make_error(rpcBAD_SEED)[jss::error_message]); + RPC::make_error(rpcACT_MALFORMED)[jss::error_message]); } { // A negative limit should fail. @@ -815,7 +815,7 @@ public: R"("n9MJkEKHDhy5eTLuHUQeAAjo382frHNbFK4C8hcwN4nwM2SrLdBj"}})"); BEAST_EXPECT( lines[jss::error][jss::message] == - RPC::make_error(rpcBAD_SEED)[jss::error_message]); + RPC::make_error(rpcACT_MALFORMED)[jss::error_message]); BEAST_EXPECT( lines.isMember(jss::jsonrpc) && lines[jss::jsonrpc] == "2.0"); BEAST_EXPECT( @@ -1122,7 +1122,7 @@ public: R"("n9MJkEKHDhy5eTLuHUQeAAjo382frHNbFK4C8hcwN4nwM2SrLdBj"}})"); BEAST_EXPECT( lines[jss::error][jss::message] == - RPC::make_error(rpcBAD_SEED)[jss::error_message]); + RPC::make_error(rpcACT_MALFORMED)[jss::error_message]); BEAST_EXPECT( lines.isMember(jss::jsonrpc) && lines[jss::jsonrpc] == "2.0"); BEAST_EXPECT( diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 07fd5d6ddd..7de5b73671 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -131,7 +131,7 @@ public: "n94JNrQYkDrpt62bbSR7nVEhdyAvcJXRAsjEkFYyqRkh9SUTYEqV"; auto resp = env.rpc("json", "account_objects", to_string(params)); BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Disallowed seed."); + resp[jss::result][jss::error_message] == "Account malformed."); } // test error on account that's not in the ledger. { diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index f4ad0a7259..a90566d9c3 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -248,15 +248,15 @@ public: "json", "account_offers", jvParams.toStyledString())[jss::result]; - BEAST_EXPECT(jrr[jss::error] == "badSeed"); + BEAST_EXPECT(jrr[jss::error] == "actMalformed"); BEAST_EXPECT(jrr[jss::status] == "error"); - BEAST_EXPECT(jrr[jss::error_message] == "Disallowed seed."); + BEAST_EXPECT(jrr[jss::error_message] == "Account malformed."); } { // bogus account value - auto const jrr = - env.rpc("account_offers", "rNOT_AN_ACCOUNT")[jss::result]; + auto const jrr = env.rpc( + "account_offers", Account("bogus").human())[jss::result]; BEAST_EXPECT(jrr[jss::error] == "actNotFound"); BEAST_EXPECT(jrr[jss::status] == "error"); BEAST_EXPECT(jrr[jss::error_message] == "Account not found."); diff --git a/src/test/rpc/NoRippleCheck_test.cpp b/src/test/rpc/NoRippleCheck_test.cpp index 73934899e0..3d34f55c90 100644 --- a/src/test/rpc/NoRippleCheck_test.cpp +++ b/src/test/rpc/NoRippleCheck_test.cpp @@ -127,8 +127,8 @@ class NoRippleCheck_test : public beast::unit_test::suite "json", "noripple_check", boost::lexical_cast(params))[jss::result]; - BEAST_EXPECT(result[jss::error] == "badSeed"); - BEAST_EXPECT(result[jss::error_message] == "Disallowed seed."); + BEAST_EXPECT(result[jss::error] == "actMalformed"); + BEAST_EXPECT(result[jss::error_message] == "Account malformed."); } } diff --git a/src/test/rpc/OwnerInfo_test.cpp b/src/test/rpc/OwnerInfo_test.cpp index 0de4ef2bd5..c510c35afc 100644 --- a/src/test/rpc/OwnerInfo_test.cpp +++ b/src/test/rpc/OwnerInfo_test.cpp @@ -56,14 +56,16 @@ class OwnerInfo_test : public beast::unit_test::suite result.isMember(jss::accepted) && result.isMember(jss::current))) { - BEAST_EXPECT(result[jss::accepted][jss::error] == "badSeed"); + BEAST_EXPECT( + result[jss::accepted][jss::error] == "actMalformed"); BEAST_EXPECT( result[jss::accepted][jss::error_message] == - "Disallowed seed."); - BEAST_EXPECT(result[jss::current][jss::error] == "badSeed"); + "Account malformed."); + BEAST_EXPECT( + result[jss::current][jss::error] == "actMalformed"); BEAST_EXPECT( result[jss::current][jss::error_message] == - "Disallowed seed."); + "Account malformed."); } } diff --git a/src/test/rpc/RPCCall_test.cpp b/src/test/rpc/RPCCall_test.cpp index 966d325f42..1ae15afa30 100644 --- a/src/test/rpc/RPCCall_test.cpp +++ b/src/test/rpc/RPCCall_test.cpp @@ -86,7 +86,7 @@ static RPCCallTestData const rpcCallTestArray[] = { __LINE__, {"account_channels", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "FEDCBA9876543210FEDCBA9876543210FEDCBA9876543210FEDCBA9876543210"}, + "rD5MbavGfiSC5m7mkxy1FANuT7s3HxqpoF"}, RPCCallTestData::no_exception, R"({ "method" : "account_channels", @@ -94,13 +94,15 @@ static RPCCallTestData const rpcCallTestArray[] = { { "api_version" : %MAX_API_VER%, "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "destination_account" : "FEDCBA9876543210FEDCBA9876543210FEDCBA9876543210FEDCBA9876543210" + "destination_account" : "rD5MbavGfiSC5m7mkxy1FANuT7s3HxqpoF" } ] })"}, {"account_channels: account and ledger index.", __LINE__, - {"account_channels", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "closed"}, + {"account_channels", + "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "r9emE59aTWb85t64dAebKrxYMBTpzK5yR7"}, RPCCallTestData::no_exception, R"({ "method" : "account_channels", @@ -108,7 +110,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "api_version" : %MAX_API_VER%, "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "destination_account" : "closed" + "destination_account" : "r9emE59aTWb85t64dAebKrxYMBTpzK5yR7" } ] })"}, @@ -186,7 +188,7 @@ static RPCCallTestData const rpcCallTestArray[] = { "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA", "current", - "strict"}, + "extra"}, RPCCallTestData::no_exception, R"({ "method" : "account_channels", @@ -218,7 +220,7 @@ static RPCCallTestData const rpcCallTestArray[] = { // account_currencies // ---------------------------------------------------------- - {"account_currencies: minimal.", + {"account_currencies: minimal 1.", __LINE__, {"account_currencies", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"}, RPCCallTestData::no_exception, @@ -231,17 +233,16 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"account_currencies: strict.", + {"account_currencies: minimal 2.", __LINE__, - {"account_currencies", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "strict"}, + {"account_currencies", "racb4o3DrdYxuCfyVa6vsLb7vgju9RFbBr"}, RPCCallTestData::no_exception, R"({ "method" : "account_currencies", "params" : [ { "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 + "account" : "racb4o3DrdYxuCfyVa6vsLb7vgju9RFbBr" } ] })"}, @@ -275,10 +276,7 @@ static RPCCallTestData const rpcCallTestArray[] = { })"}, {"account_currencies: current ledger.", __LINE__, - {"account_currencies", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "current", - "strict"}, + {"account_currencies", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "current"}, RPCCallTestData::no_exception, R"({ "method" : "account_currencies", @@ -286,8 +284,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "api_version" : %MAX_API_VER%, - "ledger_index" : "current", - "strict" : 1 + "ledger_index" : "current" } ] })"}, @@ -312,8 +309,8 @@ static RPCCallTestData const rpcCallTestArray[] = { {"account_currencies", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "current", - "strict", - "spare"}, + "spare1", + "spare2"}, RPCCallTestData::no_exception, R"({ "method" : "account_currencies", @@ -358,20 +355,6 @@ static RPCCallTestData const rpcCallTestArray[] = { ] })", }, - {"account_currencies: floating point first argument.", - __LINE__, - {"account_currencies", "3.14159", "strict"}, - RPCCallTestData::no_exception, - R"({ - "method" : "account_currencies", - "params" : [ - { - "api_version" : %MAX_API_VER%, - "account" : "3.14159", - "strict" : 1 - } - ] - })"}, // account_info // ---------------------------------------------------------------- @@ -432,26 +415,9 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"account_info: strict.", + {"account_info: with ledger index.", __LINE__, - {"account_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "strict"}, - RPCCallTestData::no_exception, - R"({ - "method" : "account_info", - "params" : [ - { - "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 - } - ] - })"}, - {"account_info: with ledger index and strict.", - __LINE__, - {"account_info", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "validated", - "strict"}, + {"account_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "validated"}, RPCCallTestData::no_exception, R"({ "method" : "account_info", @@ -459,8 +425,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "api_version" : %MAX_API_VER%, - "ledger_index" : "validated", - "strict" : 1 + "ledger_index" : "validated" } ] })"}, @@ -485,8 +450,8 @@ static RPCCallTestData const rpcCallTestArray[] = { {"account_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "current", - "strict", - "extra"}, + "extra1", + "extra2"}, RPCCallTestData::no_exception, R"({ "method" : "account_info", @@ -798,9 +763,9 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"account_objects: strict.", + {"account_objects: with ledger index.", __LINE__, - {"account_objects", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "strict"}, + {"account_objects", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "validated"}, RPCCallTestData::no_exception, R"({ "method" : "account_objects", @@ -808,25 +773,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "api_version" : %MAX_API_VER%, "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 - } - ] - })"}, - {"account_objects: with ledger index and strict.", - __LINE__, - {"account_objects", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "validated", - "strict"}, - RPCCallTestData::no_exception, - R"({ - "method" : "account_objects", - "params" : [ - { - "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "ledger_index" : "validated", - "strict" : 1 + "ledger_index" : "validated" } ] })"}, @@ -853,8 +800,8 @@ static RPCCallTestData const rpcCallTestArray[] = { "account_objects", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "current", - "extra", - "strict", + "extra1", + "extra2", }, RPCCallTestData::no_exception, R"({ @@ -862,8 +809,7 @@ static RPCCallTestData const rpcCallTestArray[] = { "params" : [ { "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 + "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh" } ] })"}, @@ -876,7 +822,7 @@ static RPCCallTestData const rpcCallTestArray[] = { "current", "extra1", "extra2", - "strict", + "extra3", }, RPCCallTestData::no_exception, R"({ @@ -884,8 +830,7 @@ static RPCCallTestData const rpcCallTestArray[] = { "params" : [ { "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 + "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh" } ] })"}, @@ -898,7 +843,7 @@ static RPCCallTestData const rpcCallTestArray[] = { "extra1", "extra2", "extra3", - "strict", + "extra4", }, RPCCallTestData::no_exception, R"({ @@ -953,10 +898,7 @@ static RPCCallTestData const rpcCallTestArray[] = { // cannot currently occur because jvParseLedger() always returns true. "account_objects: invalid ledger selection 2.", __LINE__, - {"account_objects", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "no_ledger", - "strict"}, + {"account_objects", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "no_ledger"}, RPCCallTestData::no_exception, R"({ "method" : "account_objects", @@ -964,8 +906,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "api_version" : %MAX_API_VER%, "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "ledger_index" : 0, - "strict" : 1 + "ledger_index" : 0 } ] })", @@ -1030,9 +971,9 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"account_offers: strict.", + {"account_offers: with ledger index.", __LINE__, - {"account_offers", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "strict"}, + {"account_offers", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "validated"}, RPCCallTestData::no_exception, R"({ "method" : "account_offers", @@ -1040,25 +981,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "api_version" : %MAX_API_VER%, "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 - } - ] - })"}, - {"account_offers: with ledger index and strict.", - __LINE__, - {"account_offers", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "validated", - "strict"}, - RPCCallTestData::no_exception, - R"({ - "method" : "account_offers", - "params" : [ - { - "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "ledger_index" : "validated", - "strict" : 1 + "ledger_index" : "validated" } ] })"}, @@ -1081,21 +1004,17 @@ static RPCCallTestData const rpcCallTestArray[] = { {// Note: I believe this _ought_ to be detected as too many arguments. "account_offers: four arguments.", __LINE__, - { - "account_offers", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "current", - "extra", - "strict", - }, + {"account_offers", + "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "current", + "extra"}, RPCCallTestData::no_exception, R"({ "method" : "account_offers", "params" : [ { "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 + "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh" } ] })"}, @@ -1107,7 +1026,7 @@ static RPCCallTestData const rpcCallTestArray[] = { "current", "extra1", "extra2", - "strict", + "extra3", }, RPCCallTestData::no_exception, R"({ @@ -1162,10 +1081,7 @@ static RPCCallTestData const rpcCallTestArray[] = { // cannot currently occur because jvParseLedger() always returns true. "account_offers: invalid ledger selection 2.", __LINE__, - {"account_offers", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "no_ledger", - "strict"}, + {"account_offers", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "no_ledger"}, RPCCallTestData::no_exception, R"({ "method" : "account_offers", @@ -1173,8 +1089,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "api_version" : %MAX_API_VER%, "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "ledger_index" : 0, - "strict" : 1 + "ledger_index" : 0 } ] })", @@ -4429,26 +4344,9 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"owner_info: strict.", + {"owner_info: with ledger index.", __LINE__, - {"owner_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "strict"}, - RPCCallTestData::no_exception, - R"({ - "method" : "owner_info", - "params" : [ - { - "api_version" : %MAX_API_VER%, - "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "strict" : 1 - } - ] - })"}, - {"owner_info: with ledger index and strict.", - __LINE__, - {"owner_info", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "validated", - "strict"}, + {"owner_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "validated"}, RPCCallTestData::no_exception, R"({ "method" : "owner_info", @@ -4456,8 +4354,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "api_version" : %MAX_API_VER%, - "ledger_index" : "validated", - "strict" : 1 + "ledger_index" : "validated" } ] })"}, @@ -4483,8 +4380,8 @@ static RPCCallTestData const rpcCallTestArray[] = { "owner_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "current", - "extra", - "strict", + "extra1", + "extra2", }, RPCCallTestData::no_exception, R"({ @@ -4537,12 +4434,9 @@ static RPCCallTestData const rpcCallTestArray[] = { { // Note: there is code in place to return rpcLGR_IDX_MALFORMED. That // cannot currently occur because jvParseLedger() always returns true. - "owner_info: invalid ledger selection and strict.", + "owner_info: invalid ledger selection.", __LINE__, - {"owner_info", - "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", - "no_ledger", - "strict"}, + {"owner_info", "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "no_ledger"}, RPCCallTestData::no_exception, R"({ "method" : "owner_info", @@ -4550,8 +4444,7 @@ static RPCCallTestData const rpcCallTestArray[] = { { "account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "api_version" : %MAX_API_VER%, - "ledger_index" : 0, - "strict" : 1 + "ledger_index" : 0 } ] })",