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

Blank arguments can lead to extra spaces in script arguments #2999

Closed
vaclav-krauz opened this issue Oct 2, 2019 · 7 comments
Closed

Blank arguments can lead to extra spaces in script arguments #2999

vaclav-krauz opened this issue Oct 2, 2019 · 7 comments
Labels
bug Undesired behaviour resolved A fixed issue
Milestone

Comments

@vaclav-krauz
Copy link

vaclav-krauz commented Oct 2, 2019

Describe the bug

version 1.2.6

When a device has set snmp version 2, it has empty items of snmp settings for version 3 and when I have data input method (script server) where I use all snmp settings like special type codes [snmp_community]...:

ss_gateway_wan_gpns_monitoring.php <func> <ip> <interface> <circuit_size_down> <circuit_size_up> <_community> ...

I get this error from poller:

ERROR PHP ERROR: Uncaught ArgumentCountError: Too few arguments to function ss_get_bandwidth(), 8 passed in /opt/mikenopa/cacti/site/script_server.php on line 222 and exactly 14 expected

when I check it in poller_item, I can see this:

...oring.php 'ss_get_bandwidth' '172.17.0.3' '2' '100000' '100000' 'public'       '2' '161' '500'

Screenshot at 2019-10-02 13-26-11

there is a big gap between 'public' and '2' (the yellow lines), so required fields <_community> <_username> <_password> <_auth_protocol> <_priv_passphrase> <_priv_protocol> <_context> <_version> are translated like an empty character without quotes

Screenshot at 2019-10-02 12-53-43

Expected behavior
cacti should store the full script path without the extra spaces like " ... 'public' '' '' '' '' '' '' '2' ..."

@vaclav-krauz
Copy link
Author

vaclav-krauz commented Oct 2, 2019

I would do something like this

1517 function get_full_script_path($local_data_id) {
...
1548         if (cacti_sizeof($data)) {
1549                 foreach ($data as $item) {
1550                         $value = cacti_escapeshellarg($item['value']);
1551                         if ('' === $value) {
1552                             $value = "''";
1553                         }   
1554                         $full_path = str_replace('<' . $item['data_name'] . '>', $value, $full_path);
1555                 }
1556         }

it works now

POLLER: Poller[1] Device[17] DS[116] SERVER: /opt/mikenopa/cacti/site/scripts/ss_gateway_wan_gpns_monitoring.php 'ss_get_bandwidth' '172.17.0.3' '2' '100000' '100000' 'public' '' '' '' '' '' '' '2' '161' '500', output: circuit_size_down:100000 circuit_size_up:100000 in_octets:3712342191 out_octets:1805196573

@netniV netniV changed the title wrong replacement of arguments in full script path Blank arguments can lead to extra spaces in script arguments Oct 2, 2019
@netniV
Copy link
Member

netniV commented Oct 2, 2019

I've updated the title of the issue and fixed up a bit of formatting in your original post (feel free to edit and see how it works).

If a parameter is replaced, it should be replaced with a blank and I believe that it is. The problem is that the space before or after that argument is not trimmed because we don't know if it should be or not.

To be fair though, I have never really noticed this issue with my own systems but does lead to an interesting thought this may be what's affecting one or two other obscure errors we've seen in the past.

@netniV
Copy link
Member

netniV commented Oct 2, 2019

Oh and thanks for the technical break down too, that helps 👍

@cigamit
Copy link
Member

cigamit commented Oct 3, 2019

Just modify your Data Input method and enclose the arguments in single quotes:

php -q <path_cacti>/scripts/somescript.sh '<hostname>' '<some_argument>'

That should immediately solve the issue.

@cigamit
Copy link
Member

cigamit commented Oct 3, 2019

I guess we could add that simple check, and just trim the single quotes for people who have already made this change too. Less tickets for us.

@cigamit
Copy link
Member

cigamit commented Oct 3, 2019

Have to be careful about Windows though.

@cigamit cigamit added the enhancement General tag for an enhancement label Oct 3, 2019
@netniV netniV added this to the v1.3.0 milestone Oct 3, 2019
cigamit added a commit that referenced this issue Nov 9, 2019
Blank arguments can lead to extra spaces in script arguments
@cigamit cigamit added bug Undesired behaviour and removed enhancement General tag for an enhancement labels Nov 9, 2019
@cigamit cigamit modified the milestones: v1.3.0, v1.2.8 Nov 9, 2019
@cigamit
Copy link
Member

cigamit commented Nov 9, 2019

Good catch. Bad assumption on my part. Resolved.

@cigamit cigamit closed this as completed Nov 9, 2019
@cigamit cigamit added the resolved A fixed issue label Nov 9, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

3 participants