Skip to content

Commit eba5d19

Browse files
mvadariximinez
mvadari
authored and
ximinez
committed
Improve error handling in some RPC commands
1 parent 2df6356 commit eba5d19

17 files changed

+329
-35
lines changed

‎Builds/CMake/RippledCore.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,7 @@ if (tests)
11191119
#]===============================]
11201120
src/test/rpc/AccountCurrencies_test.cpp
11211121
src/test/rpc/AccountInfo_test.cpp
1122-
src/test/rpc/AccountLinesRPC_test.cpp
1122+
src/test/rpc/AccountLines_test.cpp
11231123
src/test/rpc/AccountObjects_test.cpp
11241124
src/test/rpc/AccountOffers_test.cpp
11251125
src/test/rpc/AccountSet_test.cpp

‎src/ripple/rpc/handlers/AccountChannels.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ doAccountChannels(RPC::JsonContext& context)
7171
if (!params.isMember(jss::account))
7272
return RPC::missing_field_error(jss::account);
7373

74+
if (!params[jss::account].isString())
75+
return RPC::invalid_field_error(jss::account);
76+
7477
std::shared_ptr<ReadView const> ledger;
7578
auto result = RPC::lookupLedger(ledger, context);
7679
if (!ledger)

‎src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,29 @@ doAccountCurrencies(RPC::JsonContext& context)
3333
{
3434
auto& params = context.params;
3535

36+
if (!(params.isMember(jss::account) || params.isMember(jss::ident)))
37+
return RPC::missing_field_error(jss::account);
38+
39+
std::string strIdent;
40+
if (params.isMember(jss::account))
41+
{
42+
if (!params[jss::account].isString())
43+
return RPC::invalid_field_error(jss::account);
44+
strIdent = params[jss::account].asString();
45+
}
46+
else if (params.isMember(jss::ident))
47+
{
48+
if (!params[jss::ident].isString())
49+
return RPC::invalid_field_error(jss::ident);
50+
strIdent = params[jss::ident].asString();
51+
}
52+
3653
// Get the current ledger
3754
std::shared_ptr<ReadView const> ledger;
3855
auto result = RPC::lookupLedger(ledger, context);
3956
if (!ledger)
4057
return result;
4158

42-
if (!(params.isMember(jss::account) || params.isMember(jss::ident)))
43-
return RPC::missing_field_error(jss::account);
44-
45-
std::string const strIdent(
46-
params.isMember(jss::account) ? params[jss::account].asString()
47-
: params[jss::ident].asString());
48-
4959
// Get info on account.
5060
auto id = parseBase58<AccountID>(strIdent);
5161
if (!id)

‎src/ripple/rpc/handlers/AccountInfo.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <ripple/ledger/ReadView.h>
2424
#include <ripple/protocol/ErrorCodes.h>
2525
#include <ripple/protocol/Indexes.h>
26+
#include <ripple/protocol/RPCErr.h>
2627
#include <ripple/protocol/UintTypes.h>
2728
#include <ripple/protocol/jss.h>
2829
#include <ripple/rpc/Context.h>
@@ -53,9 +54,17 @@ doAccountInfo(RPC::JsonContext& context)
5354

5455
std::string strIdent;
5556
if (params.isMember(jss::account))
57+
{
58+
if (!params[jss::account].isString())
59+
return RPC::invalid_field_error(jss::account);
5660
strIdent = params[jss::account].asString();
61+
}
5762
else if (params.isMember(jss::ident))
63+
{
64+
if (!params[jss::ident].isString())
65+
return RPC::invalid_field_error(jss::ident);
5866
strIdent = params[jss::ident].asString();
67+
}
5968
else
6069
return RPC::missing_field_error(jss::account);
6170

‎src/ripple/rpc/handlers/AccountLines.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ doAccountLines(RPC::JsonContext& context)
8080
if (!params.isMember(jss::account))
8181
return RPC::missing_field_error(jss::account);
8282

83+
if (!params[jss::account].isString())
84+
return RPC::invalid_field_error(jss::account);
85+
8386
std::shared_ptr<ReadView const> ledger;
8487
auto result = RPC::lookupLedger(ledger, context);
8588
if (!ledger)

‎src/ripple/rpc/handlers/AccountObjects.cpp‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,19 @@ doAccountNFTs(RPC::JsonContext& context)
5454
if (!params.isMember(jss::account))
5555
return RPC::missing_field_error(jss::account);
5656

57-
std::shared_ptr<ReadView const> ledger;
58-
auto result = RPC::lookupLedger(ledger, context);
59-
if (ledger == nullptr)
60-
return result;
57+
if (!params[jss::account].isString())
58+
return RPC::invalid_field_error(jss::account);
6159

6260
auto id = parseBase58<AccountID>(params[jss::account].asString());
6361
if (!id)
6462
{
65-
RPC::inject_error(rpcACT_MALFORMED, result);
66-
return result;
63+
return rpcError(rpcACT_MALFORMED);
6764
}
65+
66+
std::shared_ptr<ReadView const> ledger;
67+
auto result = RPC::lookupLedger(ledger, context);
68+
if (ledger == nullptr)
69+
return result;
6870
auto const accountID{id.value()};
6971

7072
if (!ledger->exists(keylet::account(accountID)))
@@ -167,6 +169,9 @@ doAccountObjects(RPC::JsonContext& context)
167169
if (!params.isMember(jss::account))
168170
return RPC::missing_field_error(jss::account);
169171

172+
if (!params[jss::account].isString())
173+
return RPC::invalid_field_error(jss::account);
174+
170175
std::shared_ptr<ReadView const> ledger;
171176
auto result = RPC::lookupLedger(ledger, context);
172177
if (ledger == nullptr)

‎src/ripple/rpc/handlers/AccountOffers.cpp‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ doAccountOffers(RPC::JsonContext& context)
6060
if (!params.isMember(jss::account))
6161
return RPC::missing_field_error(jss::account);
6262

63+
if (!params[jss::account].isString())
64+
return RPC::invalid_field_error(jss::account);
65+
6366
std::shared_ptr<ReadView const> ledger;
6467
auto result = RPC::lookupLedger(ledger, context);
6568
if (!ledger)
@@ -84,7 +87,7 @@ doAccountOffers(RPC::JsonContext& context)
8487
return *err;
8588

8689
if (limit == 0)
87-
return rpcError(rpcINVALID_PARAMS);
90+
return RPC::invalid_field_error(jss::limit);
8891

8992
Json::Value& jsonOffers(result[jss::offers] = Json::arrayValue);
9093
std::vector<std::shared_ptr<SLE const>> offers;
@@ -101,21 +104,21 @@ doAccountOffers(RPC::JsonContext& context)
101104
std::stringstream marker(params[jss::marker].asString());
102105
std::string value;
103106
if (!std::getline(marker, value, ','))
104-
return rpcError(rpcINVALID_PARAMS);
107+
return RPC::invalid_field_error(jss::marker);
105108

106109
if (!startAfter.parseHex(value))
107-
return rpcError(rpcINVALID_PARAMS);
110+
return RPC::invalid_field_error(jss::marker);
108111

109112
if (!std::getline(marker, value, ','))
110-
return rpcError(rpcINVALID_PARAMS);
113+
return RPC::invalid_field_error(jss::marker);
111114

112115
try
113116
{
114117
startHint = boost::lexical_cast<std::uint64_t>(value);
115118
}
116119
catch (boost::bad_lexical_cast&)
117120
{
118-
return rpcError(rpcINVALID_PARAMS);
121+
return RPC::invalid_field_error(jss::marker);
119122
}
120123

121124
// We then must check if the object pointed to by the marker is actually

‎src/ripple/rpc/handlers/AccountTx.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,12 +426,12 @@ doAccountTxJson(RPC::JsonContext& context)
426426
if (context.apiVersion > 1u && params.isMember(jss::binary) &&
427427
!params[jss::binary].isBool())
428428
{
429-
return rpcError(rpcINVALID_PARAMS);
429+
return RPC::invalid_field_error(jss::binary);
430430
}
431431
if (context.apiVersion > 1u && params.isMember(jss::forward) &&
432432
!params[jss::forward].isBool())
433433
{
434-
return rpcError(rpcINVALID_PARAMS);
434+
return RPC::invalid_field_error(jss::forward);
435435
}
436436

437437
args.limit = params.isMember(jss::limit) ? params[jss::limit].asUInt() : 0;
@@ -440,7 +440,10 @@ doAccountTxJson(RPC::JsonContext& context)
440440
params.isMember(jss::forward) && params[jss::forward].asBool();
441441

442442
if (!params.isMember(jss::account))
443-
return rpcError(rpcINVALID_PARAMS);
443+
return RPC::missing_field_error(jss::account);
444+
445+
if (!params[jss::account].isString())
446+
return RPC::invalid_field_error(jss::account);
444447

445448
auto const account =
446449
parseBase58<AccountID>(params[jss::account].asString());

‎src/ripple/rpc/handlers/NoRippleCheck.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ doNoRippleCheck(RPC::JsonContext& context)
6666

6767
if (!params.isMember("role"))
6868
return RPC::missing_field_error("role");
69+
70+
if (!params[jss::account].isString())
71+
return RPC::invalid_field_error(jss::account);
72+
6973
bool roleGateway = false;
7074
{
7175
std::string const role = params["role"].asString();
@@ -90,7 +94,7 @@ doNoRippleCheck(RPC::JsonContext& context)
9094
if (context.apiVersion > 1u && params.isMember(jss::transactions) &&
9195
!params[jss::transactions].isBool())
9296
{
93-
return rpcError(rpcINVALID_PARAMS);
97+
return RPC::invalid_field_error(jss::transactions);
9498
}
9599

96100
std::shared_ptr<ReadView const> ledger;

‎src/test/app/PayChan_test.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,25 @@ struct PayChan_test : public beast::unit_test::suite
873873
auto const chan1Str = to_string(channel(alice, bob, env.seq(alice)));
874874
env(create(alice, bob, channelFunds, settleDelay, pk));
875875
env.close();
876+
{
877+
// test account non-string
878+
auto testInvalidAccountParam = [&](auto const& param) {
879+
Json::Value params;
880+
params[jss::account] = param;
881+
auto jrr = env.rpc(
882+
"json", "account_channels", to_string(params))[jss::result];
883+
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
884+
BEAST_EXPECT(
885+
jrr[jss::error_message] == "Invalid field 'account'.");
886+
};
887+
888+
testInvalidAccountParam(1);
889+
testInvalidAccountParam(1.1);
890+
testInvalidAccountParam(true);
891+
testInvalidAccountParam(Json::Value(Json::nullValue));
892+
testInvalidAccountParam(Json::Value(Json::objectValue));
893+
testInvalidAccountParam(Json::Value(Json::arrayValue));
894+
}
876895
{
877896
auto const r =
878897
env.rpc("account_channels", alice.human(), bob.human());

0 commit comments

Comments
 (0)