yiiGridView pagination stops working on ajax request

Skalin commented 4 weeks ago

The pagination and or filtration of grid views stops working upon trying to filter using Ajax request. This is caused by an updated jquery-bbq library. The library fails in "qs.HasOwnProperty" method.

What steps will reproduce the problem?

Trying to refresh, paginate or filter the Yii gridview causes javascript to fail. This is done even before any AJAX request is done. Simple call for: $("#grid-view").yiiGridView("update", {url: refreshURL}); will cause the bug.

What is the expected result?

The grid view is properly updated and the ajax request is processed.

What do you get instead?

Error response:

index:461 Uncaught TypeError: qs.hasOwnProperty is not a function
    at index:461:12
    at Function.<anonymous> (jquery.min.js:4:17819)
    at Function.each (jquery.min.js:2:2881)
    at g (jquery.min.js:4:17785)
    at Ub (jquery.min.js:4:17922)
    at Function.ajax (jquery.min.js:4:21073)
    at HTMLDivElement.<anonymous> (jquery.yiigridview.js:355:21)
    at Function.each (jquery.min.js:2:2881)
    at n.fn.init.each (jquery.min.js:2:846)
    at n.fn.init.update (jquery.yiigridview.js:260:16)

More info - the issue occurs in the function ajax.preFilter:

