Add better exception reporting when incorrectly dereferencing an iterator.

include/yaml-cpp/exceptions.h

@@ -55,6 +55,7 @@ namespace YAML
 		const char * const AMBIGUOUS_ANCHOR       = "cannot assign the same alias to multiple nodes";
 		const char * const UNKNOWN_ANCHOR         = "the referenced anchor is not defined";
 
+        const char * const INVALID_NODE           = "invalid node; this may result from using a map iterator as a sequence iterator, or vice-versa";
 		const char * const INVALID_SCALAR         = "invalid scalar";
 		const char * const KEY_NOT_FOUND          = "key not found";
         const char * const BAD_CONVERSION         = "bad conversion";
@@ -149,7 +150,13 @@ namespace YAML
 		return TypedKeyNotFound <T> (mark, key);
 	}
 
-	class BadConversion: public RepresentationException {
+	class InvalidNode: public RepresentationException {
+	public:
+		InvalidNode()
+        : RepresentationException(Mark::null_mark(), ErrorMsg::INVALID_NODE) {}
+	};
+
+    class BadConversion: public RepresentationException {
 	public:
 		BadConversion()
         : RepresentationException(Mark::null_mark(), ErrorMsg::BAD_CONVERSION) {}

include/yaml-cpp/node/impl.h

@@ -15,30 +15,34 @@
 
 namespace YAML
 {
-	inline Node::Node(): m_pNode(NULL)
+	inline Node::Node(): m_isValid(true), m_pNode(NULL)
 	{
 	}
 	
-	inline Node::Node(NodeType::value type): m_pMemory(new detail::memory_holder), m_pNode(&m_pMemory->create_node())
+	inline Node::Node(NodeType::value type): m_isValid(true), m_pMemory(new detail::memory_holder), m_pNode(&m_pMemory->create_node())
 	{
 		m_pNode->set_type(type);
 	}
 	
 	template<typename T>
-	inline Node::Node(const T& rhs): m_pMemory(new detail::memory_holder), m_pNode(&m_pMemory->create_node())
+	inline Node::Node(const T& rhs): m_isValid(true), m_pMemory(new detail::memory_holder), m_pNode(&m_pMemory->create_node())
 	{
 		Assign(rhs);
 	}
 	
-    inline Node::Node(const detail::iterator_value& rhs): m_pMemory(rhs.m_pMemory), m_pNode(rhs.m_pNode)
+    inline Node::Node(const detail::iterator_value& rhs): m_isValid(rhs.m_isValid), m_pMemory(rhs.m_pMemory), m_pNode(rhs.m_pNode)
     {
     }
 
-	inline Node::Node(const Node& rhs): m_pMemory(rhs.m_pMemory), m_pNode(rhs.m_pNode)
+	inline Node::Node(const Node& rhs): m_isValid(rhs.m_isValid), m_pMemory(rhs.m_pMemory), m_pNode(rhs.m_pNode)
 	{
 	}
 	
-	inline Node::Node(detail::node& node, detail::shared_memory_holder pMemory): m_pMemory(pMemory), m_pNode(&node)
+    inline Node::Node(Zombie): m_isValid(false), m_pNode(NULL)
+    {
+    }
+
+    inline Node::Node(detail::node& node, detail::shared_memory_holder pMemory): m_isValid(true), m_pMemory(pMemory), m_pNode(&node)
 	{
 	}
 
@@ -48,6 +52,8 @@ namespace YAML
 
 	inline void Node::EnsureNodeExists() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		if(!m_pNode) {
 			m_pMemory.reset(new detail::memory_holder);
 			m_pNode = &m_pMemory->create_node();
@@ -57,11 +63,15 @@ namespace YAML
 	
 	inline bool Node::IsDefined() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? m_pNode->is_defined() : true;
 	}
 
 	inline NodeType::value Node::Type() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? m_pNode->type() : NodeType::Null;
 	}
 	
@@ -128,27 +138,37 @@ namespace YAML
 	template<typename T>
 	inline const T Node::as() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
         return as_if<T, void>(*this)();
 	}
 
     template<typename T, typename S>
     inline const T Node::as(const S& fallback) const
     {
+        if(!m_isValid)
+            throw InvalidNode();
         return as_if<T, S>(*this)(fallback);
     }
     
 	inline const std::string& Node::Scalar() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? m_pNode->scalar() : detail::node_data::empty_scalar;
 	}
 
 	inline const std::string& Node::Tag() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? m_pNode->tag() : detail::node_data::empty_scalar;
 	}
 
 	inline void Node::SetTag(const std::string& tag)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		m_pNode->set_tag(tag);
 	}
@@ -156,6 +176,8 @@ namespace YAML
 	// assignment
 	inline bool Node::is(const Node& rhs) const
 	{
+        if(!m_isValid || !rhs.m_isValid)
+            throw InvalidNode();
         if(!m_pNode || !rhs.m_pNode)
             return false;
 		return m_pNode->is(*rhs.m_pNode);
@@ -164,12 +186,16 @@ namespace YAML
 	template<typename T>
 	inline Node& Node::operator=(const T& rhs)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		Assign(rhs);
 		return *this;
 	}
 	
     inline void Node::reset(const YAML::Node& rhs)
     {
+        if(!m_isValid || !rhs.m_isValid)
+            throw InvalidNode();
         m_pMemory = rhs.m_pMemory;
         m_pNode = rhs.m_pNode;
     }
