STObject::applyTemplate() throws with description of error:

The `STObject` member function `setType()` has been renamed to
applyTemplate() and modified to throw if there is a template
mismatch.

The error description in the exception is, in certain cases,
used, to better indicate why a particular transaction was
considered ill formed.

Fixes #2585.
This commit is contained in:
Scott Schurr
2018-11-02 17:50:27 -07:00
committed by Nik Bougalis
parent c354809e1c
commit ad5c5f1969
12 changed files with 221 additions and 132 deletions

View File

@@ -40,28 +40,6 @@ namespace ripple {
class STArray;
/** Thrown on illegal access to non-present SField. */
struct missing_field_error : std::logic_error
{
explicit
missing_field_error (SField const& f)
: logic_error(
"missing field '" + f.getName() + "'")
{
}
};
/** Thrown on a field template violation. */
struct template_field_error : std::logic_error
{
explicit
template_field_error (SField const& f)
: logic_error(
"template field error '" + f.getName() + "'")
{
}
};
//------------------------------------------------------------------------------
class STObject
@@ -140,7 +118,7 @@ private:
Throws:
missing_field_error if !engaged()
STObject::FieldErr if !engaged()
*/
value_type operator*() const;
@@ -277,14 +255,21 @@ public:
using iterator = boost::transform_iterator<
Transform, STObject::list_type::const_iterator>;
class FieldErr : public std::runtime_error
{
using std::runtime_error::runtime_error;
};
static char const* getCountedObjectName () { return "STObject"; }
STObject(STObject&&);
STObject(STObject const&) = default;
STObject (const SOTemplate & type, SField const& name);
STObject (const SOTemplate & type, SerialIter & sit, SField const& name);
STObject (SerialIter& sit, SField const& name, int depth = 0);
STObject (SerialIter&& sit, SField const& name)
STObject (const SOTemplate& type,
SerialIter& sit, SField const& name) noexcept (false);
STObject (SerialIter& sit,
SField const& name, int depth = 0) noexcept (false);
STObject (SerialIter&& sit, SField const& name) noexcept (false)
: STObject(sit, name)
{
}
@@ -327,12 +312,9 @@ public:
v_.reserve (n);
}
bool setType (const SOTemplate & type);
void applyTemplate (const SOTemplate & type) noexcept (false);
enum ResultOfSetTypeFromSField : unsigned char
{typeSetFail, typeIsSet, noTemplate};
ResultOfSetTypeFromSField setTypeFromSField (SField const&);
void applyTemplateFromSField (SField const&) noexcept (false);
bool isFree () const
{
@@ -446,7 +428,7 @@ public:
Throws:
missing_field_error if the field is
STObject::FieldErr if the field is
not present.
*/
template<class T>
@@ -465,7 +447,7 @@ public:
Throws:
missing_field_error if the field is
STObject::FieldErr if the field is
not present.
*/
template<class T>
@@ -499,6 +481,7 @@ public:
void setAccountID (SField const& field, AccountID const&);
void setFieldAmount (SField const& field, STAmount const&);
void setFieldPathSet (SField const& field, STPathSet const&);
void setFieldV256 (SField const& field, STVector256 const& v);
void setFieldArray (SField const& field, STArray const& v);
@@ -693,7 +676,8 @@ STObject::Proxy<T>::Proxy (STObject* st, TypedField<T> const* f)
{
// STObject has associated template
if (! st_->peekAtPField(*f_))
Throw<template_field_error> (*f);
Throw<STObject::FieldErr> (
"Template field error '" + this->f_->getName() + "'");
style_ = st_->mType->style(*f_);
}
else
@@ -711,7 +695,8 @@ STObject::Proxy<T>::value() const ->
if (t)
return t->value();
if (style_ != SOE_DEFAULT)
Throw<missing_field_error> (*f_);
Throw<STObject::FieldErr> (
"Missing field '" + this->f_->getName() + "'");
return value_type{};
}
@@ -867,7 +852,8 @@ STObject::OptionalProxy<T>::disengage()
{
if (this->style_ == SOE_REQUIRED ||
this->style_ == SOE_DEFAULT)
Throw<template_field_error> (*this->f_);
Throw<STObject::FieldErr> (
"Template field error '" + this->f_->getName() + "'");
if (this->style_ == SOE_INVALID)
this->st_->delField(*this->f_);
else
@@ -894,7 +880,8 @@ STObject::operator[](TypedField<T> const& f) const
if (! b)
// This is a free object (no constraints)
// with no template
Throw<missing_field_error> (f);
Throw<STObject::FieldErr> (
"Missing field '" + f.getName() + "'");
auto const u =
dynamic_cast<T const*>(b);
if (! u)
@@ -902,7 +889,8 @@ STObject::operator[](TypedField<T> const& f) const
assert(mType);
assert(b->getSType() == STI_NOTPRESENT);
if(mType->style(f) == SOE_OPTIONAL)
Throw<missing_field_error> (f);
Throw<STObject::FieldErr> (
"Missing field '" + f.getName() + "'");
assert(mType->style(f) == SOE_DEFAULT);
// Handle the case where value_type is a
// const reference, otherwise we return

View File

@@ -56,10 +56,10 @@ public:
STTx (STTx const& other) = default;
explicit STTx (SerialIter& sit);
explicit STTx (SerialIter&& sit) : STTx(sit) {}
explicit STTx (SerialIter& sit) noexcept (false);
explicit STTx (SerialIter&& sit) noexcept (false) : STTx(sit) {}
explicit STTx (STObject&& object);
explicit STTx (STObject&& object) noexcept (false);
/** Constructs a transaction.

View File

@@ -318,6 +318,14 @@ public:
{
}
// Infer the size of the data based on the size of the passed array.
template<int N>
explicit SerialIter (std::uint8_t const (&data)[N])
: SerialIter(&data[0], N)
{
static_assert (N > 0, "");
}
std::size_t
empty() const noexcept
{

View File

@@ -98,10 +98,7 @@ STArray::STArray (SerialIter& sit, SField const& f, int depth)
v_.emplace_back(sit, fn, depth+1);
if (v_.back().setTypeFromSField (fn) == STObject::typeSetFail)
{
Throw<std::runtime_error> ("Malformed object in array");
}
v_.back().applyTemplateFromSField (fn); // May throw
}
}

View File

@@ -73,17 +73,7 @@ void STLedgerEntry::setSLEType ()
Throw<std::runtime_error> ("invalid ledger entry type");
type_ = format->getType ();
if (!setType (format->elements))
{
if (auto j = debugLog().error())
{
j << "Ledger entry not valid for type " << format->getName ();
j << "Object: " << getJson (0);
}
Throw<std::runtime_error> ("ledger entry not valid for type");
}
applyTemplate (format->elements); // May throw
}
std::string STLedgerEntry::getFullText () const

View File

@@ -58,15 +58,16 @@ STObject::STObject (SOTemplate const& type,
}
STObject::STObject (SOTemplate const& type,
SerialIter & sit, SField const& name)
SerialIter & sit, SField const& name) noexcept (false)
: STBase (name)
{
v_.reserve(type.size());
set (sit);
setType (type);
applyTemplate (type); // May throw
}
STObject::STObject (SerialIter& sit, SField const& name, int depth)
STObject::STObject (
SerialIter& sit, SField const& name, int depth) noexcept (false)
: STBase(name)
, mType(nullptr)
{
@@ -99,9 +100,17 @@ void STObject::set (const SOTemplate& type)
}
}
bool STObject::setType (const SOTemplate& type)
void STObject::applyTemplate (const SOTemplate& type) noexcept (false)
{
bool valid = true;
auto throwFieldErr = [] (std::string const& field, char const* description)
{
std::stringstream ss;
ss << "Field '" << field << "' " << description;
std::string text {ss.str()};
JLOG (debugLog().error()) << "STObject::applyTemplate failed: " << text;
Throw<FieldErr> (text);
};
mType = &type;
decltype(v_) v;
v.reserve(type.size());
@@ -114,10 +123,8 @@ bool STObject::setType (const SOTemplate& type)
{
if ((e->flags == SOE_DEFAULT) && iter->get().isDefault())
{
JLOG (debugLog().error())
<< "setType(" << getFName().getName()
<< "): explicit default " << e->e_field.fieldName;
valid = false;
throwFieldErr (e->e_field.fieldName,
"may not be explicitly set to default.");
}
v.emplace_back(std::move(*iter));
v_.erase(iter);
@@ -126,10 +133,8 @@ bool STObject::setType (const SOTemplate& type)
{
if (e->flags == SOE_REQUIRED)
{
JLOG (debugLog().error())
<< "setType(" << getFName().getName()
<< "): missing " << e->e_field.fieldName;
valid = false;
throwFieldErr (e->e_field.fieldName,
"is required but missing.");
}
v.emplace_back(detail::nonPresentObject, e->e_field);
}
@@ -139,34 +144,25 @@ bool STObject::setType (const SOTemplate& type)
// Anything left over in the object must be discardable
if (! e->getFName().isDiscardable())
{
JLOG (debugLog().error())
<< "setType(" << getFName().getName()
<< "): non-discardable leftover " << e->getFName().getName ();
valid = false;
throwFieldErr (e->getFName().getName(),
"found in disallowed location.");
}
}
// Swap the template matching data in for the old data,
// freeing any leftover junk
v_.swap(v);
return valid;
}
STObject::ResultOfSetTypeFromSField
STObject::setTypeFromSField (SField const& sField)
void STObject::applyTemplateFromSField (SField const& sField) noexcept (false)
{
ResultOfSetTypeFromSField ret = noTemplate;
SOTemplate const* elements =
InnerObjectFormats::getInstance ().findSOTemplateBySField (sField);
if (elements)
{
ret = setType (*elements) ? typeIsSet : typeSetFail;
}
return ret;
applyTemplate (*elements); // May throw
}
// return true = terminated with end-of-object
bool STObject::set (SerialIter& sit, int depth)
bool STObject::set (SerialIter& sit, int depth) noexcept (false)
{
bool reachedEndOfObject = false;
@@ -211,10 +207,8 @@ bool STObject::set (SerialIter& sit, int depth)
// If the object type has a known SOTemplate then set it.
STObject* const obj = dynamic_cast <STObject*> (&(v_.back().get()));
if (obj && (obj->setTypeFromSField (fn) == typeSetFail))
{
Throw<std::runtime_error> ("field deserialization error");
}
if (obj)
obj->applyTemplateFromSField (fn); // May throw
}
}
@@ -628,6 +622,11 @@ void STObject::setFieldAmount (SField const& field, STAmount const& v)
setFieldUsingAssignment (field, v);
}
void STObject::setFieldPathSet (SField const& field, STPathSet const& v)
{
setFieldUsingAssignment (field, v);
}
void STObject::setFieldArray (SField const& field, STArray const& v)
{
setFieldUsingAssignment (field, v);

View File

@@ -804,20 +804,19 @@ static boost::optional <STObject> parseObject (
}
// Some inner object types have templates. Attempt to apply that.
if (data.setTypeFromSField (inName) == STObject::typeSetFail)
{
error = template_mismatch (inName);
return boost::none;
}
data.applyTemplateFromSField (inName); // May throw
return std::move (data);
}
catch (STObject::FieldErr const&)
{
error = template_mismatch (inName);
}
catch (std::exception const&)
{
error = invalid_data (json_name);
return boost::none;
}
return boost::none;
}
static boost::optional <detail::STVar> parseArray (

View File

@@ -55,18 +55,15 @@ auto getTxFormat (TxType type)
return format;
}
STTx::STTx (STObject&& object)
STTx::STTx (STObject&& object) noexcept (false)
: STObject (std::move (object))
{
tx_type_ = static_cast <TxType> (getFieldU16 (sfTransactionType));
if (!setType (getTxFormat (tx_type_)->elements))
Throw<std::runtime_error> ("transaction not valid");
applyTemplate (getTxFormat (tx_type_)->elements); // may throw
tid_ = getHash(HashPrefix::transactionID);
}
STTx::STTx (SerialIter& sit)
STTx::STTx (SerialIter& sit) noexcept (false)
: STObject (sfTransaction)
{
int length = sit.getBytesLeft ();
@@ -77,9 +74,7 @@ STTx::STTx (SerialIter& sit)
set (sit);
tx_type_ = static_cast<TxType> (getFieldU16 (sfTransactionType));
if (!setType (getTxFormat (tx_type_)->elements))
Throw<std::runtime_error> ("transaction not valid");
applyTemplate (getTxFormat (tx_type_)->elements); // May throw
tid_ = getHash(HashPrefix::transactionID);
}

View File

@@ -497,6 +497,10 @@ transactionPreProcessImpl (
stpTrans = std::make_shared<STTx> (
std::move (parsed.object.get()));
}
catch (STObject::FieldErr& err)
{
return RPC::make_error (rpcINVALID_PARAMS, err.what());
}
catch (std::exception&)
{
return RPC::make_error (rpcINTERNAL,
@@ -1098,6 +1102,10 @@ Json::Value transactionSubmitMultiSigned (
stpTrans = std::make_shared<STTx>(
std::move(parsedTx_json.object.get()));
}
catch (STObject::FieldErr& err)
{
return RPC::make_error (rpcINVALID_PARAMS, err.what());
}
catch (std::exception& ex)
{
std::string reason (ex.what ());

View File

@@ -363,7 +363,7 @@ public:
BEAST_EXPECT(st[sf1] == 1);
BEAST_EXPECT(st[sf2] == 2);
except<missing_field_error>([&]()
except<STObject::FieldErr>([&]()
{ st[sf3]; });
BEAST_EXPECT(*st[~sf1] == 1);
BEAST_EXPECT(*st[~sf2] == 2);

View File

@@ -17,9 +17,11 @@
*/
//==============================================================================
#include <ripple/protocol/STAmount.h>
#include <ripple/protocol/Sign.h>
#include <ripple/protocol/STTx.h>
#include <ripple/protocol/STParsedJSON.h>
#include <ripple/protocol/TxFormats.h>
#include <ripple/protocol/UintTypes.h>
#include <ripple/json/to_string.h>
#include <ripple/beast/unit_test.h>
@@ -41,6 +43,9 @@ public:
testcase ("ed25519 signatures");
testSTTx (KeyType::ed25519);
testcase ("STObject constructor errors");
testObjectCtorErrors();
}
void testDeepNesting()
@@ -1229,7 +1234,7 @@ public:
try
{
ripple::SerialIter sit (Slice{payload2, sizeof(payload2)});
ripple::SerialIter sit {payload2};
auto stx = std::make_shared<ripple::STTx const>(sit);
fail("An exception should have been thrown");
}
@@ -1287,6 +1292,95 @@ public:
pass ();
}
}
void testObjectCtorErrors ()
{
auto const kp1 = randomKeyPair (KeyType::secp256k1);
auto const id1 = calcAccountID(kp1.first);
auto const kp2 = randomKeyPair (KeyType::secp256k1);
auto const id2 = calcAccountID(kp2.first);
// Lambda that returns a Payment STObject.
auto getPayment = [kp1, id1, id2] ()
{
// Account id1 pays account id2 10,000 XRP.
STObject payment (sfGeneric);
payment.setFieldU16 (sfTransactionType, ttPAYMENT);
payment.setAccountID (sfAccount, id1);
payment.setAccountID (sfDestination, id2);
payment.setFieldAmount (sfAmount, STAmount (10000000000ull));
payment.setFieldAmount (sfFee, STAmount (10ull));
payment.setFieldU32 (sfSequence, 1);
payment.setFieldVL (
sfSigningPubKey, Slice (kp1.first.data(), kp1.first.size()));
return payment;
};
{
// Verify that getPayment() returns a viable Payment.
std::string got;
try
{
STTx {getPayment()};
}
catch (STTx::FieldErr const& err)
{
got = err.what();
}
BEAST_EXPECT (got.empty());
}
{
// Make a payment with a defaulted PathSet field, which is invalid.
STObject defaultPath {getPayment()};
defaultPath.setFieldPathSet (sfPaths, STPathSet{});
std::string got;
try
{
STTx {std::move (defaultPath)};
}
catch (STTx::FieldErr const& err)
{
got = err.what();
}
BEAST_EXPECT (
got == "Field 'Paths' may not be explicitly set to default.");
}
{
// Make a Payment with an extra "SignerWeight" field.
STObject extraField {getPayment()};
extraField.setFieldU16 (sfSignerWeight, 7);
std::string got;
try
{
STTx {std::move (extraField)};
}
catch (STTx::FieldErr const& err)
{
got = err.what();
}
BEAST_EXPECT (
got == "Field 'SignerWeight' found in disallowed location.");
}
{
// Make a Payment that is missing the required Fee field.
STObject extraField {getPayment()};
extraField.delField (sfFee);
std::string got;
try
{
STTx {std::move (extraField)};
}
catch (STTx::FieldErr const& err)
{
got = err.what();
}
BEAST_EXPECT (
got == "Field 'Fee' is required but missing.");
}
}
};
class InnerObjectFormatsSerializer_test : public beast::unit_test::suite

