diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index ded64b7d7..6fba59095 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -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"; diff --git a/src/ripple/app/misc/impl/AccountTxPaging.cpp b/src/ripple/app/misc/impl/AccountTxPaging.cpp index 6ce0ed901..7d4d5f1c0 100644 --- a/src/ripple/app/misc/impl/AccountTxPaging.cpp +++ b/src/ripple/app/misc/impl/AccountTxPaging.cpp @@ -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; diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index fca5448ca..c4307ef3c 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -223,7 +223,7 @@ ValidatorList::applyList ( std::vector manifests; for (auto const& val : newList) { - if (val.isObject () && + if (val.isObject() && val.isMember ("validation_public_key") && val["validation_public_key"].isString ()) { diff --git a/src/ripple/app/misc/impl/ValidatorSite.cpp b/src/ripple/app/misc/impl/ValidatorSite.cpp index 58052ef84..127095da7 100644 --- a/src/ripple/app/misc/impl/ValidatorSite.cpp +++ b/src/ripple/app/misc/impl/ValidatorSite.cpp @@ -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() && diff --git a/src/ripple/json/impl/Object.cpp b/src/ripple/json/impl/Object.cpp index 5a9d1ca82..67141172c 100644 --- a/src/ripple/json/impl/Object.cpp +++ b/src/ripple/json/impl/Object.cpp @@ -196,7 +196,7 @@ namespace { template 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]; diff --git a/src/ripple/json/impl/json_reader.cpp b/src/ripple/json/impl/json_reader.cpp index 45b2794c7..948fb4b35 100644 --- a/src/ripple/json/impl/json_reader.cpp +++ b/src/ripple/json/impl/json_reader.cpp @@ -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; diff --git a/src/ripple/json/impl/json_value.cpp b/src/ripple/json/impl/json_value.cpp index a9a965f58..b2fe80acc 100644 --- a/src/ripple/json/impl/json_value.cpp +++ b/src/ripple/json/impl/json_value.cpp @@ -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 @@ -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; } diff --git a/src/ripple/json/impl/json_writer.cpp b/src/ripple/json/impl/json_writer.cpp index 8a05e5556..f7625f6de 100644 --- a/src/ripple/json/impl/json_writer.cpp +++ b/src/ripple/json/impl/json_writer.cpp @@ -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 ); } @@ -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 ); } diff --git a/src/ripple/json/json_value.h b/src/ripple/json/json_value.h index 423942dd1..0aefe26d7 100644 --- a/src/ripple/json/json_value.h +++ b/src/ripple/json/json_value.h @@ -262,8 +262,10 @@ public: 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; diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index 360cbc2e6..4450a2812 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -173,7 +173,7 @@ private: { Json::Value v (Json::objectValue); - if (jvParams.isArray () && (jvParams.size () > 0)) + if (jvParams.isArray() && (jvParams.size () > 0)) v[jss::params] = jvParams; return v; @@ -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; } diff --git a/src/ripple/net/impl/RPCErr.cpp b/src/ripple/net/impl/RPCErr.cpp index 56e5728cc..b4a16e483 100644 --- a/src/ripple/net/impl/RPCErr.cpp +++ b/src/ripple/net/impl/RPCErr.cpp @@ -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 diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index f7288ecef..cec560d1c 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -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 ("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); @@ -846,7 +850,7 @@ amountFromJson (SField const& name, Json::Value const& v) if (native) { - if (v.isObject ()) + if (v.isObjectOrNull ()) Throw ("XRP may not be specified as an object"); issue = xrpIssue (); } diff --git a/src/ripple/protocol/impl/STParsedJSON.cpp b/src/ripple/protocol/impl/STParsedJSON.cpp index 603324884..72990b3d4 100644 --- a/src/ripple/protocol/impl/STParsedJSON.cpp +++ b/src/ripple/protocol/impl/STParsedJSON.cpp @@ -495,7 +495,7 @@ static boost::optional 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 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 parseLeaf ( { STPath p; - if (!value[i].isArray ()) + if (!value[i].isArrayOrNull ()) { std::stringstream ss; ss << fieldName << "[" << i << "]"; @@ -555,7 +555,7 @@ static boost::optional parseLeaf ( Json::Value pathEl = value[i][j]; - if (!pathEl.isObject ()) + if (!pathEl.isObject()) { error = not_an_object (element_name); return ret; @@ -709,7 +709,7 @@ static boost::optional 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 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 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 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; } diff --git a/src/ripple/rpc/handlers/BookOffers.cpp b/src/ripple/rpc/handlers/BookOffers.cpp index b63d2f436..6bc26019a 100644 --- a/src/ripple/rpc/handlers/BookOffers.cpp +++ b/src/ripple/rpc/handlers/BookOffers.cpp @@ -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"); diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 087ca2213..d6ce69157 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -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]); diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index d9d9b31cf..0e930d6a7 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -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 ()); } @@ -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 ()); } @@ -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 () diff --git a/src/ripple/rpc/handlers/Subscribe.cpp b/src/ripple/rpc/handlers/Subscribe.cpp index ecbf0b309..8af4e9129 100644 --- a/src/ripple/rpc/handlers/Subscribe.cpp +++ b/src/ripple/rpc/handlers/Subscribe.cpp @@ -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; diff --git a/src/ripple/rpc/handlers/Unsubscribe.cpp b/src/ripple/rpc/handlers/Unsubscribe.cpp index 2ec10c753..7c8141b58 100644 --- a/src/ripple/rpc/handlers/Unsubscribe.cpp +++ b/src/ripple/rpc/handlers/Unsubscribe.cpp @@ -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]) @@ -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); } diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index d42f32747..688b9b2b6 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -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, @@ -712,7 +711,7 @@ ServerHandlerImp::processRequest (Port const& port, if (! params) params = Json::Value (Json::objectValue); - else if (!params.isArray () || params.size() != 1) + else if (!params.isArray() || params.size() != 1) { usage.charge(Resource::feeInvalidRPC); HTTPReply (400, "params unparseable", output, rpcJ); @@ -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); diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 01eb9bfec..c3bf14a17 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -253,7 +253,7 @@ checkTxJsonFields ( { std::pair ret; - if (! tx_json.isObject ()) + if (!tx_json.isObject()) { ret.first = RPC::object_field_error (jss::tx_json); return ret; diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp index f301ab488..b9e9e5fd9 100644 --- a/src/test/app/Regression_test.cpp +++ b/src/test/app/Regression_test.cpp @@ -213,8 +213,7 @@ struct Regression_test : public beast::unit_test::suite std::vector 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 diff --git a/src/test/protocol/STObject_test.cpp b/src/test/protocol/STObject_test.cpp index 7c7d4305f..4ab578de4 100644 --- a/src/test/protocol/STObject_test.cpp +++ b/src/test/protocol/STObject_test.cpp @@ -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 () diff --git a/src/test/rpc/AmendmentBlocked_test.cpp b/src/test/rpc/AmendmentBlocked_test.cpp index cd68e76ba..b6ba0a9ea 100644 --- a/src/test/rpc/AmendmentBlocked_test.cpp +++ b/src/test/rpc/AmendmentBlocked_test.cpp @@ -64,7 +64,7 @@ class AmendmentBlocked_test : public beast::unit_test::suite pf_req[jss::destination_amount] = bob["USD"](20).value ().getJson (0); jr = wsc->invoke("path_find", pf_req) [jss::result]; BEAST_EXPECT (jr.isMember (jss::alternatives) && - jr[jss::alternatives].isArray () && + jr[jss::alternatives].isArray() && jr[jss::alternatives].size () == 1); // submit diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index 3e99db0ae..b3dc646e7 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -462,11 +462,16 @@ public: BEAST_EXPECT(jr[jss::error_message] == "You don't have permission for this command."); } + std::initializer_list 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.");