Cleanup some Json::Value methods:

* 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
This commit is contained in:
Howard Hinnant
2018-02-01 15:23:41 -05:00
committed by Mike Ellery
parent 20defb4844
commit 1a245234f1
24 changed files with 90 additions and 63 deletions

View File

@@ -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";
@@ -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";

View File

@@ -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;

View File

@@ -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];

View File

@@ -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;

View File

@@ -779,7 +779,7 @@ Value::operator bool () const
if (isString ())
{
auto s = asCString();
return s && strlen(s);
return s && s[0];
}
return ! (isArray() || isObject()) || size ();
@@ -1093,6 +1093,12 @@ Value::isString () const
bool
Value::isArray() const
{
return type_ == arrayValue;
}
bool
Value::isArrayOrNull () const
{
return type_ == nullValue || type_ == arrayValue;
}
@@ -1100,6 +1106,12 @@ Value::isArray () const
bool
Value::isObject() const
{
return type_ == objectValue;
}
bool
Value::isObjectOrNull () const
{
return type_ == nullValue || type_ == objectValue;
}

View File

@@ -263,7 +263,9 @@ public:
bool isNumeric () const;
bool isString () const;
bool isArray() const;
bool isArrayOrNull () const;
bool isObject() const;
bool isObjectOrNull () const;
bool isConvertibleTo ( ValueType other ) const;

View File

@@ -513,7 +513,7 @@ private:
if (reader.parse (jvParams[1u].asString (), jvRequest))
{
if (!jvRequest.isObject ())
if (!jvRequest.isObjectOrNull ())
return rpcError (rpcINVALID_PARAMS);
jvRequest[jss::method] = jvParams[0u];
@@ -544,7 +544,8 @@ private:
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;
}

View File

@@ -806,7 +806,11 @@ 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];
@@ -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 ();
}

View File

@@ -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;
@@ -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;
@@ -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 << "]";
@@ -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;
@@ -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;
@@ -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;
@@ -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;
}

View File

@@ -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");

View File

@@ -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]);

View File

@@ -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 ());
}

View File

@@ -195,8 +195,8 @@ Json::Value doSubscribe (RPC::Context& context)
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;

View File

@@ -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);
}

View File

@@ -334,7 +334,6 @@ ServerHandlerImp::onWSMessage(
auto const size = boost::asio::buffer_size(buffers);
if (size > RPC::Tuning::maxRequestSize ||
! Json::Reader{}.parse(jv, buffers) ||
! jv ||
! jv.isObject())
{
Json::Value jvResult(Json::objectValue);
@@ -603,7 +602,7 @@ ServerHandlerImp::processRequest (Port const& port,
if (jsonRPC.isMember(jss::params) &&
jsonRPC[jss::params].isArray() &&
jsonRPC[jss::params].size() > 0 &&
jsonRPC[jss::params][Json::UInt(0)].isObject())
jsonRPC[jss::params][Json::UInt(0)].isObjectOrNull())
{
role = requestRole(
required,
@@ -721,7 +720,7 @@ ServerHandlerImp::processRequest (Port const& port,
else
{
params = std::move (params[0u]);
if (!params.isObject())
if (!params.isObjectOrNull())
{
usage.charge(Resource::feeInvalidRPC);
HTTPReply (400, "params unparseable", output, rpcJ);

View File

@@ -213,8 +213,7 @@ struct Regression_test : public beast::unit_test::suite
std::vector<boost::asio::const_buffer> buffers;
buffers.emplace_back(buffer(request, 1024));
buffers.emplace_back(buffer(request.data() + 1024, request.length() - 1024));
BEAST_EXPECT(jrReader.parse(jvRequest, buffers) &&
jvRequest && jvRequest.isObject());
BEAST_EXPECT(jrReader.parse(jvRequest, buffers) && jvRequest.isObject());
}
void run() override

View File

@@ -38,9 +38,7 @@ public:
bool parseJSONString (std::string const& json, Json::Value& to)
{
Json::Reader reader;
return reader.parse(json, to) &&
bool (to) &&
to.isObject();
return reader.parse(json, to) && to.isObject();
}
void testParseJSONArrayWithInvalidChildrenObjects ()

View File

@@ -462,11 +462,16 @@ public:
BEAST_EXPECT(jr[jss::error_message] == "You don't have permission for this command.");
}
std::initializer_list<Json::Value> const nonArrays {Json::nullValue,
Json::intValue, Json::uintValue, Json::realValue, "",
Json::booleanValue, Json::objectValue};
for (auto const& f : {jss::accounts_proposed, jss::accounts})
{
for (auto const& nonArray : nonArrays)
{
Json::Value jv;
jv[f] = "";
jv[f] = nonArray;
auto jr = wsc->invoke(method, jv) [jss::result];
BEAST_EXPECT(jr[jss::error] == "invalidParams");
BEAST_EXPECT(jr[jss::error_message] == "Invalid parameters.");
@@ -481,9 +486,10 @@ public:
}
}
for (auto const& nonArray : nonArrays)
{
Json::Value jv;
jv[jss::books] = "";
jv[jss::books] = nonArray;
auto jr = wsc->invoke(method, jv) [jss::result];
BEAST_EXPECT(jr[jss::error] == "invalidParams");
BEAST_EXPECT(jr[jss::error_message] == "Invalid parameters.");
@@ -608,9 +614,10 @@ public:
BEAST_EXPECT(jr[jss::error_message] == "No such market.");
}
for (auto const& nonArray : nonArrays)
{
Json::Value jv;
jv[jss::streams] = "";
jv[jss::streams] = nonArray;
auto jr = wsc->invoke(method, jv) [jss::result];
BEAST_EXPECT(jr[jss::error] == "invalidParams");
BEAST_EXPECT(jr[jss::error_message] == "Invalid parameters.");