ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-20 02:55:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve error handling in some RPC commands

Browse Source
This commit is contained in:
Mayukha Vadari
2024-07-12 17:58:17 -04:00
committed by Ed Hennis
parent 2df635693d
commit eba5d19377
17 changed files with 329 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Builds/CMake/RippledCore.cmake
View File

@@ -1119,7 +1119,7 @@ if (tests)
#]===============================]
src/test/rpc/AccountCurrencies_test.cpp
src/test/rpc/AccountInfo_test.cpp
src/test/rpc/AccountLinesRPC_test.cpp
src/test/rpc/AccountLines_test.cpp
src/test/rpc/AccountObjects_test.cpp
src/test/rpc/AccountOffers_test.cpp
src/test/rpc/AccountSet_test.cpp

3
src/ripple/rpc/handlers/AccountChannels.cpp
View File

@@ -71,6 +71,9 @@ doAccountChannels(RPC::JsonContext& context)
if (!params.isMember(jss::account))
return RPC::missing_field_error(jss::account);
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (!ledger)

24
src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp
View File

@@ -33,19 +33,29 @@ doAccountCurrencies(RPC::JsonContext& context)
{
auto& params = context.params;
if (!(params.isMember(jss::account) || params.isMember(jss::ident)))
return RPC::missing_field_error(jss::account);
std::string strIdent;
if (params.isMember(jss::account))
{
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
strIdent = params[jss::account].asString();
}
else if (params.isMember(jss::ident))
{
if (!params[jss::ident].isString())
return RPC::invalid_field_error(jss::ident);
strIdent = params[jss::ident].asString();
}
// Get the current ledger
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (!ledger)
return result;
if (!(params.isMember(jss::account) || params.isMember(jss::ident)))
return RPC::missing_field_error(jss::account);
std::string const strIdent(
params.isMember(jss::account) ? params[jss::account].asString()
: params[jss::ident].asString());
// Get info on account.
auto id = parseBase58<AccountID>(strIdent);
if (!id)

9
src/ripple/rpc/handlers/AccountInfo.cpp
View File

@@ -23,6 +23,7 @@
#include <ripple/ledger/ReadView.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/RPCErr.h>
#include <ripple/protocol/UintTypes.h>
#include <ripple/protocol/jss.h>
#include <ripple/rpc/Context.h>
@@ -53,9 +54,17 @@ doAccountInfo(RPC::JsonContext& context)
std::string strIdent;
if (params.isMember(jss::account))
{
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
strIdent = params[jss::account].asString();
}
else if (params.isMember(jss::ident))
{
if (!params[jss::ident].isString())
return RPC::invalid_field_error(jss::ident);
strIdent = params[jss::ident].asString();
}
else
return RPC::missing_field_error(jss::account);

3
src/ripple/rpc/handlers/AccountLines.cpp
View File

@@ -80,6 +80,9 @@ doAccountLines(RPC::JsonContext& context)
if (!params.isMember(jss::account))
return RPC::missing_field_error(jss::account);
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (!ledger)

17
src/ripple/rpc/handlers/AccountObjects.cpp
View File

@@ -54,17 +54,19 @@ doAccountNFTs(RPC::JsonContext& context)
if (!params.isMember(jss::account))
return RPC::missing_field_error(jss::account);
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (ledger == nullptr)
return result;
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
auto id = parseBase58<AccountID>(params[jss::account].asString());
if (!id)
{
RPC::inject_error(rpcACT_MALFORMED, result);
return result;
return rpcError(rpcACT_MALFORMED);
}
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (ledger == nullptr)
return result;
auto const accountID{id.value()};
if (!ledger->exists(keylet::account(accountID)))
@@ -167,6 +169,9 @@ doAccountObjects(RPC::JsonContext& context)
if (!params.isMember(jss::account))
return RPC::missing_field_error(jss::account);
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (ledger == nullptr)

13
src/ripple/rpc/handlers/AccountOffers.cpp
View File

@@ -60,6 +60,9 @@ doAccountOffers(RPC::JsonContext& context)
if (!params.isMember(jss::account))
return RPC::missing_field_error(jss::account);
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (!ledger)
@@ -84,7 +87,7 @@ doAccountOffers(RPC::JsonContext& context)
return *err;
if (limit == 0)
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::limit);
Json::Value& jsonOffers(result[jss::offers] = Json::arrayValue);
std::vector<std::shared_ptr<SLE const>> offers;
@@ -101,13 +104,13 @@ doAccountOffers(RPC::JsonContext& context)
std::stringstream marker(params[jss::marker].asString());
std::string value;
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::marker);
if (!startAfter.parseHex(value))
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::marker);
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::marker);
try
{
@@ -115,7 +118,7 @@ doAccountOffers(RPC::JsonContext& context)
}
catch (boost::bad_lexical_cast&)
{
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::marker);
}
// We then must check if the object pointed to by the marker is actually

