partial bitcoin#25551: Throw exception on invalid Univalue pushes over silent ignore

kwvg · kwvg · commit 94bbbf1b4fc3 · 2025-07-23T05:04:39.000Z
excludes: - ccccc17 (impacts code that is SegWit-specific)
diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h
@@ -82,14 +82,14 @@ class UniValue {
     bool isArray() const { return (typ == VARR); }
     bool isObject() const { return (typ == VOBJ); }
 
-    bool push_back(const UniValue& val);
-    bool push_backV(const std::vector<UniValue>& vec);
+    void push_back(const UniValue& val);
+    void push_backV(const std::vector<UniValue>& vec);
     template <class It>
-    bool push_backV(It first, It last);
+    void push_backV(It first, It last);
 
     void __pushKV(const std::string& key, const UniValue& val);
-    bool pushKV(const std::string& key, const UniValue& val);
-    bool pushKVs(const UniValue& obj);
+    void pushKV(const std::string& key, const UniValue& val);
+    void pushKVs(const UniValue& obj);
 
     std::string write(unsigned int prettyIndent = 0,
                       unsigned int indentLevel = 0) const;
@@ -140,11 +140,10 @@ class UniValue {
 };
 
 template <class It>
-bool UniValue::push_backV(It first, It last)
+void UniValue::push_backV(It first, It last)
 {
-    if (typ != VARR) return false;
+    if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
     values.insert(values.end(), first, last);
-    return true;
 }
 
 enum jtokentype {
diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp
@@ -108,53 +108,45 @@ bool UniValue::setObject()
     return true;
 }
 
-bool UniValue::push_back(const UniValue& val_)
+void UniValue::push_back(const UniValue& val_)
 {
-    if (typ != VARR)
-        return false;
+    if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
 
     values.push_back(val_);
-    return true;
 }
 
-bool UniValue::push_backV(const std::vector<UniValue>& vec)
+void UniValue::push_backV(const std::vector<UniValue>& vec)
 {
-    if (typ != VARR)
-        return false;
+    if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
 
     values.insert(values.end(), vec.begin(), vec.end());
-
-    return true;
 }
 
 void UniValue::__pushKV(const std::string& key, const UniValue& val_)
 {
+    if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
+
     keys.push_back(key);
     values.push_back(val_);
 }
 
-bool UniValue::pushKV(const std::string& key, const UniValue& val_)
+void UniValue::pushKV(const std::string& key, const UniValue& val_)
 {
-    if (typ != VOBJ)
-        return false;
+    if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
 
     size_t idx;
     if (findKey(key, idx))
         values[idx] = val_;
     else
         __pushKV(key, val_);
-    return true;
 }
 
-bool UniValue::pushKVs(const UniValue& obj)
+void UniValue::pushKVs(const UniValue& obj)
 {
-    if (typ != VOBJ || obj.typ != VOBJ)
-        return false;
+    if (typ != VOBJ || obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
 
     for (size_t i = 0; i < obj.keys.size(); i++)
         __pushKV(obj.keys[i], obj.values.at(i));
-
-    return true;
 }
 
 void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const
diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp
@@ -85,6 +85,16 @@ BOOST_AUTO_TEST_CASE(univalue_constructor)
     BOOST_CHECK_EQUAL(v9.getValStr(), "zappa");
 }
 
+BOOST_AUTO_TEST_CASE(univalue_push_throw)
+{
+    UniValue j;
+    BOOST_CHECK_THROW(j.push_back(1), std::runtime_error);
+    BOOST_CHECK_THROW(j.push_backV({1}), std::runtime_error);
+    BOOST_CHECK_THROW(j.__pushKV("k", 1), std::runtime_error);
+    BOOST_CHECK_THROW(j.pushKV("k", 1), std::runtime_error);
+    BOOST_CHECK_THROW(j.pushKVs({}), std::runtime_error);
+}
+
 BOOST_AUTO_TEST_CASE(univalue_typecheck)
 {
     UniValue v1;
@@ -198,13 +208,13 @@ BOOST_AUTO_TEST_CASE(univalue_array)
     UniValue arr(UniValue::VARR);
 
     UniValue v((int64_t)1023LL);
-    BOOST_CHECK(arr.push_back(v));
+    arr.push_back(v);
 
     std::string vStr("zippy");
-    BOOST_CHECK(arr.push_back(vStr));
+    arr.push_back(vStr);
 
     const char *s = "pippy";
-    BOOST_CHECK(arr.push_back(s));
+    arr.push_back(s);
 
     std::vector<UniValue> vec;
     v.setStr("boing");
@@ -213,13 +223,13 @@ BOOST_AUTO_TEST_CASE(univalue_array)
     v.setStr("going");
     vec.push_back(v);
 
-    BOOST_CHECK(arr.push_backV(vec));
+    arr.push_backV(vec);
 
-    BOOST_CHECK(arr.push_back((uint64_t) 400ULL));
-    BOOST_CHECK(arr.push_back((int64_t) -400LL));
-    BOOST_CHECK(arr.push_back((int) -401));
-    BOOST_CHECK(arr.push_back(-40.1));
-    BOOST_CHECK(arr.push_back(true));
+    arr.push_back(uint64_t{400ULL});
+    arr.push_back(int64_t{-400LL});
+    arr.push_back(int{-401});
+    arr.push_back(-40.1);
+    arr.push_back(true);
 
     BOOST_CHECK_EQUAL(arr.empty(), false);
     BOOST_CHECK_EQUAL(arr.size(), 10);
@@ -260,39 +270,39 @@ BOOST_AUTO_TEST_CASE(univalue_object)
 
     strKey = "age";
     v.setInt(100);
-    BOOST_CHECK(obj.pushKV(strKey, v));
+    obj.pushKV(strKey, v);
 
     strKey = "first";
     strVal = "John";
-    BOOST_CHECK(obj.pushKV(strKey, strVal));
+    obj.pushKV(strKey, strVal);
 
     strKey = "last";
-    const char *cVal = "Smith";
-    BOOST_CHECK(obj.pushKV(strKey, cVal));
+    const char* cVal = "Smith";
+    obj.pushKV(strKey, cVal);
 
     strKey = "distance";
-    BOOST_CHECK(obj.pushKV(strKey, (int64_t) 25));
+    obj.pushKV(strKey, int64_t{25});
 
     strKey = "time";
-    BOOST_CHECK(obj.pushKV(strKey, (uint64_t) 3600));
+    obj.pushKV(strKey, uint64_t{3600});
 
     strKey = "calories";
-    BOOST_CHECK(obj.pushKV(strKey, (int) 12));
+    obj.pushKV(strKey, int{12});
 
     strKey = "temperature";
-    BOOST_CHECK(obj.pushKV(strKey, (double) 90.012));
+    obj.pushKV(strKey, double{90.012});
 
     strKey = "moon";
-    BOOST_CHECK(obj.pushKV(strKey, true));
+    obj.pushKV(strKey, true);
 
     strKey = "spoon";
-    BOOST_CHECK(obj.pushKV(strKey, false));
+    obj.pushKV(strKey, false);
 
     UniValue obj2(UniValue::VOBJ);
-    BOOST_CHECK(obj2.pushKV("cat1", 9000));
-    BOOST_CHECK(obj2.pushKV("cat2", 12345));
+    obj2.pushKV("cat1", 9000);
+    obj2.pushKV("cat2", 12345);
 
-    BOOST_CHECK(obj.pushKVs(obj2));
+    obj.pushKVs(obj2);
 
     BOOST_CHECK_EQUAL(obj.empty(), false);
     BOOST_CHECK_EQUAL(obj.size(), 11);

Original file line number	Diff line number	Diff line change
`@@ -108,53 +108,45 @@ bool UniValue::setObject()`
`108`	`108`	`return true;`
`109`	`109`	`}`
`110`	`110`
`111`		`-bool UniValue::push_back(const UniValue& val_)`
	`111`	`+void UniValue::push_back(const UniValue& val_)`
`112`	`112`	`{`
`113`		`- if (typ != VARR)`
`114`		`- return false;`
	`113`	`+ if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};`
`115`	`114`
`116`	`115`	`values.push_back(val_);`
`117`		`- return true;`
`118`	`116`	`}`
`119`	`117`
`120`		`-bool UniValue::push_backV(const std::vector<UniValue>& vec)`
	`118`	`+void UniValue::push_backV(const std::vector<UniValue>& vec)`
`121`	`119`	`{`
`122`		`- if (typ != VARR)`
`123`		`- return false;`
	`120`	`+ if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};`
`124`	`121`
`125`	`122`	`values.insert(values.end(), vec.begin(), vec.end());`
`126`		`-`
`127`		`- return true;`
`128`	`123`	`}`
`129`	`124`
`130`	`125`	`void UniValue::__pushKV(const std::string& key, const UniValue& val_)`
`131`	`126`	`{`
	`127`	`+ if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};`
	`128`	`+`
`132`	`129`	`keys.push_back(key);`
`133`	`130`	`values.push_back(val_);`
`134`	`131`	`}`
`135`	`132`
`136`		`-bool UniValue::pushKV(const std::string& key, const UniValue& val_)`
	`133`	`+void UniValue::pushKV(const std::string& key, const UniValue& val_)`
`137`	`134`	`{`
`138`		`- if (typ != VOBJ)`
`139`		`- return false;`
	`135`	`+ if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};`
`140`	`136`
`141`	`137`	`size_t idx;`
`142`	`138`	`if (findKey(key, idx))`
`143`	`139`	`values[idx] = val_;`
`144`	`140`	`else`
`145`	`141`	`__pushKV(key, val_);`
`146`		`- return true;`
`147`	`142`	`}`
`148`	`143`
`149`		`-bool UniValue::pushKVs(const UniValue& obj)`
	`144`	`+void UniValue::pushKVs(const UniValue& obj)`
`150`	`145`	`{`
`151`		`- if (typ != VOBJ \|\| obj.typ != VOBJ)`
`152`		`- return false;`
	`146`	`+ if (typ != VOBJ \|\| obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};`
`153`	`147`
`154`	`148`	`for (size_t i = 0; i < obj.keys.size(); i++)`
`155`	`149`	`__pushKV(obj.keys[i], obj.values.at(i));`
`156`		`-`
`157`		`- return true;`
`158`	`150`	`}`
`159`	`151`
`160`	`152`	`void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const`