Merge pull request #436 from biojppm/fix/434-deserialize_bad_vals

ensure empty values are not deserialized

changelog/current.md

@@ -5,7 +5,7 @@ Most of the changes are from the giant Parser refactor described below. Before g
 
 - [#PR431](https://github.com/biojppm/rapidyaml/pull/431) - Emitter: prevent stack overflows when emitting malicious trees by providing a max tree depth for the emit visitor. This was done by adding an `EmitOptions` structure as an argument both to the emitter and to the emit functions, which is then forwarded to the emitter. This `EmitOptions` structure has a max tree depth setting with a default value of 64.
 - [#PR431](https://github.com/biojppm/rapidyaml/pull/431) - Fix `_RYML_CB_ALLOC()` using `(T)` in parenthesis, making the macro unusable.
-
+- [#434] - Ensure empty vals are not deserialized ([#PR436](https://github.com/biojppm/rapidyaml/pull/436)).
 
 ### New features
 

samples/quickstart.cpp

@@ -3221,7 +3221,7 @@ QWxsIHRoYXQgZ2xpdHRlcnMgaXMgbm90IGdvbGQu: All that glitters is not gold.
         result.resize(len); // trim to the length of the decoded buffer
         CHECK(result == c.text);
         //
-        // Note also these are just syntatic wrappers to simplify client code.
+        // Note also these are just syntactic wrappers to simplify client code.
         // You can call into the lower level functions without much effort:
         result.clear(); // this is not needed. We do it just to show that the first call can fail.
         ryml::csubstr encoded = tree[c.base64].key();

src/c4/yml/common.hpp

@@ -34,7 +34,7 @@
 
 #ifndef RYML_LOGBUF_SIZE_MAX
 /// size for the fallback larger log buffer. When @ref
-/// RYML_LOGBUG_SIZE is not large enough to convert a value to string,
+/// RYML_LOGBUF_SIZE is not large enough to convert a value to string,
 /// then temporary stack memory is allocated up to
 /// RYML_LOGBUF_SIZE_MAX. This limit is in place to prevent a stack
 /// overflow. If the printed value requires more than

src/c4/yml/node.hpp

@@ -603,7 +603,7 @@ struct RoNodeMethods
         _RYML_CB_CHECK(tree_->m_callbacks, (pos >= 0 && pos < cap));
         _RYML_CB_CHECK(tree_->m_callbacks, ((Impl const*)this)->readable());
         _RYML_CB_CHECK(tree_->m_callbacks, tree_->is_container(id_));
-        id_type ch = tree_->child(id_, pos);
+        const id_type ch = tree_->child(id_, pos);
         _RYML_CB_CHECK(tree_->m_callbacks, ch != NONE);
         return {tree_, ch};
     }
@@ -615,7 +615,8 @@ struct RoNodeMethods
     /** @name deserialization */
     /** @{ */
 
-    /** deserialize the node's val to the given variable */
+    /** deserialize the node's val to the given variable, forwarding
+     * to the user-overrideable @ref read() function. */
     template<class T>
     ConstImpl const& operator>> (T &v) const
     {
@@ -625,13 +626,14 @@ struct RoNodeMethods
         return *((ConstImpl const*)this);
     }
 
-    /** deserialize the node's key to the given variable; use @ref key()
+    /** deserialize the node's key to the given variable, forwarding
+     * to the user-overrideable @ref read() function; use @ref key()
      * to disambiguate; for example: `node >> ryml::key(var)` */
     template<class T>
     ConstImpl const& operator>> (Key<T> v) const
     {
         _C4RR();
-        if( ! from_chars(key(), &v.k))
+        if(key().empty() || ! from_chars(key(), &v.k))
             _RYML_CB_ERR(tree_->m_callbacks, "could not deserialize key");
         return *((ConstImpl const*)this);
     }
@@ -1602,30 +1604,94 @@ inline void write(NodeRef *n, T const& v)
     n->set_val_serialized(v);
 }
 
+/** convert the val of a scalar node to a particular type, by
+ * forwarding its val to @ref from_chars<T>(). The full string is
+ * used.
+ * @return false if the conversion failed */
 template<class T>
 typename std::enable_if< ! std::is_floating_point<T>::value, bool>::type
 inline read(NodeRef const& n, T *v)
 {
-    return from_chars(n.val(), v);
+    csubstr val = n.val();
+    if(val.empty())
+        return false;
+    return from_chars(val, v);
 }
+/** convert the val of a scalar node to a particular type, by
+ * forwarding its val to @ref from_chars<T>(). The full string is
+ * used.
+ * @return false if the conversion failed */
 template<class T>
 typename std::enable_if< ! std::is_floating_point<T>::value, bool>::type
 inline read(ConstNodeRef const& n, T *v)
 {
-    return from_chars(n.val(), v);
+    csubstr val = n.val();
+    if(val.empty())
+        return false;
+    return from_chars(val, v);
 }
 
+/** convert the val of a scalar node to a floating point type, by
+ * forwarding its val to @ref from_chars_float<T>().
+ *
+ * @return false if the conversion failed
+ *
+ * @warning Unlike non-floating types, only the leading part of the
+ * string that may constitute a number is processed. This happens
+ * because the float parsing is delegated to fast_float, which is
+ * implemented that way. Consequently, for example, all of `"34"`,
+ * `"34 "` `"34hg"` `"34 gh"` will be read as 34. If you are not sure
+ * about the contents of the data, you can use
+ * csubstr::first_real_span() to check before calling `>>`, for
+ * example like this:
+ *
+ * ```cpp
+ * csubstr val = node.val();
+ * if(val.first_real_span() == val)
+ *     node >> v;
+ * else
+ *     ERROR("not a real")
+ * ```
+ */
 template<class T>
 typename std::enable_if<std::is_floating_point<T>::value, bool>::type
 inline read(NodeRef const& n, T *v)
 {
-    return from_chars_float(n.val(), v);
+    csubstr val = n.val();
+    if(val.empty())
+        return false;
+    return from_chars_float(val, v);
 }
+/** convert the val of a scalar node to a floating point type, by
+ * forwarding its val to @ref from_chars_float<T>().
+ *
+ * @return false if the conversion failed
+ *
+ * @warning Unlike non-floating types, only the leading part of the
+ * string that may constitute a number is processed. This happens
+ * because the float parsing is delegated to fast_float, which is
+ * implemented that way. Consequently, for example, all of `"34"`,
+ * `"34 "` `"34hg"` `"34 gh"` will be read as 34. If you are not sure
+ * about the contents of the data, you can use
+ * csubstr::first_real_span() to check before calling `>>`, for
+ * example like this:
+ *
+ * ```cpp
+ * csubstr val = node.val();
+ * if(val.first_real_span() == val)
+ *     node >> v;
+ * else
+ *     ERROR("not a real")
+ * ```
+ */
 template<class T>
 typename std::enable_if<std::is_floating_point<T>::value, bool>::type
 inline read(ConstNodeRef const& n, T *v)
 {
-    return from_chars_float(n.val(), v);
+    csubstr val = n.val();
+    if(val.empty())
+        return false;
+    return from_chars_float(val, v);
 }
 
 /** @} */

test/test_serialize.cpp

@@ -517,6 +517,126 @@ TEST(serialize, create_anchor_ref_trip)
     EXPECT_EQ(emitrs_yaml<std::string>(tree), expected_yaml);
 }
 