$.ajaxPrefilter(function (options, originalOptions, jqXHR) {
    var qs = $.deparam.querystring(options.url);
    if (qs.hasOwnProperty("ajax") && qs.ajax == "grid-view")

Additional info

Q	A
Yii version	1.1.30
PHP version	8.2
Operating system	Debian
jQuery version	v1.12.4 (Yii packed one)

Skalin commented 4 weeks ago

The temporary solution is to revert back to package 1.1.29. There might be a possibility to update jquery to newer versino using client script, but it has to be tested which version will work properly.

marcovtwout commented 3 weeks ago

@kevin-foster-uk It seems https://github.com/yiisoft/yii/pull/4563 has caused a regression, could you take a look at this?

@Skalin Could you point out exactly where you think the update jquery-bbq has caused this? I don't see it in your stack trace.

Skalin commented 3 weeks ago

Well the javascripts throws the error on the: "qs.hasOwnProperty" method call inside the ajaxPreFilter method.

I believe that the bbq library expects the deparam function to return object. I tried logging it from console and it returns the object, but somehow it does not have the methods of the object prototype.

Theoretically, rewriting the call "qs.hasOwnProperty("ajax")" to "Object.prototype.hasOwnProperty.call(qs, 'ajax')" should work. But I am not sure whether the issue would occur anywhere else.

kevin-foster-uk commented 3 weeks ago

@Skalin Can I ask what browser you are using? This update removed support for old IE browsers (edge is still supported).

In my testing using chrome, the ajax pagination is working fine.

Animation

Busterkinng88 commented 3 weeks ago

🤴

Skalin commented 2 weeks ago

Sorry for a bit of false alarm guys. :)

The issue happens in the Bootstrap extension (~~https://www.yiiframework.com/extension/bootstrap~~ https://github.com/clevertech/YiiBooster), it expects that the querystring returns proper object, but the bbq returns some weird kind of object which does not have the "hasOwnProperty" method. To be precise, the issue occurs here: https://github.com/clevertech/YiiBooster/blob/c1d377a82581250558e7a6ff446ef7088f859f9e/src/widgets/TbExtendedGridView.php#L756

This can be fixed by updating the bootstrap extension, nevertheless I think that the proper fix should be that the deparam.querystring from bbq library returns a standard Object.

What do you think?

marcovtwout commented 2 weeks ago

@Skalin The docblock above the function states it should return an object: https://github.com/LimeSurvey/yii/blob/ae2af5c65bb12dbd99cf0d69fdbaa4cc0b5f0ee1/framework/web/js/source/jquery.ba-bbq.js#L472

Can you post a full stack trace from your browsers inspector with clear input and output from the $.deparam function? What return path is taken inside the function?

kevin-foster-uk commented 1 week ago

The doc block is not correct. It will return undefined if the param is not found (line #569). The changes we made recently result in undefined being returned if the parameter is prohibited (line #494).

I am not sure what the solution is here. I guess we could return an empty string instead of null.

Skalin commented 1 week ago

Hi,

I am sending the whole trace that I got.

Input and ouptut of deparam.querystring method:

xpage:511 $.deparam.querystring(/en/xadmin/xpage/page/index?Page[id]=&Page[name]=&Page[title]=&Page[layout]=&Page[active]=1&Page_page=1&ajax=page-grid&yw3_options=on) =  
{
   Page: 
   {
         active: "1"
         id: ""
         layout: ""
         name: ""
         title: ""
   }
   Page_page: "1"
   ajax: "page-grid"
   yw3_options: "on"
}

Calling then the following method result in an exception:

 qs.hasOwnProperty("ajax") =

Error trace of the method (I am sorry, Yii has already the minified version and I was not able to make it use the non-minified version):

(anonymní) @ xpage:511
(anonymní) @ jquery.min.js:4
each @ jquery.min.js:2
g @ jquery.min.js:4
Ub @ jquery.min.js:4
ajax @ jquery.min.js:4
(anonymní) @ jquery.yiigridview.js:355
each @ jquery.min.js:2
each @ jquery.min.js:2
update @ jquery.yiigridview.js:260
$.fn.yiiGridView @ jquery.yiigridview.js:418
(anonymní) @ jquery.yiigridview.js:142
dispatch @ jquery.min.js:3
r.handle @ jquery.min.js:3
handleMouseUp_ @ neznámý
xpage:516 Uncaught TypeError: qs.hasOwnProperty is not a function
    at xpage:516:10
    at Function.<anonymous> (jquery.min.js:4:17819)
    at Function.each (jquery.min.js:2:2881)
    at g (jquery.min.js:4:17785)
    at Ub (jquery.min.js:4:17922)
    at Function.ajax (jquery.min.js:4:21073)
    at HTMLDivElement.<anonymous> (jquery.yiigridview.js:355:21)
    at Function.each (jquery.min.js:2:2881)
    at n.fn.init.each (jquery.min.js:2:846)
    at n.fn.init.update (jquery.yiigridview.js:260:16)

I think that this all comes to jquery.ba-bbq.js, where the object is being created with "null" as param:


    var obj = Object.create(null),
      coerce_types = { 'true': !0, 'false': !1, 'null': null };

Creating object this way results in missing methods like hasOwnProperty. Creating object as this will result in methods like "hasOwnProperty" to be present:

Object.create({});

Is this enough information?

marcovtwout commented 1 week ago

Reading the diagnosis and code again, the method still always returns object in the final line, consistent with the docblock. But Stalin is correct, what changed is that object no longer contains prototype methods:

@kevin-foster-uk When you create an object with Object.create(null), it will not inherit any object methods from prototype (see https://stackoverflow.com/questions/15518328/is-creating-js-object-with-object-createnull-the-same-as). Could you explain why you changed from var obj = {} to var obj = Object.create(null), was this intentional?

kevin-foster-uk commented 1 week ago

@marcovtwout The Object.create(null) is from the original PR submitted to the upstream (by @cee-chen) that we based our fix on. Thinking about it now, I can see this change is unnecessary since we now have an explicit check for prohibitedKeys (proto).

I will create a new PR to fix the issue.

marcovtwout commented 1 week ago

@kevin-foster-uk I would suggest to first push this upstream https://github.com/cowboy/jquery-bbq/issues/62 / https://github.com/cowboy/jquery-bbq/pull/61, that way more eyes can look at the security fix.

@Skalin Regardless of this fix, I suggest to submit a PR to https://github.com/clevertech/YiiBooster as well. Looking at that code, it doesn't really need "hasOwnProperty" but could just do qs.ajax !== undefined as well. A similar issue was reported earlier as well: https://github.com/clevertech/YiiBooster/issues/1052

kevin-foster-uk commented 1 week ago

A PR has been created: https://github.com/yiisoft/yii/pull/4571

yiisoft / yii