qooxdoo / qooxdoo

qooxdoo - Universal JavaScript Framework
http://qooxdoo.org
Other
764 stars 259 forks source link

Native JSON support (BZ#1400) #1561

Closed qx-bug-importer closed 8 years ago

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

From the IEBlog at http://blogs.msdn.com/ie/archive/2008/09/10/native-json-in-ie8.aspx

"The Jscript engine for IE8 (as of Beta 2) has a full native implementation of JSON that significantly improves the speed of serialization, deserialization and improves the overall safety of parsing untrusted payloads while maintaining compliance with JSON support as described in ES3.1 Proposal Working Draft. [...] So if you are using eval() or even your own JSON library, consider checking for the native implementation in IE8 to get increased performance and safer operation."

Start using native JSON once IE8 or other browsers with native JSON support become available ( Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=408838 or Safari: https://bugs.webkit.org/show_bug.cgi?id=20031 )

assigned to Fabian Jakobs (@fjakobs)

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

Native JSON in Firefox 3.1: http://blog.mozilla.com/webdev/2009/02/12/native-json-in-firefox-31/

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

* BZ#1951 has been marked as a duplicate of this bug. *

qx-bug-importer commented 15 years ago

Fabian Jakobs (@fjakobs) wrote:

If we use native JSON it must be clear that our current approach to encode Date in JSON-RCP will no longer work. It should be discussed whether it still makes sense to have special support for date data types. This essentially blocks us from using native JSON encoding and deconding on the client and on the server side.

qx-bug-importer commented 15 years ago

Andreas Junghans (andreas.junghans) wrote:

Good point about the Date handling. It sure is nice to have transparent Date support (without having to traverse the whole object tree to parse Date strings, which can be a considerable performance hit). But switching to native JSON parsing seems more important to me in the long run, so I think the Date handling can be removed in trunk.

Derrell: Any thoughts on this?

qx-bug-importer commented 15 years ago

Sebastian Werner (@swernerx) wrote:

I think the same. Native JSON parse capabilities are higher priority than customized date handling. Are there any other widely-used ideas how to deal with dates in JSON and still being compatible?

qx-bug-importer commented 15 years ago

Derrell Lipman (@derrell) wrote:

Yes, I have strong feelings about this. I find it ludicrous that JSON doesn't support dates in some standardized fashion, but it is critical, as far as I'm concerned, that dates be supported. That doesn't mean, however, that they the current inefficient implementation must always be used. The PHP backend already has provisions for using "native" encoders if the user specifies that date support is not required. I'd propose something similar on the frontend. If date support isn't required then use the fast, efficient, built-ins; and if it is necessary, continue to use the existing mechanism.

Dates are just sooooooo important of a feature to be able to transfer reliably from client to server that it's completely unreasonable not to support them properly.

Sigh.

Derrell

qx-bug-importer commented 15 years ago

Fabian Jakobs (@fjakobs) wrote:

Derrell,

what's wrong with just transferring the numeric time value as returned by Date.getValue()? It's as easy as "new Date(value)" to convert it back to a Date instance. Since JavaScript has no buildin Date literal I don't feel it belongs into a JSON file.

qx-bug-importer commented 15 years ago

Sebastian Werner (@swernerx) wrote:

I think the issue is that in this case it is no Date instance already when trying to work with it. I don't think that's a huge topic, but it is a change from the previous solution. Other then that I would really prefer such a solution. The argument that there is no built-in literal for Dates is really good.

qx-bug-importer commented 15 years ago

Andreas Junghans (andreas.junghans) wrote:

I agree with Derrell that native Date support is a big benefit. There's no doubt about the interpretation of a transmitted date ("UTC" is right there in the value!), it's convenient to work with, and it's fast. If you're transmiting lots of Date values, walking the tree to convert numbers (or strings) to dates can take some time.

For me, the main argument for dropping the current Date handling (NB: I'm the one who introduced it in the first place) is standards compliance. Swimming against the stream (in this case the upcoming native JSON parsers) is always a problem. It means more code, less security checks, and possible compatibility problems. For example, eval() can handle keys like {key:"value"} just fine (without the quotes), while some JSON libs in other languages don't like them (and rightfully so). Using a native parser would enforce proper syntax. Validation of JSON results also becomes a non-issue (for the framework), which makes it much safer to exchange JSON data with unknown servers. Since you don't have to resort to complex regexes, you get good performance and good security at the same time - at least that's the idea ;-)

qx-bug-importer commented 15 years ago

Sebastian Werner (@swernerx) wrote:

Just want to mention that normally there is no need to post-process the tree to translate all date strings to date instances. This can happen in the user code, where the variable is accessed as well. Normally one knows about the content of a specific path in the "tree" so that it could easily be processed there in the application code.

Security and performance are really good arguments as well. Dealing with two different ways to parse JSON is not a good idea though as it would additional overhead to the code base.

To interpret JSON the standard way is also best for people worked with other solutions before. This is even more compatible to other frameworks that might be used in conjunction with qooxdoo.

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

I'm also in favor of being/becoming standards compliant here.

One thing to note is that the actual conversion to Date objects (or to rather arbitrary instances when implementing kinda class hinting) could be handled "on-demand" by the application code, instead of expensive post-processing of all the data. Sure, it's much nicer to have that handled transparently some place else, unfortunately JSON implementations won't let us do so ...

What could be of interest to application developers is to use the optional "reviver" capability. AFAIK only IE8 offers such a user-defined function (FF3.1+ anyone?). It is applied to all nodes, and therefore I expect significant drops in performance. Maybe future JS engines are able to seamlessly blend such functions into their jitted code, just a guess. IE won't be the browser to spearhead this, though.

Talking of the "on-demand" conversion at access time in an application, I also like to add that much of the JSON handling of future qooxdoo apps may be done with the data binding layer currently being developed. Converting specific model data to Date objects (and vice versa, even including incremental caching of converted objects) could probably handled quite easily using data binding features.

qx-bug-importer commented 15 years ago

Helder Magalhães (helder.magalhaes) wrote:

> To interpret JSON the standard way is also best for people worked with other > solutions before. This is even more compatible to other frameworks that might > be used in conjunction with qooxdoo.

I generally agree with standards here. Nevertheless, I've always been a fan of this qooxdoo extension to JSON so I seriously recommend submitting it to an appropriate mailing list [1] for evaluation/discussion, so that the qooxdoo extension can possibly be part of a future version of the standard [2](or commonly agreed to be a bad idea for security and interoperability reasons).

Regards, Helder Magalhães

[1] http://tech.groups.yahoo.com/group/json/ [2] http://www.ietf.org/rfc/rfc4627.txt

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

Too important to be left unassigned...

qx-bug-importer commented 14 years ago

Helder Magalhães (helder.magalhaes) wrote:

> [...] I've always been a fan of this [the date] qooxdoo extension to JSON so I > seriously recommend submitting it to an appropriate mailing list [1] > for evaluation/discussion

For the record, although I see this as a good idea, as this isn't part of the standard and so not supported by (both client-side and server-side) implementations in an interoperable way, we are not using it: currently, we are sending dates in a two integer array, first meaning seconds and the second meaning the fraction within the second. Of course this is only to be seen as an example, although it sounds like a compact and intuitive way to specify dates in a JSON-friendly way. :-)

> Converting specific model data to Date objects (and vice versa, even including > incremental caching of converted objects) could probably handled quite easily > using data binding features.

+1 for this approach. Basically that's similar to we are already doing: using a shared date object to perform the conversion (in the client-side component). Using the data binding capabilities would probably be even simpler and more intuitive. ;-)

qx-bug-importer commented 14 years ago

Colin Snover (bugzilla.qooxdoo.org) wrote:

A test to evaluate speed difference between eval and JSON.parse with a callback function

I created a fairly hacky test script that iterates 50 times over a generated string of JSON containing 1000 fake jCard records (attached), and then re-serializes them.

Execution against Firefox 3.5 shows these numbers:

JSON.parse Min: 72 Max: 905 Avg: 91.54 Avg w/o outliers: 75

JSON.stringify Min: 33 Max: 446 Avg: 42.34 Avg w/o outliers: 34.125

qx.util.Json.parseQx Min: 21 Max: 574 Avg: 44.06 Avg w/o outliers: 33.5

qx.util.Json.stringify Min: 231 Max: 2482 Avg: 329.36 Avg w/o outliers: 286.5625

JSON.parse seems to be about 75% slower than the current method of doing a straight eval. However, JSON.stringify is 778% faster than the current stringify method.

I personally believe that the extra protection against XSS attacks that JSON.parse provides is worth the performance decrease on the deserialization side, and of course the disproportionately higher performance of JSON.stringify speaks for itself. (This performance benefit will also be realized on server-side JSON encoding, though probably to a lesser extent. I haven’t looked at that.)

Hope this is helpful!

ERROR: no way found to migrate Application.js (application/javascript) from bugzilla.

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

Maintaining our current implementations seems to becomes more and more a problem (cf. #2078, #2565). As mentioned in BZ#2565 I would suggest:

  1. deprecate qx.util.Json
  2. create qx.bom.Json, which wraps the native JSON object on browsers, which support it and closely emulate it if it is not available.
  3. replace all uses of qx.util.Json with qx.bom.Json

The main problem will be Date support. However the native JSON object supports custom serializer and deserializer, which could be used to emulate the old Date support if necessary.

Here are examples from the standard document:

myData = JSON.parse(text, function (key, value) { var a; if (typeof value === 'string') { a = /^(\d{4})(\d{2})(\d{2})T(\d{2}):(\d{2}):(\d{2}(?:.\d*)?)Z$/.exec(value); if (a) { return new Date(Date.UTC(+a[1], +a[2] - 1, +a[3], +a[4], +a[5], +a[6])); } } return value; });

text = JSON.stringify([new Date()], function (key, value) { return this[key] instanceof Date ? 'Date(' + this[key] + ')' : value; });

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

Given the potential risk of XSS attacks I would advocate to use a real JSON parser in the non native mode instead of using "eval"

qx-bug-importer commented 14 years ago

Derrell Lipman (@derrell) wrote:

> * parse json and convert date strings to date instances: > > myData = JSON.parse(text, function (key, value) { > var a; > if (typeof value === 'string') { > a = > /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2}(?:.\d*)?)Z$/.exec(value); > if (a) { > return new Date(Date.UTC(+a[1], +a[2] - 1, +a[3], +a[4], > +a[5], +a[6])); > } > } > return value; > });

This looks to me like every "string" will be run through the regular expression parser. Don't you think that will slow things down, possibly substantially, parsing large JSON documents?

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

If you want this automatic conversion, then yes, it might slow things down. However as Sebastian and Helder already noted, most of the time the conversion to Date can easily be done in the application code e.g. in a data binding layer. For small JSON files it should not matter too much.

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

Further its the way the native JSON object is supposed to work according to the EcmaScript 3.1 specification and this is what the browsers implement.

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

I would suggest we port http://json.org/json2.js to qooxdoo. This is the JSON reference implementation by Douglas Crockford

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

mass renaming of 0.9 target to 1.0 (for issues with status "NEW")

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

This is a larger topic that should be addressed by looking at the latest specs and implementations. Also the ongoing improvements and harmonizations in the RPC backends may provide valuable input. Thanks to everyone for all the input so far. Marked this correctly as enhancement, with high prio, unassign, moved to 1.1.

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

I have ported json2 to qooxdoo, fixed some bugs and added most of the tests from the EcmaScript 5 test suite. (rev. r21354)

The new JSON class is now at qx.lang.Json and should behave exactly the same as window.JSON in modern browsers. This also means that it doen't support the Date extension and probably never will.

I would like to deprecate qx.util.Json but this will only be possible if the RPC code is updated to handle dates differently. I've opened a separate BZ#3335 for the deprecation

qx-bug-importer commented 14 years ago

Sebastian Werner (@swernerx) wrote:

Just my cents:

qx-bug-importer commented 14 years ago

Sebastian Werner (@swernerx) wrote:

qx.Class.define("qx.lang.Json", { statics : { parse : window.JSON && JSON.parse('{"x":1}').x === 1 ? window.JSON.parse : function() { ... },

 ...

} });

In our tests it was important to try the parse() method instead of just detecting the availability because of issues on iPhone/Android where they had versions with not-fully supported implementations.

qx-bug-importer commented 14 years ago

Andreas Junghans (andreas.junghans) wrote:

Regarding comment #25:

> What I would optimize is the usage of eval() - in jQuery 1.4 they use new Function instead which was a lot faster in their tests. Just look at their code.

I don't believe "new Function" is faster - it's actually significantly slower in Safari and only as fast as eval() in other browsers. I suppose the jQuery guys have been bitten by Firebug - see this thread on the mailing list: http://n2.nabble.com/JSON-eval-vs-Function-td4174532.html

In short, Firebug causes a huge performance hit when using eval(). I wouldn't object to the "new Function" variant if it weren't for the performance problems it causes in Safari. Or maybe the package "Firefox+Firebug" is considered a more important target platform than Safari by now? (I'm only half joking ...)

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

Fabian, reopened this report so you can comment on the add'tl input and maybe update the current implementation.

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

> * What I would optimize is the usage of eval() - in jQuery 1.4 they use new > Function instead which was a lot faster in their tests. Just look at their code.

I'll have to do some benchmarks myself but for the moment I'll stick with eval. See Andreas' comment as well.

> * I don't like the way you deal with the alternative implementation, putting it > into a separate class. This for example means an additional function call for > the native methods. In our local implementation we just assign "parse" to > JSON.parse when available and otherwise point to our local implementation which > is directly implemented in this class. The "Impl" class may confuse users.

I've introduced the impl class to make testing easier. I have to test the qooxdoo implementation and this is not possible in modern browsers we use define the implementation inline. I don't think it will confuse users because nobody is going to extend this class and it doesn't show up in the API viewer because it is marked as internal.

However I've changed the implementation a little to get rid of the indirection if the browser supports JSON natively.

> * I am not sure that the "lang" namespace is a good match here. But I also do > not like the "Type" class there. It seems that the original intention of the > namespace, to just contain classes for every native type, is not given anymore. > Json is just not a type in a group of Array, Boolean etc. I would prefer the > Json class under "qx.util".

With ES5 JSON becomes part of the language, just as RegExp, Date or String. For this reason qx.lang is a perfect place.

> * Just to mention this. I would think that the qx.lang.Type should be splitted > and moved individually to the separate classes in qx.lang. e.g. > qx.lang.Type.isString => qx.lang.String.isString(). This would be comparable to > the ES5 way where the native Array/String etc will have a method isXXX(), too. > See: http://twitter.com/kangax/status/7008405285

Right, but in my opinion that is not enough to justify an API change.

qx-bug-importer commented 14 years ago

Fabian Jakobs (@fjakobs) wrote:

The fixes are in r21361

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

.

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

fix assignment

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

bug report corrected

qx-bug-importer commented 10 years ago

Martin Wittemann (@wittemann) wrote:

Closed all bugs already shipped with a release.