You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "David Reiss (JIRA)" <ji...@apache.org> on 2008/09/08 19:39:44 UTC
[jira] Created: (THRIFT-135) Nulls in set throw an
exception in Java
Nulls in set<string> throw an exception in Java
-----------------------------------------------
Key: THRIFT-135
URL: https://issues.apache.org/jira/browse/THRIFT-135
Project: Thrift
Issue Type: Bug
Components: Compiler (Java)
Reporter: David Reiss
>From Amit Sudharshan:
I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "Johan Stuyts (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629230#action_12629230 ]
Johan Stuyts commented on THRIFT-135:
-------------------------------------
I prefer the first approach. I'd rather have the error be noticed as soon as possible instead of wondering what I did wrong and/or thinking Thrift does not work correctly.
If the documentation does not cleary state that {{null}} is not allowed in collections it should be added.
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (THRIFT-135) Nulls in set throw an exception in Java
Posted by Amit Sudharshan <am...@onellama.com>.
We should also remember to propogate changes to python and other
languages which have this issue
Sent from my iPhone
On Sep 8, 2008, at 1:41 PM, Amit Sudharshan <am...@onellama.com> wrote:
> A thriftexception with a well written message, would have saved
> quite a lot of digging to find the anomaly .
>
> Sent from my iPhone
>
> On Sep 8, 2008, at 1:33 PM, "Chad Walters (JIRA)" <ji...@apache.org>
> wrote:
>
>>
>> [ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629252#action_12629252
>> ]
>>
>> Chad Walters commented on THRIFT-135:
>> -------------------------------------
>>
>> I agree that #1 is best. Perhaps we should make sure this error
>> condition throws a Thrift exception rather than a Java NPE, though.
>>
>>> Nulls in set<string> throw an exception in Java
>>> -----------------------------------------------
>>>
>>> Key: THRIFT-135
>>> URL: https://issues.apache.org/jira/browse/THRIFT-135
>>> Project: Thrift
>>> Issue Type: Bug
>>> Components: Compiler (Java)
>>> Reporter: David Reiss
>>>
>>> From Amit Sudharshan:
>>> I recently noticed a bug(feature?) in
>>> com.facebook.thrift.protocol.TBinaryProtocol.writeString where if
>>> it is passed a null pointer it will throw NPE.
>>> Now, the autogenerated stub code tries to prevent this, however we
>>> recently came across a case where we had a Set<String> which
>>> contained a "NULL" (legal in java). Thrift tests to see if the set
>>> is non-null and implicitely whether it has any elements, both of
>>> these pass in this case, and so the null string is passed to the
>>> writeString method where we get the NPE.
>>
>> --
>> This message is automatically generated by JIRA.
>> -
>> You can reply to this email to add a comment to the issue online.
>>
Re: [jira] Commented: (THRIFT-135) Nulls in set throw an exception in Java
Posted by Amit Sudharshan <am...@onellama.com>.
A thriftexception with a well written message, would have saved quite
a lot of digging to find the anomaly .
Sent from my iPhone
On Sep 8, 2008, at 1:33 PM, "Chad Walters (JIRA)" <ji...@apache.org>
wrote:
>
> [ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629252#action_12629252
> ]
>
> Chad Walters commented on THRIFT-135:
> -------------------------------------
>
> I agree that #1 is best. Perhaps we should make sure this error
> condition throws a Thrift exception rather than a Java NPE, though.
>
>> Nulls in set<string> throw an exception in Java
>> -----------------------------------------------
>>
>> Key: THRIFT-135
>> URL: https://issues.apache.org/jira/browse/THRIFT-135
>> Project: Thrift
>> Issue Type: Bug
>> Components: Compiler (Java)
>> Reporter: David Reiss
>>
>> From Amit Sudharshan:
>> I recently noticed a bug(feature?) in
>> com.facebook.thrift.protocol.TBinaryProtocol.writeString where if
>> it is passed a null pointer it will throw NPE.
>> Now, the autogenerated stub code tries to prevent this, however we
>> recently came across a case where we had a Set<String> which
>> contained a "NULL" (legal in java). Thrift tests to see if the set
>> is non-null and implicitely whether it has any elements, both of
>> these pass in this case, and so the null string is passed to the
>> writeString method where we get the NPE.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
[jira] Commented: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "Chad Walters (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629252#action_12629252 ]
Chad Walters commented on THRIFT-135:
-------------------------------------
I agree that #1 is best. Perhaps we should make sure this error condition throws a Thrift exception rather than a Java NPE, though.
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-135:
---------------------------------
Fix Version/s: 0.1
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
> Fix For: 0.1
>
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (THRIFT-135) Nulls in set throw an exception in Java
Posted by Amit Sudharshan <am...@onellama.com>.
I agree that throwing NPE makes sense as I can see bad things
happening with the other cases (discrepencies in length, and ""
replacing null could break if/then logic).
I don't think I read this limitation on the docs, but could be wrong.
-Amit
Sent from my iPhone
On Sep 8, 2008, at 12:39 PM, "David Reiss (JIRA)" <ji...@apache.org>
wrote:
>
> [ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629225#action_12629225
> ]
>
> David Reiss commented on THRIFT-135:
> ------------------------------------
>
> I'm open to suggestions on how we should deal with this. My
> instinct is that we should not allow nulls in sets since they cannot
> be represented in C++ or PHP (though they can in Python). If we
> take that approach, I guess the choices are:
>
> 1/ Continue throwing an exception.
> 2/ Silently convert to "".
> 3/ Silently drop from the set.
>
> Thoughts?
>
>> Nulls in set<string> throw an exception in Java
>> -----------------------------------------------
>>
>> Key: THRIFT-135
>> URL: https://issues.apache.org/jira/browse/THRIFT-135
>> Project: Thrift
>> Issue Type: Bug
>> Components: Compiler (Java)
>> Reporter: David Reiss
>>
>> From Amit Sudharshan:
>> I recently noticed a bug(feature?) in
>> com.facebook.thrift.protocol.TBinaryProtocol.writeString where if
>> it is passed a null pointer it will throw NPE.
>> Now, the autogenerated stub code tries to prevent this, however we
>> recently came across a case where we had a Set<String> which
>> contained a "NULL" (legal in java). Thrift tests to see if the set
>> is non-null and implicitely whether it has any elements, both of
>> these pass in this case, and so the null string is passed to the
>> writeString method where we get the NPE.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
[jira] Commented: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629225#action_12629225 ]
David Reiss commented on THRIFT-135:
------------------------------------
I'm open to suggestions on how we should deal with this. My instinct is that we should not allow nulls in sets since they cannot be represented in C++ or PHP (though they can in Python). If we take that approach, I guess the choices are:
1/ Continue throwing an exception.
2/ Silently convert to "".
3/ Silently drop from the set.
Thoughts?
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12632790#action_12632790 ]
Bryan Duxbury commented on THRIFT-135:
--------------------------------------
How about we make a wrapper Set implementation that throws IllegalArgumentException if you try to add null, and use that as the implementation in generated structs?
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (THRIFT-135) Nulls in set throw an exception
in Java
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury closed THRIFT-135.
--------------------------------
Resolution: Won't Fix
I'm thinking that this is the right behavior.
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
> Fix For: 0.3
>
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12632813#action_12632813 ]
David Reiss commented on THRIFT-135:
------------------------------------
Normally we try to use the default collection types that the language provides to make Thrift feel more native. However, I think this is a bigger deal in C++ which uses nonvirtual methods. I think this would be okay in Java since the focus is more on the Set interface than any particular Set class. However, we should keep in mind that an application developer can put any Set implementation into a structure, so we would still have to do checking on serialization (unless we're okay with just throwing a null pointer exception).
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-135) Nulls in set throw an
exception in Java
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-135?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-135:
---------------------------------
Fix Version/s: (was: 0.1)
0.2
> Nulls in set<string> throw an exception in Java
> -----------------------------------------------
>
> Key: THRIFT-135
> URL: https://issues.apache.org/jira/browse/THRIFT-135
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Reporter: David Reiss
> Fix For: 0.2
>
>
> From Amit Sudharshan:
> I recently noticed a bug(feature?) in com.facebook.thrift.protocol.TBinaryProtocol.writeString where if it is passed a null pointer it will throw NPE.
> Now, the autogenerated stub code tries to prevent this, however we recently came across a case where we had a Set<String> which contained a "NULL" (legal in java). Thrift tests to see if the set is non-null and implicitely whether it has any elements, both of these pass in this case, and so the null string is passed to the writeString method where we get the NPE.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.