Jens-G commented on a change in pull request #2208:
URL: https://github.com/apache/thrift/pull/2208#discussion_r465614445
##########
File path: lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c
##########
@@ -557,6 +561,14 @@ thrift_binary_protocol_read_map_begin (ThriftProtocol
*protocol,
return -1;
}
+ if(FALSE == ttc->checkReadBytesAvailable(THRIFT_TRANSPORT(tp->transport),
Review comment:
`if( FALSE == ...)` - we can do better, can we?
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_buffered_transport.c
##########
@@ -379,6 +424,36 @@ thrift_buffered_transport_class_init
(ThriftBufferedTransportClass *cls)
PROP_THRIFT_BUFFERED_TRANSPORT_WRITE_BUFFER_SIZE,
param_spec);
+ param_spec = g_param_spec_object ("configuration",
+ "configuration (construct)",
+ "thrift configuration",
+ THRIFT_TYPE_CONFIGURATION,
+ G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY);
+ g_object_class_install_property (gobject_class,
+ PROP_THRIFT_BUFFERED_TRANSPORT_CONFIGURATION,
+ param_spec);
+
+ param_spec = g_param_spec_long ("remainingmessagesize",
+ "remainingmessagesize (construct)",
+ "Set the size of the remaining message",
+ 0, /* min */
+ G_MAXINT32, /* max */
+ 10485760, /* default by construct */
Review comment:
can we put that `10485760` into some constant and give it a meaningful
name?
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_transport.c
##########
@@ -129,17 +141,209 @@ thrift_transport_real_read_all (ThriftTransport
*transport, gpointer buf,
return have;
}
+gboolean
+thrift_transport_updateKnownMessageSize(ThriftTransport *transport, glong
size, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ ThriftTransportClass *ttc = THRIFT_TRANSPORT_GET_CLASS (transport);
+ glong consumed = tt->knowMessageSize_ - tt->remainingMessageSize_;
+ if(FALSE == ttc->resetConsumedMessageSize(transport, size, error))
+ {
+ boolean = FALSE;
+ }
+ if(FALSE == ttc->countConsumedMessageBytes(transport, consumed, error))
+ {
+ boolean = FALSE;
+ }
+ return boolean;
+}
+
+gboolean
+thrift_transport_checkReadBytesAvailable(ThriftTransport *transport, glong
numBytes, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(tt->remainingMessageSize_ < numBytes)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ boolean = FALSE;
+ }
+
+
+ return boolean;
+}
+
+gboolean
+thrift_transport_resetConsumedMessageSize(ThriftTransport *transport, glong
newSize, GError **error)
+{
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(newSize < 0)
+ {
+ if(tt->configuration != NULL)
+ {
+ tt->knowMessageSize_ = tt->configuration->maxMessageSize_;
Review comment:
minor: indent?
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_transport.c
##########
@@ -129,17 +141,209 @@ thrift_transport_real_read_all (ThriftTransport
*transport, gpointer buf,
return have;
}
+gboolean
+thrift_transport_updateKnownMessageSize(ThriftTransport *transport, glong
size, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ ThriftTransportClass *ttc = THRIFT_TRANSPORT_GET_CLASS (transport);
+ glong consumed = tt->knowMessageSize_ - tt->remainingMessageSize_;
+ if(FALSE == ttc->resetConsumedMessageSize(transport, size, error))
+ {
+ boolean = FALSE;
+ }
+ if(FALSE == ttc->countConsumedMessageBytes(transport, consumed, error))
+ {
+ boolean = FALSE;
+ }
+ return boolean;
+}
+
+gboolean
+thrift_transport_checkReadBytesAvailable(ThriftTransport *transport, glong
numBytes, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(tt->remainingMessageSize_ < numBytes)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ boolean = FALSE;
+ }
+
+
+ return boolean;
+}
+
+gboolean
+thrift_transport_resetConsumedMessageSize(ThriftTransport *transport, glong
newSize, GError **error)
+{
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(newSize < 0)
+ {
+ if(tt->configuration != NULL)
+ {
+ tt->knowMessageSize_ = tt->configuration->maxMessageSize_;
+ tt->remainingMessageSize_ = tt->configuration->maxMessageSize_;
+ }
+ else
+ {
+ tt->knowMessageSize_ = 10485760;
+ tt->remainingMessageSize_ = 10485760;
+ }
+ return TRUE;
+ }
+ /* update only: message size can shrink, but not grow */
+ if(newSize > tt->knowMessageSize_)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ return FALSE;
+ }
+
+ tt->knowMessageSize_ = newSize;
+ tt->remainingMessageSize_ = newSize;
+
+ return TRUE;
+}
+
+gboolean
+thrift_transport_countConsumedMessageBytes(ThriftTransport *transport, glong
numBytes, GError **error)
+{
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(tt->remainingMessageSize_ > numBytes)
+ {
+ tt->remainingMessageSize_ -= numBytes;
+ }
+ else
+ {
+ tt->remainingMessageSize_ = 0;
+ g_set_error(error,
Review comment:
indent?
##########
File path: lib/c_glib/src/thrift/c_glib/protocol/thrift_binary_protocol.c
##########
@@ -603,6 +618,13 @@ thrift_binary_protocol_read_list_begin (ThriftProtocol
*protocol,
return -1;
}
+ if(FALSE == ttc->checkReadBytesAvailable(THRIFT_TRANSPORT(tp->transport),
Review comment:
Same here
##########
File path: lib/c_glib/src/thrift/c_glib/protocol/thrift_compact_protocol.c
##########
@@ -1106,6 +1108,15 @@ thrift_compact_protocol_read_map_begin (ThriftProtocol
*protocol,
return -1;
}
+
+ if(FALSE == ttc->checkReadBytesAvailable(THRIFT_TRANSPORT (tp->transport),
Review comment:
again, and there are a few more below
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_server_transport.c
##########
@@ -21,15 +21,216 @@
#include <thrift/c_glib/transport/thrift_transport.h>
#include <thrift/c_glib/transport/thrift_server_transport.h>
+/* object properties */
+enum _ThriftServerTransportProperties
+{
+ PROP_0,
+ PROP_THRIFT_SERVER_TRANSPORT_CONFIGURATION,
+ PROP_THRIFT_SERVER_TRANSPORT_REMAINING_MESSAGE_SIZE,
+ PROP_THRIFT_SERVER_TRANSPORT_KNOW_MESSAGE_SIZE
+};
+
G_DEFINE_ABSTRACT_TYPE(ThriftServerTransport, thrift_server_transport,
G_TYPE_OBJECT)
+gboolean
+thrift_server_transport_updateKnownMessageSize(ThriftServerTransport
*transport, glong size, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftServerTransport *tst = THRIFT_TRANSPORT (transport);
+ ThriftServerTransportClass *tstc = THRIFT_SERVER_TRANSPORT_GET_CLASS
(transport);
+ glong consumed = tst->knowMessageSize_ - tst->remainingMessageSize_;
+ if(FALSE == tstc->resetConsumedMessageSize(transport, size, error))
+ {
+ boolean = FALSE;
+ }
+ if(FALSE == tstc->countConsumedMessageBytes(transport, consumed, error))
+ {
+ boolean = FALSE;
+ }
+ return boolean;
+}
+
+gboolean
+thrift_server_transport_checkReadBytesAvailable(ThriftServerTransport
*transport, glong numBytes, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftServerTransport *tst = THRIFT_SERVER_TRANSPORT (transport);
+ if(tst->remainingMessageSize_ < numBytes)
+ {
+ if(*error == NULL)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ }
+ boolean = FALSE;
+ }
+
+ return boolean;
+}
+
+gboolean
+thrift_server_transport_resetConsumedMessageSize(ThriftServerTransport
*transport, glong newSize, GError **error)
+{
+ ThriftServerTransport *tst = THRIFT_SERVER_TRANSPORT (transport);
+ if(newSize < 0)
+ {
+ if(tst->configuration != NULL)
+ {
+ tst->knowMessageSize_ = tst->configuration->maxMessageSize_;
+ tst->remainingMessageSize_ = tst->configuration->maxMessageSize_;
+ }
+ else
+ {
+ tst->knowMessageSize_ = 10485760;
+ tst->remainingMessageSize_ = 10485760;
+ }
+ return TRUE;
+ }
+ /* update only: message size can shrink, but not grow */
+ if(newSize > tst->knowMessageSize_)
+ {
+ if(*error == NULL)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ }
+ return FALSE;
+ }
+
+ tst->knowMessageSize_ = newSize;
+ tst->remainingMessageSize_ = newSize;
+
+ return TRUE;
+}
+
+gboolean
+thrift_server_transport_countConsumedMessageBytes(ThriftServerTransport
*transport, glong numBytes, GError **error)
+{
+ ThriftServerTransport *tst = THRIFT_SERVER_TRANSPORT (transport);
+ if(tst->remainingMessageSize_ > numBytes)
+ {
+ tst->remainingMessageSize_ -= numBytes;
+ }
+ else
+ {
+ tst->remainingMessageSize_ = 0;
+ if(*error == NULL)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ }
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+/* property accesor */
+void
+thrift_server_transport_get_property(GObject *object, guint property_id,
+ GValue *value, GParamSpec *pspec)
+{
+ ThriftServerTransport *transport = THRIFT_SERVER_TRANSPORT (object);
+
+ THRIFT_UNUSED_VAR (pspec);
+
+ switch (property_id)
+ {
+ case PROP_THRIFT_SERVER_TRANSPORT_CONFIGURATION:
+ g_value_set_object (value, transport->configuration);
+ break;
+ case PROP_THRIFT_SERVER_TRANSPORT_REMAINING_MESSAGE_SIZE:
+ g_value_set_long (value, transport->remainingMessageSize_);
+ break;
+ case PROP_THRIFT_SERVER_TRANSPORT_KNOW_MESSAGE_SIZE:
+ g_value_set_long (value, transport->knowMessageSize_);
+ break;
+ }
+}
+
+/* property mutator */
+void
+thrift_server_transport_set_property (GObject *object, guint property_id,
+ const GValue *value, GParamSpec *pspec)
+{
+ ThriftServerTransport *transport = THRIFT_SERVER_TRANSPORT (object);
+
+ THRIFT_UNUSED_VAR (pspec);
+
+ switch (property_id)
+ {
+ case PROP_THRIFT_SERVER_TRANSPORT_CONFIGURATION:
+ transport->configuration = g_value_get_object (value);
+ if(transport->configuration != NULL)
+ {
+ transport->remainingMessageSize_ =
transport->configuration->maxMessageSize_;
Review comment:
minor: indent?
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_buffered_transport.c
##########
@@ -379,6 +424,36 @@ thrift_buffered_transport_class_init
(ThriftBufferedTransportClass *cls)
PROP_THRIFT_BUFFERED_TRANSPORT_WRITE_BUFFER_SIZE,
param_spec);
+ param_spec = g_param_spec_object ("configuration",
+ "configuration (construct)",
+ "thrift configuration",
+ THRIFT_TYPE_CONFIGURATION,
+ G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY);
+ g_object_class_install_property (gobject_class,
+ PROP_THRIFT_BUFFERED_TRANSPORT_CONFIGURATION,
+ param_spec);
+
+ param_spec = g_param_spec_long ("remainingmessagesize",
+ "remainingmessagesize (construct)",
+ "Set the size of the remaining message",
+ 0, /* min */
+ G_MAXINT32, /* max */
+ 10485760, /* default by construct */
+ G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY);
+ g_object_class_install_property (gobject_class,
+
PROP_THRIFT_BUFFERED_TRANSPORT_REMAINING_MESSAGE_SIZE,
+ param_spec);
+
+ param_spec = g_param_spec_long ("knowmessagesize",
+ "knowmessagesize (construct)",
+ "Set the size of the know message",
+ 0, /* min */
+ G_MAXINT32, /* max */
+ 10485760, /* default by construct */
Review comment:
same, plus some more below
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_server_socket.c
##########
@@ -246,6 +270,20 @@ thrift_server_socket_set_property (GObject *object, guint
property_id,
case PROP_THRIFT_SERVER_SOCKET_BACKLOG:
socket->backlog = g_value_get_uint (value);
break;
+ case PROP_THRIFT_SERVER_SOCKET_CONFIGURATION:
+ transport->configuration = g_value_dup_object (value);
+ if(transport->configuration != NULL)
+ {
+ transport->remainingMessageSize_ =
transport->configuration->maxMessageSize_;
Review comment:
minor: indent?
##########
File path: lib/c_glib/src/thrift/c_glib/transport/thrift_transport.c
##########
@@ -129,17 +141,209 @@ thrift_transport_real_read_all (ThriftTransport
*transport, gpointer buf,
return have;
}
+gboolean
+thrift_transport_updateKnownMessageSize(ThriftTransport *transport, glong
size, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ ThriftTransportClass *ttc = THRIFT_TRANSPORT_GET_CLASS (transport);
+ glong consumed = tt->knowMessageSize_ - tt->remainingMessageSize_;
+ if(FALSE == ttc->resetConsumedMessageSize(transport, size, error))
+ {
+ boolean = FALSE;
+ }
+ if(FALSE == ttc->countConsumedMessageBytes(transport, consumed, error))
+ {
+ boolean = FALSE;
+ }
+ return boolean;
+}
+
+gboolean
+thrift_transport_checkReadBytesAvailable(ThriftTransport *transport, glong
numBytes, GError **error)
+{
+ gboolean boolean = TRUE;
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(tt->remainingMessageSize_ < numBytes)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ boolean = FALSE;
+ }
+
+
+ return boolean;
+}
+
+gboolean
+thrift_transport_resetConsumedMessageSize(ThriftTransport *transport, glong
newSize, GError **error)
+{
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(newSize < 0)
+ {
+ if(tt->configuration != NULL)
+ {
+ tt->knowMessageSize_ = tt->configuration->maxMessageSize_;
+ tt->remainingMessageSize_ = tt->configuration->maxMessageSize_;
+ }
+ else
+ {
+ tt->knowMessageSize_ = 10485760;
+ tt->remainingMessageSize_ = 10485760;
+ }
+ return TRUE;
+ }
+ /* update only: message size can shrink, but not grow */
+ if(newSize > tt->knowMessageSize_)
+ {
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ return FALSE;
+ }
+
+ tt->knowMessageSize_ = newSize;
+ tt->remainingMessageSize_ = newSize;
+
+ return TRUE;
+}
+
+gboolean
+thrift_transport_countConsumedMessageBytes(ThriftTransport *transport, glong
numBytes, GError **error)
+{
+ ThriftTransport *tt = THRIFT_TRANSPORT (transport);
+ if(tt->remainingMessageSize_ > numBytes)
+ {
+ tt->remainingMessageSize_ -= numBytes;
+ }
+ else
+ {
+ tt->remainingMessageSize_ = 0;
+ g_set_error(error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_MAX_MESSAGE_SIZE_REACHED,
+ "MaxMessageSize reached");
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+/* property accesor */
+void
+thrift_transport_get_property(GObject *object, guint property_id,
+ GValue *value, GParamSpec *pspec)
+{
+ ThriftTransport *transport = THRIFT_TRANSPORT (object);
+
+ THRIFT_UNUSED_VAR (pspec);
+
+ switch (property_id)
+ {
+ case PROP_THRIFT_TRANSPORT_CONFIGURATION:
+ g_value_set_object (value, transport->configuration);
+ break;
+ case PROP_THRIFT_TRANSPORT_REMAINING_MESSAGE_SIZE:
+ g_value_set_long (value, transport->remainingMessageSize_);
+ break;
+ case PROP_THRIFT_TRANSPORT_KNOW_MESSAGE_SIZE:
+ g_value_set_long (value, transport->knowMessageSize_);
+ break;
+ }
+}
+
+/* property mutator */
+void
+thrift_transport_set_property (GObject *object, guint property_id,
+ const GValue *value, GParamSpec *pspec)
+{
+ ThriftTransport *transport = THRIFT_TRANSPORT (object);
+
+ THRIFT_UNUSED_VAR (pspec);
+
+ switch (property_id)
+ {
+ case PROP_THRIFT_TRANSPORT_CONFIGURATION:
+ transport->configuration = g_value_get_object (value);
+ break;
+ case PROP_THRIFT_TRANSPORT_REMAINING_MESSAGE_SIZE:
+ transport->remainingMessageSize_ = g_value_get_long (value);
+ break;
+ case PROP_THRIFT_TRANSPORT_KNOW_MESSAGE_SIZE:
+ transport->knowMessageSize_ = g_value_get_long (value);
+ break;
+ }
+}
+
/* define the GError domain for Thrift transports */
GQuark
thrift_transport_error_quark (void)
{
return g_quark_from_static_string (THRIFT_TRANSPORT_ERROR_DOMAIN);
}
+static void
+thrift_transport_dispose (GObject *gobject)
+{
+ ThriftTransport *self = THRIFT_TRANSPORT (gobject);
+
+ if(self->configuration != NULL)
+ g_clear_object (&self->configuration);
+
+ /* Always chain up to the parent class; there is no need to check if
+ * the parent class implements the dispose() virtual function: it is
+ * always guaranteed to do so
+ */
+ G_OBJECT_CLASS (thrift_transport_parent_class)->dispose (gobject);
+}
+
/* class initializer for ThriftTransport */
static void
thrift_transport_class_init (ThriftTransportClass *cls)
{
+ GObjectClass *gobject_class = G_OBJECT_CLASS (cls);
+ GParamSpec *param_spec = NULL;
+
+ /* setup accessors and mutators */
+ gobject_class->get_property = thrift_transport_get_property;
+ gobject_class->set_property = thrift_transport_set_property;
+
+ param_spec = g_param_spec_object ("configuration",
+ "configuration (construct)",
+ "Thtift Configuration",
Review comment:
"Thtift Configuration" => "Thrift Configuration"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