@@ -177,30 +203,40 @@ namespace YAML
 	template<typename T>
 	inline void Node::Assign(const T& rhs)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		AssignData(convert<T>::encode(rhs));
 	}
 
 	template<>
 	inline void Node::Assign(const std::string& rhs)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		m_pNode->set_scalar(rhs);
 	}
 
 	inline void Node::Assign(const char *rhs)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		m_pNode->set_scalar(rhs);
 	}
 
 	inline void Node::Assign(char *rhs)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		m_pNode->set_scalar(rhs);
 	}
 	
 	inline Node& Node::operator=(const Node& rhs)
 	{
+        if(!m_isValid || !rhs.m_isValid)
+            throw InvalidNode();
 		if(is(rhs))
 			return *this;
 		AssignNode(rhs);
@@ -209,6 +245,8 @@ namespace YAML
 
 	inline void Node::AssignData(const Node& rhs)
 	{
+        if(!m_isValid || !rhs.m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		rhs.EnsureNodeExists();
 		
@@ -218,6 +256,8 @@ namespace YAML
 
 	inline void Node::AssignNode(const Node& rhs)
 	{
+        if(!m_isValid || !rhs.m_isValid)
+            throw InvalidNode();
 		rhs.EnsureNodeExists();
 
 		if(!m_pNode) {
@@ -234,26 +274,36 @@ namespace YAML
 	// size/iterator
 	inline std::size_t Node::size() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? m_pNode->size() : 0;
 	}
 
 	inline const_iterator Node::begin() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? const_iterator(m_pNode->begin(), m_pMemory) : const_iterator();
 	}
 	
 	inline iterator Node::begin()
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? iterator(m_pNode->begin(), m_pMemory) : iterator();
 	}
 
 	inline const_iterator Node::end() const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? const_iterator(m_pNode->end(), m_pMemory) : const_iterator();
 	}
 
 	inline iterator Node::end()
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		return m_pNode ? iterator(m_pNode->end(), m_pMemory) : iterator();
 	}
 	
