[psa] JSONObject.optString doesn't do what you think it does

Jim Chen nchen at mozilla.com
Thu May 22 20:50:56 PDT 2014


I think the problem comes down to null being an object type in JS (i.e.
typeof null === typeof {} === 'object'). Unlike Java where a String
variable can be null to indicate absence of value, in JS there's no such
thing as a "null string", because null is not related to strings.

That's why NativeJSObject will throw an IllegalArgumentException if you
call obj.getString("a") on obj={a:null}. It thinks "a" is an object, and
you're trying to treat it as a string. I think the correct approach here
is to use undefined to mark the absence of value instead of null. In
short, use undefined for a JS string where you would use null for a Java
String.

And NativeJSObject is not an exact drop-in replacement for JSONObject.
For example, the API for accessing arrays is different, and
NativeJSObject never does type conversion for you. With these
differences in mind, I don't think NativeJSObject has to be
bug-compatible with JSONObject either. We're still early enough in the
conversion process to make NativeJSObject behave in more reasonable
ways. It may take more effort now, but I think the possibility of
avoiding gotchas/subtle bugs down the road is worth it.

Cheers,
Jim


On 5/22/14 8:28 PM, Nick Alexander wrote:
> Hi m-f-d,
> 
> Just a friendly reminder that JSONObject.optString doesn't do what you
> think it does.  For terrible reasons documented at [1], if o =
> {'key':null}, then
> 
> JSONObject(o).optString("key") == "null"
> 
> and
> 
> JSONObject(o).optString("key", null) == "null"
> 
> That's right.  I discovered a few places in Fennec where NativeJSObject
> does a different thing [2], and then started to look at our use of
> optString, and it's not pretty.  Way too many uses to audit :(  So
> instead of trying to fix, I'm trying to educate instead.  Be careful
> when passing null from JS to Java!
> 
> We could try to make NativeJSObject do the right thing, but we have
> already gone quite far down the path of NativeJSObject is a drop-in
> replacement for JSONObject; I don't think we can go back.  And
> converting between the two conventions is as hard as auditing all
> current uses, amortized over time.
> 
> Nick
> 
> [1] http://stackoverflow.com/a/23377941
> 
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1014965
> _______________________________________________
> mobile-firefox-dev mailing list
> mobile-firefox-dev at mozilla.org
> https://mail.mozilla.org/listinfo/mobile-firefox-dev


More information about the mobile-firefox-dev mailing list