Fixed #637.

<<< Breaking change >>>
In the functions unpack() and parse(),
Old behavior: If any parse error is happend, offset is NOT updated.
New behavior: If any parse error is happend, offset is updated to the
position the error happened.

It helps MessagePack format error analysis.

If you want to old behavior, copy the original value of offset and then call unpack()
and/or parse().

include/msgpack/v1/unpack.hpp

@@ -1393,12 +1393,11 @@ inline msgpack::object_handle unpack(
     parse_return ret = detail::unpack_imp(
         data, len, noff, *z, obj, referenced, f, user_data, limit);
 
+    off = noff;
     switch(ret) {
     case PARSE_SUCCESS:
-        off = noff;
         return msgpack::object_handle(obj, msgpack::move(z));
     case PARSE_EXTRA_BYTES:
-        off = noff;
         return msgpack::object_handle(obj, msgpack::move(z));
     case PARSE_CONTINUE:
         throw msgpack::insufficient_bytes("insufficient bytes");
@@ -1450,14 +1449,13 @@ inline void unpack(
     parse_return ret = detail::unpack_imp(
         data, len, noff, *z, obj, referenced, f, user_data, limit);
 
+    off = noff;
     switch(ret) {
     case PARSE_SUCCESS:
-        off = noff;
         result.set(obj);
         result.zone() = msgpack::move(z);
         return;
     case PARSE_EXTRA_BYTES:
-        off = noff;
         result.set(obj);
         result.zone() = msgpack::move(z);
         return;
@@ -1513,12 +1511,11 @@ inline msgpack::object unpack(
     parse_return ret = detail::unpack_imp(
         data, len, noff, z, obj, referenced, f, user_data, limit);
 
+    off = noff;
     switch(ret) {
     case PARSE_SUCCESS:
-        off = noff;
         return obj;
     case PARSE_EXTRA_BYTES:
-        off = noff;
         return obj;
     case PARSE_CONTINUE:
         throw msgpack::insufficient_bytes("insufficient bytes");

include/msgpack/v2/parse.hpp

@@ -58,7 +58,6 @@ private:
         std::size_t& off) {
         typename value<T>::type size;
         load<T>(size, load_pos);
-        ++m_current;
         if (size == 0) {
             if (!sv(size)) {
                 off = m_current - m_start;
@@ -68,7 +67,7 @@ private:
                 off = m_current - m_start;
                 return PARSE_STOP_VISITOR;
             }
-            parse_return ret = m_stack.consume(holder());
+            parse_return ret = m_stack.consume(holder(), m_current);
             if (ret != PARSE_CONTINUE) {
                 off = m_current - m_start;
                 return ret;
@@ -85,17 +84,18 @@ private:
                 return ret;
             }
         }
+        ++m_current;
         m_cs = MSGPACK_CS_HEADER;
         return PARSE_CONTINUE;
     }
 
     parse_return after_visit_proc(bool visit_result, std::size_t& off) {
-        ++m_current;
         if (!visit_result) {
             off = m_current - m_start;
             return PARSE_STOP_VISITOR;
         }
-        parse_return ret = m_stack.consume(holder());
+        parse_return ret = m_stack.consume(holder(), m_current);
+        ++m_current;
         if (ret != PARSE_CONTINUE) {
             off = m_current - m_start;
         }
@@ -161,15 +161,21 @@ private:
             assert(0);
             return PARSE_STOP_VISITOR;
         }
-        parse_return consume(VisitorHolder& visitor_holder) {
+        parse_return consume(VisitorHolder& visitor_holder, char const*& current) {
             while (!m_stack.empty()) {
                 stack_elem& e = m_stack.back();
                 switch (e.m_type) {
                 case MSGPACK_CT_ARRAY_ITEM:
-                    if (!visitor_holder.visitor().end_array_item()) return PARSE_STOP_VISITOR;
+                    if (!visitor_holder.visitor().end_array_item()) {
+                        --current;
+                        return PARSE_STOP_VISITOR;
+                    }
                     if (--e.m_rest == 0)  {
                         m_stack.pop_back();
-                        if (!visitor_holder.visitor().end_array()) return PARSE_STOP_VISITOR;
+                        if (!visitor_holder.visitor().end_array()) {
+                            --current;
+                            return PARSE_STOP_VISITOR;
+                        }
                     }
                     else {
                         if (!visitor_holder.visitor().start_array_item()) return PARSE_STOP_VISITOR;
@@ -177,15 +183,24 @@ private:
                     }
                     break;
                 case MSGPACK_CT_MAP_KEY:
-                    if (!visitor_holder.visitor().end_map_key()) return PARSE_STOP_VISITOR;
+                    if (!visitor_holder.visitor().end_map_key()) {
+                        --current;
+                        return PARSE_STOP_VISITOR;
+                    }
                     if (!visitor_holder.visitor().start_map_value()) return PARSE_STOP_VISITOR;
                     e.m_type = MSGPACK_CT_MAP_VALUE;
                     return PARSE_CONTINUE;
                 case MSGPACK_CT_MAP_VALUE:
-                    if (!visitor_holder.visitor().end_map_value()) return PARSE_STOP_VISITOR;
+                    if (!visitor_holder.visitor().end_map_value()) {
+                        --current;
+                        return PARSE_STOP_VISITOR;
+                    }
                     if (--e.m_rest == 0) {
                         m_stack.pop_back();
-                        if (!visitor_holder.visitor().end_map()) return PARSE_STOP_VISITOR;
+                        if (!visitor_holder.visitor().end_map()) {
+                            --current;
+                            return PARSE_STOP_VISITOR;
+                        }
                     }
                     else {
                         e.m_type = MSGPACK_CT_MAP_KEY;
@@ -1032,13 +1047,12 @@ parse_imp(const char* data, size_t len, size_t& off, Visitor& v) {
     }
     detail::parse_helper<Visitor> h(v);
     parse_return ret = h.execute(data, len, noff);
+    off = noff;
     switch (ret) {
     case PARSE_CONTINUE:
-        off = noff;
         v.insufficient_bytes(noff - 1, noff);
         return ret;
     case PARSE_SUCCESS:
-        off = noff;
         if(noff < len) {
             return PARSE_EXTRA_BYTES;
         }

include/msgpack/v2/unpack.hpp

@@ -152,16 +152,13 @@ inline msgpack::object_handle unpack(
     msgpack::object obj;
     msgpack::unique_ptr<msgpack::zone> z(new msgpack::zone);
     referenced = false;
-    std::size_t noff = off;
     parse_return ret = detail::unpack_imp(
-        data, len, noff, *z, obj, referenced, f, user_data, limit);
+        data, len, off, *z, obj, referenced, f, user_data, limit);
 
     switch(ret) {
     case PARSE_SUCCESS:
-        off = noff;
         return msgpack::object_handle(obj, msgpack::move(z));
     case PARSE_EXTRA_BYTES:
-        off = noff;
         return msgpack::object_handle(obj, msgpack::move(z));
     default:
         break;
@@ -206,18 +203,15 @@ inline void unpack(
     msgpack::object obj;
     msgpack::unique_ptr<msgpack::zone> z(new msgpack::zone);
     referenced = false;
-    std::size_t noff = off;
     parse_return ret = detail::unpack_imp(
-        data, len, noff, *z, obj, referenced, f, user_data, limit);
+        data, len, off, *z, obj, referenced, f, user_data, limit);
 
     switch(ret) {
     case PARSE_SUCCESS:
-        off = noff;
         result.set(obj);
         result.zone() = msgpack::move(z);
         return;
     case PARSE_EXTRA_BYTES:
-        off = noff;
         result.set(obj);
         result.zone() = msgpack::move(z);
         return;
@@ -265,17 +259,14 @@ inline msgpack::object unpack(
     unpack_limit const& limit)
 {
     msgpack::object obj;
-    std::size_t noff = off;
     referenced = false;
     parse_return ret = detail::unpack_imp(
-        data, len, noff, z, obj, referenced, f, user_data, limit);
+        data, len, off, z, obj, referenced, f, user_data, limit);
 
     switch(ret) {
     case PARSE_SUCCESS:
-        off = noff;
         return obj;
     case PARSE_EXTRA_BYTES:
-        off = noff;
         return obj;
     default:
         break;

src/unpack.c

@@ -649,19 +649,19 @@ msgpack_unpack_next(msgpack_unpacked* result,
         ctx.user.referenced = false;
 
         e = template_execute(&ctx, data, len, &noff);
+
+        if(off != NULL) { *off = noff; }
+
         if(e < 0) {
             msgpack_zone_free(result->zone);
             result->zone = NULL;
             return e;
         }
 
-
         if(e == 0) {
             return MSGPACK_UNPACK_CONTINUE;
         }
 
-        if(off != NULL) { *off = noff; }
-
         result->data = template_data(&ctx);
 
         return MSGPACK_UNPACK_SUCCESS;

test/pack_unpack.cpp

@@ -370,7 +370,7 @@ TEST(unpack, insufficient_bytes_ref)
     }
     catch (msgpack::insufficient_bytes const&) {
         EXPECT_TRUE(true);
-        EXPECT_EQ(off, 0u);
+        EXPECT_EQ(1u, off);
     }
 }
 
@@ -387,7 +387,7 @@ TEST(unpack, insufficient_bytes_object_handle)
     }
     catch (msgpack::insufficient_bytes const&) {
         EXPECT_TRUE(true);
-        EXPECT_EQ(off, 0u);
+        EXPECT_EQ(1u, off);
     }
 }
 
@@ -405,7 +405,7 @@ TEST(unpack, insufficient_bytes_zone)
     }
     catch (msgpack::insufficient_bytes const&) {
         EXPECT_TRUE(true);
-        EXPECT_EQ(off, 0u);
+        EXPECT_EQ(1u, off);
     }
 }
 

test/pack_unpack_c.cpp

@@ -83,7 +83,7 @@ TEST(pack, insufficient)
 
     success = msgpack_unpack_next(&msg, sbuf->data, 1, &offset);
     EXPECT_EQ(MSGPACK_UNPACK_CONTINUE, success);
-    EXPECT_EQ(0u, offset);
+    EXPECT_EQ(1u, offset);
 
     msgpack_unpacked_destroy(&msg);
 

test/visitor.cpp

@@ -115,6 +115,7 @@ TEST(visitor, parse_error)
     bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
     EXPECT_FALSE(ret);
     EXPECT_TRUE(called);
+    EXPECT_EQ(2u, off);
 }
 
 struct insuf_bytes_check_visitor : msgpack::v2::null_visitor {
@@ -136,6 +137,304 @@ TEST(visitor, insuf_bytes)
     bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
     EXPECT_FALSE(ret);
     EXPECT_TRUE(called);
+    EXPECT_EQ(3u, off);
 }
 
+struct return_false_array_val_visitor : msgpack::v2::null_visitor {
+    return_false_array_val_visitor(std::size_t& times):m_times(times) {}
+    bool visit_positive_integer(uint64_t) {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_array_val)
+{
+    std::size_t times = 0;
+    return_false_array_val_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x93u), 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(2u, off);
+}
+
+struct return_false_start_array_item_visitor : msgpack::v2::null_visitor {
+    return_false_start_array_item_visitor(std::size_t& times):m_times(times) {}
+    bool start_array_item() {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_start_array_item)
+{
+    std::size_t times = 0;
+    return_false_start_array_item_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x93u), 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(2u, off);
+}
+
+struct return_false_end_array_item_visitor : msgpack::v2::null_visitor {
+    return_false_end_array_item_visitor(std::size_t& times):m_times(times) {}
+    bool end_array_item() {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_end_array_item)
+{
+    std::size_t times = 0;
+    return_false_end_array_item_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x93u), 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(2u, off);
+}
+
+struct return_false_start_array_visitor : msgpack::v2::null_visitor {
+    bool start_array(uint32_t) {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_start_array)
+{
+    return_false_start_array_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x93u), 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(0u, off);
+}
+
+struct return_false_start_array0_visitor : msgpack::v2::null_visitor {
+    bool start_array(uint32_t) {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_start_array0)
+{
+    return_false_start_array0_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x90u) };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(0u, off);
+}
+
+struct return_false_end_array_visitor : msgpack::v2::null_visitor {
+    bool end_array() {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_end_array)
+{
+    return_false_end_array_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x93u), 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(3u, off);
+}
+
+struct return_false_end_array0_visitor : msgpack::v2::null_visitor {
+    bool end_array() {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_end_array0)
+{
+    return_false_end_array0_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x90u) };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(0u, off);
+}
+
+struct return_false_map_val_visitor : msgpack::v2::null_visitor {
+    return_false_map_val_visitor(std::size_t& times):m_times(times) {}
+    bool visit_positive_integer(uint64_t) {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_map_val)
+{
+    std::size_t times = 0;
+    return_false_map_val_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(2u, off);
+}
+
+struct return_false_start_map_key_visitor : msgpack::v2::null_visitor {
+    return_false_start_map_key_visitor(std::size_t& times):m_times(times) {}
+    bool start_map_key() {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_start_map_key)
+{
+    std::size_t times = 0;
+    return_false_start_map_key_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(3u, off);
+}
+
+struct return_false_end_map_key_visitor : msgpack::v2::null_visitor {
+    return_false_end_map_key_visitor(std::size_t& times):m_times(times) {}
+    bool end_map_key() {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_end_map_key)
+{
+    std::size_t times = 0;
+    return_false_end_map_key_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(3u, off);
+}
+
+struct return_false_start_map_value_visitor : msgpack::v2::null_visitor {
+    return_false_start_map_value_visitor(std::size_t& times):m_times(times) {}
+    bool start_map_value() {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_start_map_value)
+{
+    std::size_t times = 0;
+    return_false_start_map_value_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(4u, off);
+}
+
+struct return_false_end_map_value_visitor : msgpack::v2::null_visitor {
+    return_false_end_map_value_visitor(std::size_t& times):m_times(times) {}
+    bool end_map_value() {
+        if (++m_times == 2) return false;
+        return true;
+    }
+    std::size_t& m_times;
+};
+
+TEST(visitor, return_false_end_map_value)
+{
+    std::size_t times = 0;
+    return_false_end_map_value_visitor v(times);
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(2u, times);
+    EXPECT_EQ(4u, off);
+}
+
+struct return_false_start_map_visitor : msgpack::v2::null_visitor {
+    bool start_map(uint32_t) {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_start_map)
+{
+    return_false_start_map_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(0u, off);
+}
+
+struct return_false_start_map0_visitor : msgpack::v2::null_visitor {
+    bool start_map(uint32_t) {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_start_map0)
+{
+    return_false_start_map0_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x80u) };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(0u, off);
+}
+
+struct return_false_end_map_visitor : msgpack::v2::null_visitor {
+    bool end_map() {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_end_map)
+{
+    return_false_end_map_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x83u), 0x01u, 0x02u, 0x03u, 0x01u, 0x02u, 0x03u };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(6u, off);
+}
+
+struct return_false_end_map0_visitor : msgpack::v2::null_visitor {
+    bool end_map() {
+        return false;
+    }
+};
+
+TEST(visitor, return_false_end_map0)
+{
+    return_false_end_map0_visitor v;
+    std::size_t off = 0;
+    char const data[] = { static_cast<char>(0x80u) };
+    bool ret = msgpack::v2::parse(data, sizeof(data), off, v);
+    EXPECT_FALSE(ret);
+    EXPECT_EQ(0u, off);
+}
+
+
 #endif // MSGPACK_DEFAULT_API_VERSION >= 1