Skip to content

Commit

Permalink
Cleanup some Json::Value methods:
Browse files Browse the repository at this point in the history
* Rename isArray to isArrayOrNull
* Rename isObject to isObjectOrNull
* Introduce isArray and isObject
* Change as many uses of isArrayorNull to isArray as possible
* Change as many uses of isObjectorNull to isObject as possible
* Reject null JSON arrays for subscribe and unsubscribe
  • Loading branch information
HowardHinnant authored and mellery451 committed Mar 1, 2018
1 parent 20defb4 commit 1a24523
Show file tree
Hide file tree
Showing 24 changed files with 90 additions and 63 deletions.
4 changes: 2 additions & 2 deletions src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1584,7 +1584,7 @@ ApplicationImp::loadLedgerFromFile (
ledger = ledger.get()["accountState"];
}

if (!ledger.get().isArray ())
if (!ledger.get().isArrayOrNull ())
{
JLOG(m_journal.fatal())
<< "State nodes must be an array";
Expand All @@ -1599,7 +1599,7 @@ ApplicationImp::loadLedgerFromFile (
{
Json::Value& entry = ledger.get()[index];

if (!entry.isObject())
if (!entry.isObjectOrNull())
{
JLOG(m_journal.fatal())
<< "Invalid entry in ledger";
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/misc/impl/AccountTxPaging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ accountTxPage (
bool bAdmin,
std::uint32_t page_length)
{
bool lookingForMarker = !token.isNull() && token.isObject();
bool lookingForMarker = token.isObject();

std::uint32_t numberOfResults;

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/misc/impl/ValidatorList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ ValidatorList::applyList (
std::vector<std::string> manifests;
for (auto const& val : newList)
{
if (val.isObject () &&
if (val.isObject() &&
val.isMember ("validation_public_key") &&
val["validation_public_key"].isString ())
{
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/misc/impl/ValidatorSite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ ValidatorSite::onSiteFetch(
Json::Reader r;
Json::Value body;
if (r.parse(res.body.data(), body) &&
body.isObject () &&
body.isObject() &&
body.isMember("blob") && body["blob"].isString () &&
body.isMember("manifest") && body["manifest"].isString () &&
body.isMember("signature") && body["signature"].isString() &&
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/json/impl/Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ namespace {
template <class Object>
void doCopyFrom (Object& to, Json::Value const& from)
{
assert (from.isObject());
assert (from.isObjectOrNull());
auto members = from.getMemberNames();
for (auto& m: members)
to[m] = from[m];
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/json/impl/json_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ Reader::parse ( const char* beginDoc, const char* endDoc,
Token token;
skipCommentTokens ( token );

if ( !root.isArray () && !root.isObject () )
if ( !root.isNull() && !root.isArray() && !root.isObject() )
{
// Set error location to start of doc, ideally should be first token found in doc
token.type_ = tokenError;
Expand Down
20 changes: 16 additions & 4 deletions src/ripple/json/impl/json_value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -779,10 +779,10 @@ Value::operator bool () const
if (isString ())
{
auto s = asCString();
return s && strlen(s);
return s && s[0];
}

return ! (isArray () || isObject ()) || size ();
return ! (isArray() || isObject()) || size ();
}

void
Expand Down Expand Up @@ -1092,14 +1092,26 @@ Value::isString () const


bool
Value::isArray () const
Value::isArray() const
{
return type_ == arrayValue;
}

bool
Value::isArrayOrNull () const
{
return type_ == nullValue || type_ == arrayValue;
}


bool
Value::isObject () const
Value::isObject() const
{
return type_ == objectValue;
}

bool
Value::isObjectOrNull () const
{
return type_ == nullValue || type_ == objectValue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/json/impl/json_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ StyledWriter::isMultineArray ( const Value& value )
{
const Value& childValue = value[index];
isMultiLine = isMultiLine ||
( (childValue.isArray () || childValue.isObject ()) &&
( (childValue.isArray() || childValue.isObject()) &&
childValue.size () > 0 );
}

Expand Down Expand Up @@ -660,7 +660,7 @@ StyledStreamWriter::isMultineArray ( const Value& value )
{
const Value& childValue = value[index];
isMultiLine = isMultiLine ||
( (childValue.isArray () || childValue.isObject ()) &&
( (childValue.isArray() || childValue.isObject()) &&
childValue.size () > 0 );
}

Expand Down
6 changes: 4 additions & 2 deletions src/ripple/json/json_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,10 @@ class Value
bool isDouble () const;
bool isNumeric () const;
bool isString () const;
bool isArray () const;
bool isObject () const;
bool isArray() const;
bool isArrayOrNull () const;
bool isObject() const;
bool isObjectOrNull () const;

bool isConvertibleTo ( ValueType other ) const;

Expand Down
7 changes: 4 additions & 3 deletions src/ripple/net/impl/RPCCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class RPCParser
{
Json::Value v (Json::objectValue);

if (jvParams.isArray () && (jvParams.size () > 0))
if (jvParams.isArray() && (jvParams.size () > 0))
v[jss::params] = jvParams;

return v;
Expand Down Expand Up @@ -513,7 +513,7 @@ class RPCParser

if (reader.parse (jvParams[1u].asString (), jvRequest))
{
if (!jvRequest.isObject ())
if (!jvRequest.isObjectOrNull ())
return rpcError (rpcINVALID_PARAMS);

jvRequest[jss::method] = jvParams[0u];
Expand Down Expand Up @@ -544,7 +544,8 @@ class RPCParser
jv.isMember(jss::id) && jv.isMember(jss::method))
{
if (jv.isMember(jss::params) &&
!(jv[jss::params].isArray() || jv[jss::params].isObject()))
!(jv[jss::params].isNull() || jv[jss::params].isArray() ||
jv[jss::params].isObject()))
return false;
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/net/impl/RPCErr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Json::Value rpcError (int iError, Json::Value jvResult)
// VFALCO NOTE Deprecated function
bool isRpcError (Json::Value jvResult)
{
return jvResult.isObject () && jvResult.isMember (jss::error);
return jvResult.isObject() && jvResult.isMember (jss::error);
}

} // ripple
10 changes: 7 additions & 3 deletions src/ripple/protocol/impl/STAmount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -806,13 +806,17 @@ amountFromJson (SField const& name, Json::Value const& v)
Json::Value currency;
Json::Value issuer;

if (v.isObject ())
if (v.isNull())
{
Throw<std::runtime_error> ("XRP may not be specified with a null Json value");
}
else if (v.isObject())
{
value = v[jss::value];
currency = v[jss::currency];
issuer = v[jss::issuer];
}
else if (v.isArray ())
else if (v.isArray())
{
value = v.get (Json::UInt (0), 0);
currency = v.get (Json::UInt (1), Json::nullValue);
Expand Down Expand Up @@ -846,7 +850,7 @@ amountFromJson (SField const& name, Json::Value const& v)

if (native)
{
if (v.isObject ())
if (v.isObjectOrNull ())
Throw<std::runtime_error> ("XRP may not be specified as an object");
issue = xrpIssue ();
}
Expand Down
21 changes: 11 additions & 10 deletions src/ripple/protocol/impl/STParsedJSON.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ static boost::optional<detail::STVar> parseLeaf (
break;

case STI_VECTOR256:
if (! value.isArray ())
if (! value.isArrayOrNull ())
{
error = array_expected (json_name, fieldName);
return ret;
Expand All @@ -521,7 +521,7 @@ static boost::optional<detail::STVar> parseLeaf (
break;

case STI_PATHSET:
if (!value.isArray ())
if (!value.isArrayOrNull ())
{
error = array_expected (json_name, fieldName);
return ret;
Expand All @@ -535,7 +535,7 @@ static boost::optional<detail::STVar> parseLeaf (
{
STPath p;

if (!value[i].isArray ())
if (!value[i].isArrayOrNull ())
{
std::stringstream ss;
ss << fieldName << "[" << i << "]";
Expand All @@ -555,7 +555,7 @@ static boost::optional<detail::STVar> parseLeaf (

Json::Value pathEl = value[i][j];

if (!pathEl.isObject ())
if (!pathEl.isObject())
{
error = not_an_object (element_name);
return ret;
Expand Down Expand Up @@ -709,7 +709,7 @@ static boost::optional <STObject> parseObject (
int depth,
Json::Value& error)
{
if (! json.isObject ())
if (! json.isObjectOrNull ())
{
error = not_an_object (json_name);
return boost::none;
Expand Down Expand Up @@ -743,7 +743,7 @@ static boost::optional <STObject> parseObject (
case STI_TRANSACTION:
case STI_LEDGERENTRY:
case STI_VALIDATION:
if (! value.isObject ())
if (! value.isObjectOrNull ())
{
error = not_an_object (json_name, fieldName);
return boost::none;
Expand Down Expand Up @@ -816,7 +816,7 @@ static boost::optional <detail::STVar> parseArray (
int depth,
Json::Value& error)
{
if (! json.isArray ())
if (! json.isArrayOrNull ())
{
error = not_an_array (json_name);
return boost::none;
Expand All @@ -834,11 +834,12 @@ static boost::optional <detail::STVar> parseArray (

for (Json::UInt i = 0; json.isValidIndex (i); ++i)
{
bool const isObject (json[i].isObject());
bool const singleKey (isObject ? json[i].size() == 1 : true);
bool const isObjectOrNull (json[i].isObjectOrNull());
bool const singleKey (isObjectOrNull ? json[i].size() == 1 : true);

if (!isObject || !singleKey)
if (!isObjectOrNull || !singleKey)
{
// null values are !singleKey
error = singleton_expected (json_name, i);
return boost::none;
}
Expand Down
11 changes: 5 additions & 6 deletions src/ripple/rpc/handlers/BookOffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,21 @@ Json::Value doBookOffers (RPC::Context& context)
if (!context.params.isMember (jss::taker_gets))
return RPC::missing_field_error (jss::taker_gets);

if (!context.params[jss::taker_pays].isObject ())
Json::Value const& taker_pays = context.params[jss::taker_pays];
Json::Value const& taker_gets = context.params[jss::taker_gets];

if (!taker_pays.isObjectOrNull ())
return RPC::object_field_error (jss::taker_pays);

if (!context.params[jss::taker_gets].isObject ())
if (!taker_gets.isObjectOrNull ())
return RPC::object_field_error (jss::taker_gets);

Json::Value const& taker_pays (context.params[jss::taker_pays]);

if (!taker_pays.isMember (jss::currency))
return RPC::missing_field_error ("taker_pays.currency");

if (! taker_pays [jss::currency].isString ())
return RPC::expected_field_error ("taker_pays.currency", "string");

Json::Value const& taker_gets = context.params[jss::taker_gets];

if (! taker_gets.isMember (jss::currency))
return RPC::missing_field_error ("taker_gets.currency");

Expand Down
3 changes: 2 additions & 1 deletion src/ripple/rpc/handlers/GatewayBalances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ Json::Value doGatewayBalances (RPC::Context& context)
Json::Value const& hw = params[jss::hotwallet];
bool valid = true;

if (hw.isArray())
// null is treated as a valid 0-sized array of hotwallet
if (hw.isArrayOrNull())
{
for (unsigned i = 0; i < hw.size(); ++i)
valid &= addHotWallet (hw[i]);
Expand Down
12 changes: 8 additions & 4 deletions src/ripple/rpc/handlers/LedgerEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ Json::Value doLedgerEntry (RPC::Context& context)
}
else if (context.params.isMember (jss::directory))
{
if (!context.params[jss::directory].isObject ())
if (context.params[jss::directory].isNull())
{
jvResult[jss::error] = "malformedRequest";
}
else if (!context.params[jss::directory].isObject())
{
uNodeIndex.SetHex (context.params[jss::directory].asString ());
}
Expand Down Expand Up @@ -114,7 +118,7 @@ Json::Value doLedgerEntry (RPC::Context& context)
}
else if (context.params.isMember (jss::offer))
{
if (!context.params[jss::offer].isObject ())
if (!context.params[jss::offer].isObject())
{
uNodeIndex.SetHex (context.params[jss::offer].asString ());
}
Expand All @@ -140,10 +144,10 @@ Json::Value doLedgerEntry (RPC::Context& context)
Currency uCurrency;
Json::Value jvRippleState = context.params[jss::ripple_state];

if (!jvRippleState.isObject ()
if (!jvRippleState.isObject()
|| !jvRippleState.isMember (jss::currency)
|| !jvRippleState.isMember (jss::accounts)
|| !jvRippleState[jss::accounts].isArray ()
|| !jvRippleState[jss::accounts].isArray()
|| 2 != jvRippleState[jss::accounts].size ()
|| !jvRippleState[jss::accounts][0u].isString ()
|| !jvRippleState[jss::accounts][1u].isString ()
Expand Down
6 changes: 3 additions & 3 deletions src/ripple/rpc/handlers/Subscribe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,11 @@ Json::Value doSubscribe (RPC::Context& context)

for (auto& j: context.params[jss::books])
{
if (!j.isObject ()
if (!j.isObject()
|| !j.isMember (jss::taker_pays)
|| !j.isMember (jss::taker_gets)
|| !j[jss::taker_pays].isObject ()
|| !j[jss::taker_gets].isObject ())
|| !j[jss::taker_pays].isObjectOrNull ()
|| !j[jss::taker_gets].isObjectOrNull ())
return rpcError (rpcINVALID_PARAMS);

Book book;
Expand Down
6 changes: 3 additions & 3 deletions src/ripple/rpc/handlers/Unsubscribe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Json::Value doUnsubscribe (RPC::Context& context)

if (context.params.isMember (jss::streams))
{
if (! context.params[jss::streams].isArray ())
if (! context.params[jss::streams].isArray())
return rpcError (rpcINVALID_PARAMS);

for (auto& it: context.params[jss::streams])
Expand Down Expand Up @@ -139,8 +139,8 @@ Json::Value doUnsubscribe (RPC::Context& context)
if (! jv.isObject() ||
! jv.isMember(jss::taker_pays) ||
! jv.isMember(jss::taker_gets) ||
! jv[jss::taker_pays].isObject() ||
! jv[jss::taker_gets].isObject())
! jv[jss::taker_pays].isObjectOrNull() ||
! jv[jss::taker_gets].isObjectOrNull())
{
return rpcError(rpcINVALID_PARAMS);
}
Expand Down
Loading

0 comments on commit 1a24523

Please sign in to comment.