[
https://issues.apache.org/jira/browse/THRIFT-3706?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15877088#comment-15877088
]
ASF GitHub Bot commented on THRIFT-3706:
----------------------------------------
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1200#discussion_r102356256
--- Diff:
lib/c_glib/src/thrift/c_glib/protocol/thrift_multiplexed_protocol.c ---
@@ -42,146 +41,119 @@ static GParamSpec
*thrift_multiplexed_protocol_obj_properties[PROP_THRIFT_MULTIP
gint32
thrift_multiplexed_protocol_write_message_begin (ThriftMultiplexedProtocol
*protocol,
- const gchar *name, const ThriftMessageType message_type,
- const gint32 seqid, GError **error)
+ const gchar *name, const ThriftMessageType message_type,
+ const gint32 seqid, GError **error)
{
- gint32 ret;
- gchar *service_name = NULL;
- g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
+ gint32 ret;
+ gchar *service_name = NULL;
+ g_return_val_if_fail (THRIFT_IS_MULTIPLEXED_PROTOCOL (protocol), -1);
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL
(protocol);
- ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
- ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
+ ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (protocol);
+ ThriftMultiplexedProtocolClass *multiplexClass =
THRIFT_MULTIPLEXED_PROTOCOL_GET_CLASS(self);
+ ThriftProtocolClass *cls = THRIFT_PROTOCOL_CLASS (multiplexClass);
- if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
- service_name = g_strdup_printf("%s%s%s", self->service_name,
self->separator, name);
+ if( (message_type == T_CALL || message_type == T_ONEWAY) &&
self->service_name != NULL) {
+ service_name = g_strdup_printf("%s%s%s", self->service_name,
THRIFT_MULTIPLEXED_PROTOCOL_DEFAULT_SEPARATOR, name);
+ }else{
+ service_name = g_strdup(name);
+ }
- }else{
- service_name = g_strdup(name);
- }
+ // relay to the protocol_decorator
+ ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
- // relay to the protocol_decorator
- ret = thrift_protocol_decorator_write_message_begin(protocol,
service_name, message_type, seqid, error);
+ g_free(service_name);
- g_free(service_name);
-
- return ret;
+ return ret;
}
-
-
static void
thrift_multiplexed_protocol_set_property (GObject *object,
- guint property_id,
- const GValue *value,
- GParamSpec *pspec)
+ guint property_id,
+ const GValue *value,
+ GParamSpec *pspec)
{
- ThriftMultiplexedProtocol *self = THRIFT_MULTIPLEXED_PROTOCOL (object);
-
- switch (property_id)
- {
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SERVICE_NAME:
- if(self->service_name!=NULL)
- g_free (self->service_name);
- self->service_name= g_value_dup_string (value);
- break;
-
- case PROP_THRIFT_MULTIPLEXED_PROTOCOL_SEPARATOR:
--- End diff --
Given it is a Thrift Protocol Decorator and the field in question can only
be a string containing the name of the multiplexed service and colon and name
of the original call, I don't see how it would be possible for any protocol to
be confused by this always being a colon. Protocols never see it. In the
decoding phase the call name is a string, so we will always read a string
length and then a set of characters, and we will always pass this along to the
Multiplexed Processor. What you suggest means that a "string read" method for
a protocol would be confused by a colon which shouldn't ever happen.
In theiry it might be possible for a Transport to be confused, however if a
Transport is even looking at the content of the payload at all that is a poorly
written transport. So as I see it, using a fixed string across all languages
seems quite reasonable.
Now that said, I didn't check the MultiplexedProtocol implementations to
make sure they throw an exception if a colon is used in the multiplexed service
name. That should probably be a requirement for all implementations.
In this case, allowing the colon separator to be changed to something else,
seems like too much complexity for no gain.
> There's no support for Multiplexed protocol on c_glib library
> -------------------------------------------------------------
>
> Key: THRIFT-3706
> URL: https://issues.apache.org/jira/browse/THRIFT-3706
> Project: Thrift
> Issue Type: Improvement
> Components: C glib - Library
> Affects Versions: 0.9.3
> Reporter: Gonzalo Aguilar
> Assignee: James E. King, III
> Fix For: 0.11.0
>
>
> There's no multiplexed protocol.
> I will implement the same way it's done in Java an C++
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
