PDA

View Full Version : [CLOSED][3.0rc2] potential null pointer dereference in JsonReader.readRecords



agfk
26 Jun 2009, 11:48 AM
Any objections to the following patch?



*** ext-all-debug.js Tue Jun 02 22:37:07 2009
--- ext-all-debug_patched.js Fri Jun 26 13:14:13 2009
***************
*** 13639,13645 ****
if (!this.ef) {
this.ef = this.buildExtractors();
}
! var root = this.getRoot(o), c = root.length, totalRecords = c, success = true;
if(s.totalProperty){
var v = parseInt(this.getTotal(o), 10);
if(!isNaN(v)){
--- 13639,13645 ----
if (!this.ef) {
this.ef = this.buildExtractors();
}
! var root = this.getRoot(o), c = (root != null ? root.length : 0), totalRecords = c, success = true;
if(s.totalProperty){
var v = parseInt(this.getTotal(o), 10);
if(!isNaN(v)){

evant
26 Jun 2009, 3:45 PM
Do you have a test case that demonstrates this, or was it just from looking at the code?

Note that the construction of getRoot is:



this.getRoot = s.root ? this.getJsonAccessor(s.root) : function(p){return p;};

agfk
26 Jun 2009, 8:58 PM
No, I actually ran into this problem the other day.

I used the following JsonReader (defined inside a FormPanel):


reader: new Ext.data.JsonReader({
root: 'myobject.data',
id: 'objectId',
successProperty: 'myobject.success'
},[ ... ])


When the server sends a null data object (which is sometimes more convenient than sending an empty array)


{"myobject":{"data":null,"message":"OK","success":true,"totalRows":0}}

the reader dies trying to dereference root.length.

evant
27 Jun 2009, 10:44 PM
Probably not a bug since it expects that you're returning an array of data, regardless I think we could possibly accommodate the case where the root is null.

mjlecomte
28 Jun 2009, 5:54 AM
Note the following:

The requirement that the root is an array has been noted in the Grid FAQ for sometime.

Changing this may have subsequent consequences as current documentation indicates an exception will be thrown.

In the OP's example, totalRows was properly typed to a number (0), so I'm not sure I see why the root can't properly be initialized as an array. The Grid FAQ suggests initializing the root to an array and then pushing data onto it, ensuring an array would be returned to client.

evant
28 Jun 2009, 8:26 AM
I don't think this is worth changing, it's quite well documented what kind of response is expected. As such, it's not a bug and works as described. Marking this as closed.

agfk
28 Jun 2009, 9:29 AM
I don't think this is worth changing, it's quite well documented what kind of response is expected. As such, it's not a bug and works as described. Marking this as closed.
Fair enough - thanks!