View File

@@ -38,34 +38,43 @@ public:
{
testcase ("Deserialization");
constexpr unsigned char payload1[] =
{ // specifies an Ed25519 public key
0x72, 0x00, 0x73, 0x21, 0xed, 0x78, 0x00, 0xe6, 0x73, 0x00, 0x72, 0x00, 0x3c, 0x00, 0x00, 0x00,
0x88, 0x00, 0xe6, 0x73, 0x38, 0x00, 0x00, 0x8a, 0x00, 0x88, 0x4e, 0x31, 0x30, 0x5f, 0x5f, 0x63,
0x78, 0x78, 0x61, 0x62, 0x69, 0x76, 0x31, 0x30, 0x37, 0x5f, 0x5f, 0x63, 0x6c, 0x61, 0x73, 0x73,
0x5f, 0x74, 0x79, 0x70, 0x65, 0x5f, 0x69, 0x6e, 0x66, 0x6f, 0x45, 0x00, 0xe6, 0x88, 0x54, 0x72,
0x75, 0x73, 0x74, 0x53, 0x65, 0x74, 0x65, 0x61, 0x74, 0x65, 0x88, 0x00, 0xe6, 0x88, 0x00, 0xe6,
0x73, 0x00, 0x72, 0x00, 0x8a, 0x00, 0x88, 0x00, 0xe6
};
constexpr unsigned char payload2[] =
constexpr std::uint8_t payload1[] =
{
0x73, 0x21, 0xed, 0xff, 0x03, 0x1c, 0xbe, 0x65, 0x22, 0x61, 0x9c, 0x5e, 0x13, 0x12, 0x00, 0x3b,
0x43, 0x00, 0x00, 0x00, 0xf7, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x13, 0x13, 0x13,
0x3a, 0x27, 0xff
// specifies an Ed25519 public key.
0x22, 0x00, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00, 0x00, 0x51,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2a, 0x73,
0x21, 0xed, 0x78, 0x00, 0xe6, 0x73, 0x00, 0x72, 0x00, 0x3c, 0x00,
0x00, 0x00, 0x88, 0x00, 0xe6, 0x73, 0x38, 0x00, 0x00, 0x8a, 0x00,
0x88, 0x4e, 0x31, 0x30, 0x5f, 0x5f, 0x63, 0x78, 0x78, 0x61, 0x62,
0x69
};
constexpr unsigned char payload3[] =
{ // Has no public key at all
0x72, 0x00, 0x76, 0x31, 0x30, 0x37, 0x5f, 0x5f, 0x63, 0x6c, 0x61, 0x73, 0x73, 0x5f, 0x74, 0x79,
0x70, 0x65, 0x5f, 0x69, 0x6e, 0x66, 0x6f, 0x45, 0x00, 0xe6, 0x88, 0x54, 0x72, 0x75, 0x73, 0x74,
0x53, 0x65, 0x74, 0x65, 0x61, 0x74, 0x65, 0x88, 0x00, 0xe6, 0x88, 0x00, 0xe6, 0x73, 0x00, 0x72,
0x00, 0x8a, 0x00, 0x88, 0x00, 0xe6
constexpr std::uint8_t payload2[] =
{
0x22, 0x00, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00, 0x00, 0x51,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2a, 0x73,
0x21, 0xed, 0xff, 0x03, 0x1c, 0xbe, 0x65, 0x22, 0x61, 0x9c, 0x5e,
0x13, 0x12, 0x00, 0x3b, 0x43, 0x00, 0x00, 0x00, 0xf7, 0x3f, 0x3f,
0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x13, 0x13, 0x13, 0x3a, 0x27,
0xff
};
constexpr std::uint8_t payload3[] =
{
// Has no public key at all
0x22, 0x00, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00, 0x00, 0x51,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2a
};
try
{
SerialIter sit{payload1, sizeof(payload1)};
SerialIter sit {payload1};
auto stx = std::make_shared<ripple::STValidation>(sit,
[](PublicKey const& pk) {
return calcNodeID(pk);
@@ -74,12 +83,13 @@ public:
}
catch (std::exception const& e)
{
BEAST_EXPECT(strcmp(e.what(), "Invalid public key in validation") == 0);
BEAST_EXPECT(strcmp(e.what(),
"Invalid public key in validation") == 0);
}
try
{
SerialIter sit{payload2, sizeof(payload2)};
SerialIter sit {payload2};
auto stx = std::make_shared<ripple::STValidation>(sit,
[](PublicKey const& pk) {
return calcNodeID(pk);
@@ -88,12 +98,13 @@ public:
}
catch (std::exception const& e)
{
BEAST_EXPECT(strcmp(e.what(), "Invalid public key in validation") == 0);
BEAST_EXPECT(strcmp(e.what(),
"Invalid public key in validation") == 0);
}
try
{
SerialIter sit{payload3, sizeof(payload3)};
SerialIter sit {payload3};
auto stx = std::make_shared<ripple::STValidation>(sit,
[](PublicKey const& pk) {
return calcNodeID(pk);
@@ -102,8 +113,8 @@ public:
}
catch (std::exception const& e)
{
log << e.what() << "\n";
BEAST_EXPECT(strcmp(e.what(), "Invalid public key in validation") == 0);
BEAST_EXPECT(strcmp(e.what(),
"Field 'SigningPubKey' is required but missing.") == 0);
}
}