Parcourir la source

Add extra tests for rudundant fields deserialization skip

- Make implementation for length-delimited fields skipping bit more optimized
- Add and update tests
Alexey Edelev il y a 5 ans
Parent
commit
2d066967d4

+ 13 - 12
src/protobuf/qprotobufobject_p.cpp

@@ -144,35 +144,36 @@ void ProtobufObjectPrivate::deserializeUserType(const QMetaProperty &metaPropert
     deserializer(it, out);
 }
 
-void ProtobufObjectPrivate::skipVarint(SelfcheckIterator &it) {
-    while (true) {
-        if (((*it) & 0x80) == 0) {
-            break;
-        }
+void ProtobufObjectPrivate::skipVarint(SelfcheckIterator &it)
+{
+    while ((*it) & 0x80) {
         ++it;
     }
     ++it;
 }
 
-void ProtobufObjectPrivate::skipLengthLimited(SelfcheckIterator &it) {
-    // return value intentionaly ignored
-    deserializeLengthDelimited(it);
+void ProtobufObjectPrivate::skipLengthDelimited(SelfcheckIterator &it)
+{
+    //Get length of lenght-delimited field
+    unsigned int length = deserializeBasic<uint32>(it).toUInt();
+    it += length;
 }
 
-long ProtobufObjectPrivate::skipSerializedFieldBytes(SelfcheckIterator &it, WireTypes type) {
+int 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);
+        it += sizeof(decltype(fixed32::_t));
         break;
     case WireTypes::Fixed64:
-        it += sizeof(int64_t);
+        it += sizeof(decltype(fixed64::_t));
         break;
     case WireTypes::LengthDelimited:
-        skipLengthLimited(it);
+        skipLengthDelimited(it);
         break;
     case WireTypes::UnknownWireType:
     default:

+ 4 - 3
src/protobuf/qprotobufobject_p.h

@@ -685,8 +685,8 @@ public:
     }
 
     static void skipVarint(SelfcheckIterator &it);
-    static void skipLengthLimited(SelfcheckIterator &it);
-    static long skipSerializedFieldBytes(SelfcheckIterator &it, WireTypes type);
+    static void skipLengthDelimited(SelfcheckIterator &it);
+    static int skipSerializedFieldBytes(SelfcheckIterator &it, WireTypes type);
 
     template<typename T>
     static void deserialize(QObject *object, const QByteArray &array) {
@@ -706,7 +706,8 @@ public:
             auto propertyNumberIt = T::propertyOrdering.find(fieldNumber);
             if (propertyNumberIt == std::end(T::propertyOrdering)) {
                 auto bytesCount = skipSerializedFieldBytes(it, wireType);
-                qProtoWarning() << "Message received contains unexpected field. WireType:" << wireType << ", field number: " << fieldNumber << "Skipped:" << bytesCount << "B";
+                qProtoWarning() << "Message received contains unexpected/optional field. WireType:" << wireType
+                                << ", field number: " << fieldNumber << "Skipped:" << (bytesCount + 1) << "bytes";
                 continue;
             }
 

+ 16 - 1
tests/test_protobuf/deserializationtest.cpp

@@ -861,7 +861,22 @@ TEST_F(DeserializationTest, SimpleUInt64ComplexMapCorruptedDeserializeTest)
 TEST_F(DeserializationTest, RedundantFieldIsIgnoredAtDeserializationTest)
 {
     ComplexMessage test;
-    // the byte-array contains redundant bytes deserialazable to an additional field
+    //3206717765727479 length delimited field number 6
+    ASSERT_NO_THROW(test.deserialize(QByteArray::fromHex("120832067177657274793206717765727479")));
+    EXPECT_EQ(test.testFieldInt(), 0);
+    EXPECT_STREQ(test.testComplexField().testFieldString().toStdString().c_str(), "qwerty");
+
+    //3dcdcccc3d fixed32 field number 7
+    ASSERT_NO_THROW(test.deserialize(QByteArray::fromHex("120832067177657274793dcdcccc3d")));
+    EXPECT_EQ(test.testFieldInt(), 0);
+    EXPECT_STREQ(test.testComplexField().testFieldString().toStdString().c_str(), "qwerty");
+
+    //419a9999999999b93f fixed64 field number 8
+    ASSERT_NO_THROW(test.deserialize(QByteArray::fromHex("12083206717765727479419a9999999999b93f")));
+    EXPECT_EQ(test.testFieldInt(), 0);
+    EXPECT_STREQ(test.testComplexField().testFieldString().toStdString().c_str(), "qwerty");
+
+    //60d3ffffffffffffffff01 varint field number 12
     ASSERT_NO_THROW(test.deserialize(QByteArray::fromHex("60d3ffffffffffffffff0112083206717765727479")));
     EXPECT_EQ(test.testFieldInt(), 0);
     EXPECT_STREQ(test.testComplexField().testFieldString().toStdString().c_str(), "qwerty");