Treatment of optional fields is not consistent between C++ and Java
-------------------------------------------------------------------
Key: THRIFT-1394
URL: https://issues.apache.org/jira/browse/THRIFT-1394
Project: Thrift
Issue Type: Bug
Components: C++ - Compiler, Java - Compiler
Affects Versions: 0.7
Environment: Mac OSX, Thrift trunk
Reporter: Diwaker Gupta
Assignee: Jake Farrell
*Motivation*
We are trying to implement message signing in the application layer for an
internal project. The transport is over Thrift -- the server side is using Java
and the client side is using C++. We noticed that the message signing would
fail because the client and server would serialize the same Thrift object in
different ways.
*Problem*
The semantics of "optional" fields differ between thrift code generated for
Java and CPP. In CPP, all optional fields are guarded by the {{isset}} helper
struct. On Java, however, the generated code takes advantage of nullable types:
for containers, structs, exceptions, enums, and, notably, strings, the
generator elides explicit use of an "isset" bit vector and instead emits checks
of the form "field null". This leads to varying behavior between the two
languages: an optional string field with a default value will have
{{isset[fieldid]}} false on C, but the equivalent test in Java will be true.
To be concrete, consider the following example:
{code}
struct Bar {
1: required i32 foo;
}
struct Foo {
1: optional Bar bar;
2: optional i32 i;
}
{code}
In C++, the checks for 'bar' and 'i' are similar:
{code}
if (this->__isset.bar) {
xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRUCT,
1);
xfer += this->bar.write(oprot);
xfer += oprot->writeFieldEnd();
}
if (this->__isset.i) {
xfer += oprot->writeFieldBegin("i", ::apache::thrift::protocol::T_I32, 2);
xfer += oprot->writeI32(this->i);
xfer += oprot->writeFieldEnd();
}
{code}
However, in Java, the checks are different:
{code}
public boolean isSetI() {
return __isset_bit_vector.get(__I_ISSET_ID);
}
/** Returns true if field bar is set (has been assigned a value) and false
otherwise */
public boolean isSetBar() {
return this.bar != null;
}
{code}
For a system like Thrift, I would eschew a few bytes saved over correctness and
reliability. As a user of Thrift, I *do* expect that the wire data generated
for identical Thrift objects will be identical, regardless of the language used.
*Proposal*
We already use a BitSet to track primitive types in Java. The compiler should
extend the bit vector to also guard nullable types, to be consistent with C++.
This is pretty easy and low impact -- I'm happy to provide a patch.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
