Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootgrid rowCount weirdness #2

Open
agh1467 opened this issue Jun 30, 2022 · 1 comment
Open

Bootgrid rowCount weirdness #2

agh1467 opened this issue Jun 30, 2022 · 1 comment

Comments

@agh1467
Copy link
Owner

agh1467 commented Jun 30, 2022

Unconfirmed at the moment, but cursory investigation leads to a change in how rowCount is being sent to the API causes an exception.

{
   "errorMessage" : "/usr/local/opnsense/mvc/app/controllers/OPNsense/Diagnostics/Api/LogController.php:76: Unsupported operand types",
   "errorTitle" : "An API exception occured",
   "errorTrace" : "
  #0 [internal function]: OPNsense\\Diagnostics\\Api\\LogController->__call(coreAction, Array)
  #1 [internal function]: Phalcon\\Dispatcher\\AbstractDispatcher->callActionMethod(Object(OPNsense\\Diagnostics\\Api\\LogController), coreAction, Array)
  #2 [internal function]: Phalcon\\Dispatcher\\AbstractDispatcher->dispatch()\n#3 /usr/local/opnsense/www/api.php(26): Phalcon\\Mvc\\Application->handle(/api/diagnostic...)\n#4 {main}"
}

The line in question is this one:
https://github.com/opnsense/core/blob/f159f68f9728348d35841721251325d613f4c053/src/opnsense/mvc/app/controllers/OPNsense/Diagnostics/Api/LogController.php#L76

                    ($currentPage - 1) * $itemsPerPage,

Checking the variables in this leads me to finding this in the $itemsPerPage:

array(7) {
  [0]=>  int(20)
  [1]=>  int(50)
  [2]=>  int(100)
  [3]=>  int(200)
  [4]=>  int(500)
  [5]=>  int(1000)
  [6]=>  int(5000)
}

I found that a bit unusual. I think that's supposed to be an integer, for whatever value is specifically selected for the bootgrid.

Looking at the request, I found the following form data of the request:

current | "1"
-- | --
rowCount[0] | "20"
rowCount[1] | "50"
rowCount[2] | "100"
rowCount[3] | "200"
rowCount[4] | "500"
rowCount[5] | "1000"
rowCount[6] | "5000"
searchPhrase | ""
severity[] | […]
    0 | "Emergency"
    1 | "Alert"
    2 | "Critical"
    3 | "Error"
    4 | "Warning"
current=1&rowCount%5B0%5D=20&rowCount%5B1%5D=50&rowCount%5B2%5D=100&rowCount%5B3%5D=200&rowCount%5B4%5D=500&rowCount%5B5%5D=1000&rowCount%5B6%5D=5000&searchPhrase=&severity%5B%5D=Emergency&severity%5B%5D=Alert&severity%5B%5D=Critical&severity%5B%5D=Error&severity%5B%5D=Warning

So the rowCount is being treated like an indexed array? But it doesn't look like severity which appears to be an array.

Looking into that I found this here:
https://github.com/opnsense/core/blame/master/src/opnsense/www/js/jquery.bootgrid.js#L81

    this.rowCount = localStorage.getItem('rowCount[' + this.uid + ']') || this.rowCount;

This appears to be the same syntax as I'm seeing in the form data being sent. Blame indicates it was changed 5 months ago covering another effort here: opnsense#5443

So, in my debugging I found that changing it like this:

this.rowCount = localStorage.getItem('rowCount[' + this.uid + ']') || this.rowCount[0];

Allows it to work again, and rowCount becomes an integer instead of an array.

current=1&rowCount=20&searchPhrase=&severity%5B%5D=Emergency&severity%5B%5D=Alert&severity%5B%5D=Critical&severity%5B%5D=Error&severity%5B%5D=Warning

Also in debugging, the localStorage appears to not have this setting. It does have a setting for severity after I've selected/messed with that setting.

With using the first index, some pages are now reporting:

Uncaught TypeError: that.rowCount is undefined

I'm also not seeing the row selection on any bootgrids, except I found one on Firewall -> Aliases. This bootgrid appears to operate correctly.

So it looks like there's possibly two issues to address here. The LogController, and the bootgrid javascript.

The LogController could probably go for validation of the input, and override with a default if given something non-integer.

The javascript could probably got for something similar.

The issue of the rowCount selector not appearing is something else, but I have a feeling it's related to this issue.

@agh1467
Copy link
Owner Author

agh1467 commented Jun 30, 2022

I found this guy here to mitigate the issue:

https://github.com/opnsense/core/blame/master/src/opnsense/www/js/jquery.bootgrid.js#L1001

I found that the rowCount isn't being detected by isArray() as an "array." And also that rowCount is showing up as an object. Adding some conditions for the object allows that ternary to work as expected, and select the [0] index of rowCount.

this.rowCount = ($.isArray(rowCount) || (typeof(rowCount) === 'object' && rowCount !== null)) ? rowCount[0] : rowCount;

Looking at the blame on that line, the line, and the whole section hasn't changed for 3 years.

Seems something else changed with how the rowCount is being presented and the code isn't ready to handle it. I need to look into it further to see. So far it doesn't look like issues have been reported on it. It may still be something with my machine. I'll probably do a full wipe and reset to make sure before submitting anything.

agh1467 pushed a commit that referenced this issue Mar 8, 2023
…n_array(): Argument #2 ($haystack) must be of type array, null given in /usr/local/www/vpn_ipsec_phase1.php:997
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant