THRIFT-4024, THRIFT-4783: throw when skipping invalid type (#1742)

* THRIFT-4024: make c_glib throw on unsupported type when skipping
* THRIFT-4783: throw on invalid skip (py)
* THRIFT-4024: make cpp throw on unsupported type when skipping
* THRIFT-4024: uniform skip behavior on unsupported type

1 files changed, 47 insertions, 25 deletions

diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
index 8296a8cad..6e6ae4d9a 100644
--- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
+++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
@@ -419,6 +419,13 @@ thrift_protocol_read_binary (ThriftProtocol *protocol, gpointer *buf,
                                                             len, error);
 }
 
+#define THRIFT_SKIP_RESULT_OR_RETURN(_RES, _CALL) \
+  { \
+    gint32 _x = (_CALL); \
+    if (_x < 0) { return _x; } \
+    (_RES) += _x; \
+  }
+
 gint32
 thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
 {
@@ -469,24 +476,24 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
         gchar *name;
         gint16 fid;
         ThriftType ftype;
-        result += thrift_protocol_read_struct_begin (protocol, &name, error);
-
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_struct_begin (protocol, &name, error))
         while (1)
         {
-          result += thrift_protocol_read_field_begin (protocol, &name, &ftype,
-                                                      &fid, error);
-          if (result < 0)
-          {
-            return result;  
-          }
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_read_field_begin (protocol, &name, &ftype,
+                                              &fid, error))
           if (ftype == T_STOP)
           {
             break;
           }
-          result += thrift_protocol_skip (protocol, ftype, error);
-          result += thrift_protocol_read_field_end (protocol, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, ftype, error))
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_read_field_end (protocol, error))
         }
-        result += thrift_protocol_read_struct_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_struct_end (protocol, error))
         return result;
       }
     case T_SET:
@@ -494,13 +501,16 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
         gint32 result = 0;
         ThriftType elem_type;
         guint32 i, size;
-        result += thrift_protocol_read_set_begin (protocol, &elem_type, &size,
-                                                  error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_set_begin (protocol, &elem_type, &size,
+                                          error))
         for (i = 0; i < size; i++)
         {
-          result += thrift_protocol_skip (protocol, elem_type, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, elem_type, error))
         }
-        result += thrift_protocol_read_set_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_set_end (protocol, error))
         return result;
       }
     case T_MAP:
@@ -509,14 +519,18 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
         ThriftType elem_type;
         ThriftType key_type;
         guint32 i, size;
-        result += thrift_protocol_read_map_begin (protocol, &key_type, &elem_type, &size,
-                                                  error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_map_begin (protocol, &key_type, &elem_type, &size,
+                                          error))
         for (i = 0; i < size; i++)
         {
-          result += thrift_protocol_skip (protocol, key_type, error);
-          result += thrift_protocol_skip (protocol, elem_type, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, key_type, error))
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, elem_type, error))
         }
-        result += thrift_protocol_read_map_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_map_end (protocol, error))
         return result;
       }
     case T_LIST:
@@ -524,18 +538,26 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
         gint32 result = 0;
         ThriftType elem_type;
         guint32 i, size;
-        result += thrift_protocol_read_list_begin (protocol, &elem_type, &size,
-                                                   error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_list_begin (protocol, &elem_type, &size,
+                                           error))
         for (i = 0; i < size; i++)
         {
-          result += thrift_protocol_skip (protocol, elem_type, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_skip (protocol, elem_type, error))
         }
-        result += thrift_protocol_read_list_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_list_end (protocol, error))
         return result;
       }
     default:
-      return 0;
+      break;
   }
+
+  g_set_error (error, THRIFT_PROTOCOL_ERROR,
+               THRIFT_PROTOCOL_ERROR_INVALID_DATA,
+               "unrecognized type");
+  return -1;
 }
 
 /* define the GError domain for Thrift protocols */