@@ -261,11 +311,15 @@ namespace YAML
 	template<typename T>
 	inline void Node::push_back(const T& rhs)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		push_back(Node(rhs));
 	}
 	
 	inline void Node::push_back(const Node& rhs)
 	{
+        if(!m_isValid || !rhs.m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		rhs.EnsureNodeExists();
 		
@@ -322,6 +376,8 @@ namespace YAML
 	template<typename Key>
 	inline const Node Node::operator[](const Key& key) const
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		detail::node& value = static_cast<const detail::node&>(*m_pNode).get(detail::to_value(key), m_pMemory);
 		return Node(value, m_pMemory);
@@ -330,6 +386,8 @@ namespace YAML
 	template<typename Key>
 	inline Node Node::operator[](const Key& key)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		detail::node& value = m_pNode->get(detail::to_value(key), m_pMemory);
 		return Node(value, m_pMemory);
@@ -338,12 +396,16 @@ namespace YAML
 	template<typename Key>
 	inline bool Node::remove(const Key& key)
 	{
+        if(!m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		return m_pNode->remove(detail::to_value(key), m_pMemory);
 	}
 	
 	inline const Node Node::operator[](const Node& key) const
 	{
+        if(!m_isValid || !key.m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		key.EnsureNodeExists();
 		detail::node& value = static_cast<const detail::node&>(*m_pNode).get(*key.m_pNode, m_pMemory);
@@ -352,6 +414,8 @@ namespace YAML
 	
 	inline Node Node::operator[](const Node& key)
 	{
+        if(!m_isValid || !key.m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		key.EnsureNodeExists();
 		detail::node& value = m_pNode->get(*key.m_pNode, m_pMemory);
@@ -360,6 +424,8 @@ namespace YAML
 	
 	inline bool Node::remove(const Node& key)
 	{
+        if(!m_isValid || !key.m_isValid)
+            throw InvalidNode();
 		EnsureNodeExists();
 		key.EnsureNodeExists();
 		return m_pNode->remove(*key.m_pNode, m_pMemory);
@@ -369,6 +435,8 @@ namespace YAML
     template<typename Key, typename Value>
     inline void Node::force_insert(const Key& key, const Value& value)
     {
+        if(!m_isValid)
+            throw InvalidNode();
         EnsureNodeExists();
 		m_pNode->force_insert(detail::to_value(key), detail::to_value(value), m_pMemory);
     }

include/yaml-cpp/node/iterator.h

@@ -19,8 +19,8 @@ namespace YAML
 	namespace detail {
 		struct iterator_value: public Node, std::pair<Node, Node> {
 			iterator_value() {}
-			explicit iterator_value(const Node& rhs): Node(rhs) {}
+			explicit iterator_value(const Node& rhs): Node(rhs), std::pair<Node, Node>(Node(Node::ZombieNode), Node(Node::ZombieNode)) {}
-			explicit iterator_value(const Node& key, const Node& value): std::pair<Node, Node>(key, value) {}
+			explicit iterator_value(const Node& key, const Node& value): Node(Node::ZombieNode), std::pair<Node, Node>(key, value) {}
 		};
 	}
 }

include/yaml-cpp/node/node.h

@@ -20,6 +20,7 @@ namespace YAML
 	public:
 		friend class NodeBuilder;
 		friend class NodeEvents;
+        friend struct detail::iterator_value;
 		friend class detail::node_data;
 		template<typename> friend class detail::iterator_base;
         template<typename T, typename S> friend struct as_if;
@@ -85,6 +86,8 @@ namespace YAML
         void force_insert(const Key& key, const Value& value);
 
 	private:
+        enum Zombie { ZombieNode };
+        explicit Node(Zombie);
 		explicit Node(detail::node& node, detail::shared_memory_holder pMemory);
 		
 		void EnsureNodeExists() const;
@@ -97,6 +100,7 @@ namespace YAML
 		void AssignNode(const Node& rhs);
 		
 	private:
+        bool m_isValid;
 		mutable detail::shared_memory_holder m_pMemory;
 		mutable detail::node *m_pNode;
 	};

test/new-api/nodetests.cpp

@@ -8,15 +8,33 @@ namespace {
 		TEST(): ok(false) {}
 		TEST(bool ok_): ok(ok_) {}
 		TEST(const char *error_): ok(false), error(error_) {}
+        TEST(const std::string& error_): ok(false), error(error_) {}
 		
 		bool ok;
 		std::string error;
 	};
 }
 
-#define YAML_ASSERT(cond) do { if(!(cond)) return "  Assert failed: " #cond; } while(false)
+#define YAML_ASSERT(cond)\
+    do {\
+        if(!(cond))\
+            return "  Assert failed: " #cond;\
+    } while(false)
 
-#define YAML_ASSERT_THROWS(cond, exc) do { try { (cond); return "  Expression did not throw: " #cond; } catch(const exc&) {} catch(...) { return "  Expression threw something other than " #exc ": " #cond; } } while(false)
+#define YAML_ASSERT_THROWS(cond, exc)\
+    do {\
+        try {\
+            (cond);\
+            return "  Expression did not throw: " #cond;\
+        } catch(const exc&) {\
+        } catch(const std::runtime_error& e) {\
+            std::stringstream stream;\
+            stream << "  Expression threw runtime error ther than " #exc ":\n    " #cond "\n    " << e.what();\
+            return stream.str();\
+        } catch(...) {\
+            return "  Expression threw unknown exception, other than " #exc ":\n    " #cond;\
+        }\
+    } while(false)
 
 namespace Test
 {
@@ -319,7 +337,7 @@ namespace Test
             YAML::Node node = YAML::Load("[1.5, 1, .nan, .inf, -.inf, 0x15, 015]");
             YAML_ASSERT(node[0].as<float>() == 1.5f);
             YAML_ASSERT(node[0].as<double>() == 1.5);
-            YAML_ASSERT_THROWS(node[0].as<int>(), std::runtime_error);
+            YAML_ASSERT_THROWS(node[0].as<int>(), YAML::TypedBadConversion<int>);
             YAML_ASSERT(node[1].as<int>() == 1);
             YAML_ASSERT(node[1].as<float>() == 1.0f);
             YAML_ASSERT(node[2].as<float>() != node[2].as<float>());
@@ -469,6 +487,17 @@ namespace Test
             YAML_ASSERT(other == node);
             return true;
         }
+
+        TEST DereferenceIteratorError()
+        {
+            YAML::Node node = YAML::Load("[{a: b}, 1, 2]");
+            YAML_ASSERT_THROWS(node.begin()->first.as<int>(), YAML::InvalidNode);
+            YAML_ASSERT((*node.begin()).IsMap() == true);
+            YAML_ASSERT(node.begin()->IsMap() == true);
+            YAML_ASSERT_THROWS((*node.begin()->begin()).IsDefined(), YAML::InvalidNode);
+            YAML_ASSERT_THROWS(node.begin()->begin()->IsDefined(), YAML::InvalidNode);
+            return true;
+        }
     }
 	
 	void RunNodeTest(TEST (*test)(), const std::string& name, int& passed, int& total) {
@@ -530,6 +559,7 @@ namespace Test
 		RunNodeTest(&Node::CloneAlias, "clone alias", passed, total);
         RunNodeTest(&Node::ForceInsertIntoMap, "force insert into map", passed, total);
         RunNodeTest(&Node::ResetNode, "reset node", passed, total);
+        RunNodeTest(&Node::DereferenceIteratorError, "dereference iterator error", passed, total);
 
 		std::cout << "Node tests: " << passed << "/" << total << " passed\n";
 		return passed == total;