+TEST(deserialize, issue434_0)
+{
+    Tree tree = parse_in_arena("{empty: }");
+    ConstNodeRef cnode = tree["empty"];
+    NodeRef node = tree["empty"];
+    {
+        int value = 0;
+        EXPECT_FALSE(read(cnode, &value));
+    }
+    {
+        int value = 0;
+        EXPECT_FALSE(read(node, &value));
+    }
+    ExpectError::do_check(&tree, [&]{
+        int value = 0;
+        cnode >> value;
+    });
+    ExpectError::do_check(&tree, [&]{
+        int value = 0;
+        node >> value;
+    });
+    {
+        double value = 0;
+        EXPECT_FALSE(read(cnode, &value));
+    }
+    {
+        double value = 0;
+        EXPECT_FALSE(read(node, &value));
+    }
+    ExpectError::do_check(&tree, [&]{
+        double value = 0;
+        cnode >> value;
+    });
+    ExpectError::do_check(&tree, [&]{
+        double value = 0;
+        node >> value;
+    });
+}
+
+void test_deserialize_trailing_434(csubstr yaml, csubstr val, csubstr first, double dval)
+{
+    Tree tree = parse_in_arena(yaml);
+    ASSERT_EQ(tree["val"].val(), val);
+    EXPECT_EQ(tree["val"].val().first_int_span(), first);
+    EXPECT_EQ(tree["val"].val().first_real_span(), first);
+    ConstNodeRef cnode = tree["val"];
+    NodeRef node = tree["val"];
+    {
+        int value;
+        EXPECT_FALSE(read(cnode, &value));
+    }
+    {
+        int value;
+        EXPECT_FALSE(read(node, &value));
+    }
+    ExpectError::do_check(&tree, [&]{
+        int value = 1;
+        cnode >> value;
+    });
+    ExpectError::do_check(&tree, [&]{
+        int value = 1;
+        node >> value;
+    });
+    float fval = (float)dval;
+    {
+        float value;
+        EXPECT_TRUE(read(cnode, &value));
+        EXPECT_EQ(memcmp(&value, &fval, sizeof(fval)), 0);
+    }
+    {
+        float value;
+        EXPECT_TRUE(read(node, &value));
+        EXPECT_EQ(memcmp(&value, &fval, sizeof(fval)), 0);
+    }
+    {
+        double value;
+        EXPECT_TRUE(read(cnode, &value));
+        EXPECT_EQ(memcmp(&value, &dval, sizeof(dval)), 0);
+    }
+    {
+        double value;
+        EXPECT_TRUE(read(node, &value));
+        EXPECT_EQ(memcmp(&value, &dval, sizeof(dval)), 0);
+    }
+    {
+        float value = 1;
+        cnode >> value;
+        EXPECT_EQ(memcmp(&value, &fval, sizeof(fval)), 0);
+    }
+    {
+        float value = 1;
+        node >> value;
+        EXPECT_EQ(memcmp(&value, &fval, sizeof(fval)), 0);
+    }
+    {
+        double value = 1;
+        cnode >> value;
+        EXPECT_EQ(memcmp(&value, &dval, sizeof(fval)), 0);
+    }
+    {
+        double value = 1;
+        node >> value;
+        EXPECT_EQ(memcmp(&value, &dval, sizeof(fval)), 0);
+    }
+}
+TEST(deserialize, issue434_1)
+{
+    test_deserialize_trailing_434("{val: 0.r3}", "0.r3", "", 0.0);
+}
+
+TEST(deserialize, issue434_2)
+{
+    test_deserialize_trailing_434("{val: 34gf3}", "34gf3", "", 34.0);
+}
+
+TEST(deserialize, issue434_3)
+{
+    test_deserialize_trailing_434("{val: 34 gf3}", "34 gf3", "34", 34.0);
+}
+
 
 //-------------------------------------------
 // this is needed to use the test case library