openwrt-openwisp-monitoring icon indicating copy to clipboard operation
openwrt-openwisp-monitoring copied to clipboard

[bug] 'pairs' on unexpected variable value

Open labertasche42 opened this issue 2 years ago • 2 comments

I have too many devices and thus even in the normal openwrt 22 cli, ubus call network.device status throws a time out. Entirely my problem, but through this time out https://github.com/openwisp/openwrt-openwisp-monitoring/blob/6709f353f1bd3a99282e9e656e29b5489cd4f1c4/openwisp-monitoring/files/sbin/netjson-monitoring.lua#L65 gets an unexpected assignment and by this https://github.com/openwisp/openwrt-openwisp-monitoring/blob/6709f353f1bd3a99282e9e656e29b5489cd4f1c4/openwisp-monitoring/files/sbin/netjson-monitoring.lua#L123 fails. This in turn results in an empty report in this in a HTTP 400 from the openwisp server.

The solution could be to either check every variable, before pairs is used on it and omit this part in the report, if there should be an error, or to abort the whole script, if ubus:call has an error.

labertasche42 avatar Jan 25 '23 15:01 labertasche42

@labertasche42 interesting! Does it always timeout or if you keep repeating it would work?

nemesifier avatar Jan 25 '23 20:01 nemesifier

Hey @nemesisdesign,

as every timeout is 30 seconds I just did test 6 calls, but yes, even after 6 calls it always times out. But this is a problem of the ubus command line tool and should probably a report somewhere else.

~~openwrt-openwisp-monitoring's~~ Line 123 and Line 112's problem is missing input sanitization which subsequently led in my case to errors like #101 or #102 In my opinion it should be something like

if type(x) == "table" then 
for a, b in pairs(x) do
...
else
 [errorreport]

For my case, I changed L65 to local network_status = {} for now to get at least all the other informations. A filter in the ubus:call in L65 would have worked too, but I was too lazy to recode L124ff

labertasche42 avatar Jan 26 '23 10:01 labertasche42