Redundant field should not interrupt message parsing ( #127 )

src/protobuf/qprotobufobject_p.cpp

@@ -143,3 +143,41 @@ void ProtobufObjectPrivate::deserializeUserType(const QMetaProperty &metaPropert
     auto deserializer = serializers.at(userType).deserializer;//Throws exception if not found
     deserializer(it, out);
 }
+
+void ProtobufObjectPrivate::skipVarint(SelfcheckIterator &it) {
+    while (true) {
+        if (((*it) & 0x80) == 0) {
+            break;
+        }
+        ++it;
+    }
+    ++it;
+}
+
+void ProtobufObjectPrivate::skipLengthLimited(SelfcheckIterator &it) {
+    // return value intentionaly ignored
+    deserializeLengthDelimited(it);
+}
+
+long ProtobufObjectPrivate::skipSerializedFieldBytes(SelfcheckIterator &it, WireTypes type) {
+    const auto initialIt = QByteArray::const_iterator(it);
+    switch (type) {
+    case WireTypes::Varint:
+        skipVarint(it);
+        break;
+    case WireTypes::Fixed32:
+        it += sizeof(int32_t);
+        break;
+    case WireTypes::Fixed64:
+        it += sizeof(int64_t);
+        break;
+    case WireTypes::LengthDelimited:
+        skipLengthLimited(it);
+        break;
+    case WireTypes::UnknownWireType:
+    default:
+        throw std::invalid_argument("Cannot skip due to undefined length of the redundant field.");
+    }
+
+    return std::distance(initialIt, QByteArray::const_iterator(it));
+}

src/protobuf/qprotobufobject_p.h

@@ -684,6 +684,10 @@ public:
         return result;
     }
 
+    static void skipVarint(SelfcheckIterator &it);
+    static void skipLengthLimited(SelfcheckIterator &it);
+    static long skipSerializedFieldBytes(SelfcheckIterator &it, WireTypes type);
+
     template<typename T>
     static void deserialize(QObject *object, const QByteArray &array) {
         qProtoDebug() << T::staticMetaObject.className() << "deserialize";
@@ -699,16 +703,11 @@ public:
                                       "Seems stream is broken");
             }
 
-            //FIXME: This check is incorrect in protobuf syntax 3 perspective.
-            //It should be possible to process stream even if it contains field
-            //is not a part of the structure.
             auto propertyNumberIt = T::propertyOrdering.find(fieldNumber);
             if (propertyNumberIt == std::end(T::propertyOrdering)) {
-                qProtoCritical() << "Message received contains invalid field number. "
-                               "Trying next, but seems stream is broken";
-                throw std::invalid_argument("Message received contains invalid field number. "
-                                            "Seems stream is broken.\n"
-                                            "QtProtobuf doesn't support extra data for optional fields.");
+                auto bytesCount = skipSerializedFieldBytes(it, wireType);
+                qProtoWarning() << "Message received contains unexpected field. WireType:" << wireType << ", field number: " << fieldNumber << "Skipped:" << bytesCount << "B";
+                continue;
             }
 
             int propertyIndex = propertyNumberIt->second;

src/protobuf/qtprotobuflogging.cpp

@@ -25,4 +25,4 @@
 
 #include "qtprotobuflogging.h"
 
-Q_LOGGING_CATEGORY(qtprotobuflog, "qtprotobuflog", QtCriticalMsg)
+Q_LOGGING_CATEGORY(qtprotobuflog, "qtprotobuflog", QtWarningMsg)

tests/test_protobuf/deserializationtest.cpp

@@ -858,29 +858,30 @@ TEST_F(DeserializationTest, SimpleUInt64ComplexMapCorruptedDeserializeTest)
     ASSERT_TRUE(test.mapField().isEmpty());
 }
 
-TEST_F(DeserializationTest, DISABLED_InvalidFieldIndexDeserializeTest)
+TEST_F(DeserializationTest, RedundantFieldIsIgnoredAtDeserializationTest)
 {
-    ComplexMessage test{0, QString{}};
-    test.deserialize(QByteArray::fromHex("60d3ffffffffffffffff0112083206717765727479"));
-    ASSERT_EQ(test.testFieldInt(), 0);
-    ASSERT_STREQ(test.testComplexField().testFieldString().toStdString().c_str(), "qwerty");
+    ComplexMessage test;
+    // the byte-array contains redundant bytes deserialazable to an additional field
+    ASSERT_NO_THROW(test.deserialize(QByteArray::fromHex("60d3ffffffffffffffff0112083206717765727479")));
+    EXPECT_EQ(test.testFieldInt(), 0);
+    EXPECT_STREQ(test.testComplexField().testFieldString().toStdString().c_str(), "qwerty");
 }
 
 TEST_F(DeserializationTest, FieldIndexRangeTest)
 {
-    FieldIndexTest1Message msg1({0});
+    FieldIndexTest1Message msg1(0);
     msg1.deserialize(QByteArray::fromHex("f80102"));
     ASSERT_EQ(msg1.testField(), 1);
 
-    FieldIndexTest2Message msg2({0});
+    FieldIndexTest2Message msg2(0);
     msg2.deserialize(QByteArray::fromHex("f8ff0302"));
     ASSERT_EQ(msg2.testField(), 1);
 
-    FieldIndexTest3Message msg3({0});
+    FieldIndexTest3Message msg3(0);
     msg3.deserialize(QByteArray::fromHex("f8ffff0702"));
     ASSERT_EQ(msg3.testField(), 1);
 
-    FieldIndexTest4Message msg4({0});
+    FieldIndexTest4Message msg4(0);
     msg4.deserialize(QByteArray::fromHex("f8ffffff0f02"));
     ASSERT_EQ(msg4.testField(), 1);
 }