You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Diwaker Gupta (Created) (JIRA)" <ji...@apache.org> on 2011/10/18 22:35:11 UTC
[jira] [Created] (THRIFT-1394) Treatment of optional fields is not
consistent between C++ and Java
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
[jira] [Updated] (THRIFT-1394) Treatment of optional fields is not
consistent between C++ and Java
Posted by "Diwaker Gupta (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Diwaker Gupta updated THRIFT-1394:
----------------------------------
Attachment: THRIFT-1394.patch
> 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
> Labels: C++, Java
> Attachments: THRIFT-1394.patch
>
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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
[jira] [Commented] (THRIFT-1394) Treatment of optional fields is
not consistent between C++ and Java
Posted by "Diwaker Gupta (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13191481#comment-13191481 ]
Diwaker Gupta commented on THRIFT-1394:
---------------------------------------
Bump. Jake?
> 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
> Labels: C++, Java
> Attachments: THRIFT-1394.patch
>
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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
[jira] [Resolved] (THRIFT-1394) Treatment of optional fields is not
consistent between C++ and Java
Posted by "Jake Farrell (Resolved) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jake Farrell resolved THRIFT-1394.
----------------------------------
Resolution: Fixed
Fix Version/s: 0.9
Assignee: Diwaker Gupta (was: Jake Farrell)
Committed
> 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: Diwaker Gupta
> Labels: C++, Java
> Fix For: 0.9
>
> Attachments: THRIFT-1394.patch
>
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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
[jira] [Commented] (THRIFT-1394) Treatment of optional fields is
not consistent between C++ and Java
Posted by "Diwaker Gupta (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167961#comment-13167961 ]
Diwaker Gupta commented on THRIFT-1394:
---------------------------------------
Jake, considering that the current C++ <-> Java wire translation is inconsistent and likely to be inconsistent across other languages as well AND considering that this hasn't been a huge issue in the community so far, my sense is that most Thrift users just haven't run into this.
Given our limited resources, we are unable to provide patches for other client libraries. But I don't see why that should prevent this patch from going in -- it passes all tests and doesn't break any existing clients and/or functionality.
What would it take to get this patch into the next release?
> 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
> Labels: C++, Java
> Attachments: THRIFT-1394.patch
>
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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
[jira] [Commented] (THRIFT-1394) Treatment of optional fields is
not consistent between C++ and Java
Posted by "Jake Farrell (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131320#comment-13131320 ]
Jake Farrell commented on THRIFT-1394:
--------------------------------------
Diwaker, this patch fixes the immediate problem you have with c++ and java, but what about other client libraries (ruby, python, php, etc..) ?
> 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
> Labels: C++, Java
> Attachments: THRIFT-1394.patch
>
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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
[jira] [Commented] (THRIFT-1394) Treatment of optional fields is
not consistent between C++ and Java
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13194478#comment-13194478 ]
Hudson commented on THRIFT-1394:
--------------------------------
Integrated in Thrift #398 (See [https://builds.apache.org/job/Thrift/398/])
THRIFT-1394:Treatment of optional fields is not consistent between C++ and Java
Client: cpp
Patch: Diwaker Gupta
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.
jfarrell : http://svn.apache.org/viewvc/?view=rev&rev=1236529
Files :
* /thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc
* /thrift/trunk/lib/cpp/test/OptionalRequiredTest.cpp
* /thrift/trunk/test/OptionalRequiredTest.thrift
> 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: Diwaker Gupta
> Labels: C++, Java
> Fix For: 0.9
>
> Attachments: THRIFT-1394.patch
>
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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
[jira] [Updated] (THRIFT-1394) Treatment of optional fields is not
consistent between C++ and Java
Posted by "Diwaker Gupta (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Diwaker Gupta updated THRIFT-1394:
----------------------------------
Description:
*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 Foo {
1: optional string bar = "hello";
2: optional i32 baz;
}
{code}
In C++, the checks for 'bar' and 'i' are similar:
{code}
if (this->__isset.bar) {
xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
xfer += oprot->writeString(this->bar);
xfer += oprot->writeFieldEnd();
}
if (this->__isset.baz) {
xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
xfer += oprot->writeI32(this->baz);
xfer += oprot->writeFieldEnd();
}
{code}
However, in Java, the checks are different:
{code}
/** Returns true if field bar is set (has been assigned a value) and false otherwise */
public boolean isSetBar() {
return this.bar != null;
}
/** Returns true if field baz is set (has been assigned a value) and false otherwise */
public boolean isSetBaz() {
return __isset_bit_vector.get(__BAZ_ISSET_ID);
}
{code}
As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
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.
was:
*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.
> 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
> Labels: C++, Java
>
> *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 Foo {
> 1: optional string bar = "hello";
> 2: optional i32 baz;
> }
> {code}
> In C++, the checks for 'bar' and 'i' are similar:
> {code}
> if (this->__isset.bar) {
> xfer += oprot->writeFieldBegin("bar", ::apache::thrift::protocol::T_STRING, 1);
> xfer += oprot->writeString(this->bar);
> xfer += oprot->writeFieldEnd();
> }
> if (this->__isset.baz) {
> xfer += oprot->writeFieldBegin("baz", ::apache::thrift::protocol::T_I32, 2);
> xfer += oprot->writeI32(this->baz);
> xfer += oprot->writeFieldEnd();
> }
> {code}
> However, in Java, the checks are different:
> {code}
> /** Returns true if field bar is set (has been assigned a value) and false otherwise */
> public boolean isSetBar() {
> return this.bar != null;
> }
> /** Returns true if field baz is set (has been assigned a value) and false otherwise */
> public boolean isSetBaz() {
> return __isset_bit_vector.get(__BAZ_ISSET_ID);
> }
> {code}
> As a result, when the Java object is serialized, the value for 'bar' _is_ included. But when the C++ object is serialized, the value for 'bar' is NOT included.
> 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