9
src/ripple/rpc/handlers/AccountTx.cpp
View File

@@ -426,12 +426,12 @@ doAccountTxJson(RPC::JsonContext& context)
if (context.apiVersion > 1u && params.isMember(jss::binary) &&
!params[jss::binary].isBool())
{
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::binary);
}
if (context.apiVersion > 1u && params.isMember(jss::forward) &&
!params[jss::forward].isBool())
{
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::forward);
}
args.limit = params.isMember(jss::limit) ? params[jss::limit].asUInt() : 0;
@@ -440,7 +440,10 @@ doAccountTxJson(RPC::JsonContext& context)
params.isMember(jss::forward) && params[jss::forward].asBool();
if (!params.isMember(jss::account))
return rpcError(rpcINVALID_PARAMS);
return RPC::missing_field_error(jss::account);
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
auto const account =
parseBase58<AccountID>(params[jss::account].asString());

6
src/ripple/rpc/handlers/NoRippleCheck.cpp
View File

@@ -66,6 +66,10 @@ doNoRippleCheck(RPC::JsonContext& context)
if (!params.isMember("role"))
return RPC::missing_field_error("role");
if (!params[jss::account].isString())
return RPC::invalid_field_error(jss::account);
bool roleGateway = false;
{
std::string const role = params["role"].asString();
@@ -90,7 +94,7 @@ doNoRippleCheck(RPC::JsonContext& context)
if (context.apiVersion > 1u && params.isMember(jss::transactions) &&
!params[jss::transactions].isBool())
{
return rpcError(rpcINVALID_PARAMS);
return RPC::invalid_field_error(jss::transactions);
}
std::shared_ptr<ReadView const> ledger;

19
src/test/app/PayChan_test.cpp
View File

@@ -873,6 +873,25 @@ struct PayChan_test : public beast::unit_test::suite
auto const chan1Str = to_string(channel(alice, bob, env.seq(alice)));
env(create(alice, bob, channelFunds, settleDelay, pk));
env.close();
{
// test account non-string
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_channels", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
{
auto const r =
env.rpc("account_channels", alice.human(), bob.human());

47
src/test/rpc/AccountCurrencies_test.cpp
View File

@@ -39,6 +39,7 @@ class AccountCurrencies_test : public beast::unit_test::suite
{ // invalid ledger (hash)
Json::Value params;
params[jss::account] = Account{"bob"}.human();
params[jss::ledger_hash] = 1;
auto const result = env.rpc(
"json",
@@ -56,6 +57,50 @@ class AccountCurrencies_test : public beast::unit_test::suite
result[jss::error_message] == "Missing field 'account'.");
}
{
// test account non-string
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json",
"account_currencies",
to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
{
// test ident non-string
auto testInvalidIdentParam = [&](auto const& param) {
Json::Value params;
params[jss::ident] = param;
auto jrr = env.rpc(
"json",
"account_currencies",
to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'ident'.");
};
testInvalidIdentParam(1);
testInvalidIdentParam(1.1);
testInvalidIdentParam(true);
testInvalidIdentParam(Json::Value(Json::nullValue));
testInvalidIdentParam(Json::Value(Json::objectValue));
testInvalidIdentParam(Json::Value(Json::arrayValue));
}
{
Json::Value params;
params[jss::account] =
@@ -198,6 +243,6 @@ public:
}
};
BEAST_DEFINE_TESTSUITE(AccountCurrencies, app, ripple);
BEAST_DEFINE_TESTSUITE(AccountCurrencies, rpc, ripple);
} // namespace ripple

47
src/test/rpc/AccountInfo_test.cpp
View File

@@ -36,6 +36,7 @@ public:
void
testErrors()
{
testcase("Errors");
using namespace jtx;
Env env(*this);
{
@@ -78,12 +79,53 @@ public:
BEAST_EXPECT(
info[jss::result][jss::error_message] == "Account malformed.");
}
{
// Cannot pass a non-string into the `account` param
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_info", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
{
// Cannot pass a non-string into the `ident` param
auto testInvalidIdentParam = [&](auto const& param) {
Json::Value params;
params[jss::ident] = param;
auto jrr = env.rpc(
"json", "account_info", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'ident'.");
};
testInvalidIdentParam(1);
testInvalidIdentParam(1.1);
testInvalidIdentParam(true);
testInvalidIdentParam(Json::Value(Json::nullValue));
testInvalidIdentParam(Json::Value(Json::objectValue));
testInvalidIdentParam(Json::Value(Json::arrayValue));
}
}
// Test the "signer_lists" argument in account_info.
void
testSignerLists()
{
testcase("Signer lists");
using namespace jtx;
Env env(*this);
Account const alice{"alice"};
@@ -205,6 +247,7 @@ public:
void
testSignerListsApiVersion2()
{
testcase("Signer lists APIv2");
using namespace jtx;
Env env{*this};
Account const alice{"alice"};
@@ -326,6 +369,7 @@ public:
void
testSignerListsV2()
{
testcase("Signer lists v2");
using namespace jtx;
Env env(*this);
Account const alice{"alice"};
@@ -515,6 +559,7 @@ public:
void
testAccountFlags(FeatureBitset const& features)
{
testcase("Account flags");
using namespace jtx;
Env env(*this, features);
@@ -652,7 +697,7 @@ public:
}
};
BEAST_DEFINE_TESTSUITE(AccountInfo, app, ripple);
BEAST_DEFINE_TESTSUITE(AccountInfo, rpc, ripple);
} // namespace test
} // namespace ripple

23
src/test/rpc/AccountLinesRPC_test.cpp → src/test/rpc/AccountLines_test.cpp
View File

@@ -27,7 +27,7 @@ namespace ripple {
namespace RPC {
class AccountLinesRPC_test : public beast::unit_test::suite
class AccountLines_test : public beast::unit_test::suite
{
public:
void
@@ -55,6 +55,25 @@ public:
lines[jss::result][jss::error_message] ==
RPC::make_error(rpcACT_MALFORMED)[jss::error_message]);
}
{
// test account non-string
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_lines", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
Account const alice{"alice"};
{
// account_lines on an unfunded account.
@@ -1474,7 +1493,7 @@ public:
}
};
BEAST_DEFINE_TESTSUITE(AccountLinesRPC, app, ripple);
BEAST_DEFINE_TESTSUITE(AccountLines, rpc, ripple);
} // namespace RPC
} // namespace ripple

58
src/test/rpc/AccountObjects_test.cpp
View File

@@ -123,8 +123,30 @@ public:
// test error on no account
{
auto resp = env.rpc("json", "account_objects");
BEAST_EXPECT(resp[jss::error_message] == "Syntax error.");
Json::Value params;
auto resp = env.rpc("json", "account_objects", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"Missing field 'account'.");
}
// test account non-string
{
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_objects", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
// test error on malformed account string.
{
@@ -1032,6 +1054,35 @@ public:
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0));
}
void
testAccountNFTs()
{
testcase("account_nfts");
using namespace jtx;
Env env(*this);
// test validation
{
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_nfts", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
}
void
run() override
{
@@ -1039,10 +1090,11 @@ public:
testUnsteppedThenStepped();
testUnsteppedThenSteppedWithNFTs();
testObjectTypes();
testAccountNFTs();
}
};
BEAST_DEFINE_TESTSUITE(AccountObjects, app, ripple);
BEAST_DEFINE_TESTSUITE(AccountObjects, rpc, ripple);
} // namespace test
} // namespace ripple

33
src/test/rpc/AccountOffers_test.cpp
View File

@@ -37,6 +37,8 @@ public:
void
testNonAdminMinLimit()
{
testcase("Non-Admin Min Limit");
using namespace jtx;
Env env{*this, envconfig(no_admin)};
Account const gw("G1");
@@ -81,6 +83,9 @@ public:
void
testSequential(bool asAdmin)
{
testcase(
std::string("Sequential - ") + (asAdmin ? "admin" : "non-admin"));
using namespace jtx;
Env env{*this, asAdmin ? envconfig() : envconfig(no_admin)};
Account const gw("G1");
@@ -215,6 +220,8 @@ public:
void
testBadInput()
{
testcase("Bad input");
using namespace jtx;
Env env(*this);
Account const gw("G1");
@@ -233,6 +240,26 @@ public:
BEAST_EXPECT(jrr[jss::error_message] == "Syntax error.");
}
{
// test account non-string
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_offers", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
{
// empty string account
Json::Value jvParams;
@@ -282,7 +309,9 @@ public:
jvParams.toStyledString())[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(jrr[jss::status] == "error");
BEAST_EXPECT(jrr[jss::error_message] == "Invalid parameters.");
BEAST_EXPECTS(
jrr[jss::error_message] == "Invalid field 'marker'.",
jrr.toStyledString());
}
{
@@ -326,7 +355,7 @@ public:
}
};
BEAST_DEFINE_TESTSUITE(AccountOffers, app, ripple);
BEAST_DEFINE_TESTSUITE(AccountOffers, rpc, ripple);
} // namespace test
} // namespace ripple

26
src/test/rpc/AccountTx_test.cpp
View File

@@ -109,6 +109,7 @@ class AccountTx_test : public beast::unit_test::suite
void
testParameters(unsigned int apiVersion)
{
testcase("Parameters APIv" + std::to_string(apiVersion));
using namespace test::jtx;
Env env(*this);
@@ -353,6 +354,25 @@ class AccountTx_test : public beast::unit_test::suite
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));
}
// test account non-string
{
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
auto jrr = env.rpc(
"json", "account_tx", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
// test binary and forward for bool/non bool values
{
Json::Value p{jParms};
@@ -388,6 +408,8 @@ class AccountTx_test : public beast::unit_test::suite
void
testContents()
{
testcase("Contents");
// Get results for all transaction types that can be associated
// with an account. Start by generating all transaction types.
using namespace test::jtx;
@@ -600,6 +622,8 @@ class AccountTx_test : public beast::unit_test::suite
void
testAccountDelete()
{
testcase("AccountDelete");
// Verify that if an account is resurrected then the account_tx RPC
// command still recovers all transactions on that account before
// and after resurrection.
@@ -740,7 +764,7 @@ public:
testAccountDelete();
}
};
BEAST_DEFINE_TESTSUITE(AccountTx, app, ripple);
BEAST_DEFINE_TESTSUITE(AccountTx, rpc, ripple);
} // namespace test
} // namespace ripple

25
src/test/rpc/NoRippleCheck_test.cpp
View File

@@ -64,6 +64,27 @@ class NoRippleCheck_test : public beast::unit_test::suite
BEAST_EXPECT(result[jss::error_message] == "Missing field 'role'.");
}
// test account non-string
{
auto testInvalidAccountParam = [&](auto const& param) {
Json::Value params;
params[jss::account] = param;
params[jss::role] = "user";
auto jrr = env.rpc(
"json", "noripple_check", to_string(params))[jss::result];
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(
jrr[jss::error_message] == "Invalid field 'account'.");
};
testInvalidAccountParam(1);
testInvalidAccountParam(1.1);
testInvalidAccountParam(true);
testInvalidAccountParam(Json::Value(Json::nullValue));
testInvalidAccountParam(Json::Value(Json::objectValue));
testInvalidAccountParam(Json::Value(Json::arrayValue));
}
{ // invalid role field
Json::Value params;
params[jss::account] = alice.human();
@@ -369,12 +390,12 @@ public:
}
};
BEAST_DEFINE_TESTSUITE(NoRippleCheck, app, ripple);
BEAST_DEFINE_TESTSUITE(NoRippleCheck, rpc, ripple);
// These tests that deal with limit amounts are slow because of the
// offer/account setup, so making them manual -- the additional coverage
// provided by them is minimal
BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(NoRippleCheckLimits, app, ripple, 1);
BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(NoRippleCheckLimits, rpc, ripple, 1);
} // namespace ripple