Skip to content

Add Devices filtering#18

Merged
netniV merged 3 commits intoCacti:developfrom
unka65:device-filter
Jun 28, 2021
Merged

Add Devices filtering#18
netniV merged 3 commits intoCacti:developfrom
unka65:device-filter

Conversation

@unka65
Copy link
Contributor

@unka65 unka65 commented Jun 7, 2020

Fix action Update Time 1h (DB SQL parameter missing).

Fix Enter key to submit/filter a Search value.

Add HTML escaping. Add to remove "<" and ">" from the schedule name.

Add additional filtering to Devices tab.


This includes a fix for an existing SQL error for the action Update Time 1h.

Also, using the Enter key to submit text filter did not work before this change. The schedule ID was cleared. In updating the code from host.php, the Enter key worked.

I attempted to add the HTML escaping to prevent data with HTML from affecting the webpage function. I tested value "<script>" for schedule name (before changing to remove <> from the name), device description, webseer name. I looked for a data cleanup function but function clean_up_name seemed too restrictive.

I noted the added location filtering for Devices (host.php) and thought I would attempt the full filtering here using code from host.php. I have an added appreciation for all of the work you developers have done to make Cacti great.

Each of the filter prompts filter the remaining filter prompts:
Site, Collector, Associated -> filter Location -> filter Template
The location dropdown has Any for all, None for null or empty and handles data values of Any and None as additional entries.

I do not know if the Device filtering is useful or acceptable but I wanted to offer it in case it might help including with the current open issue/request.

There are a lot of changes and so additional testing by someone else would be good.

Fix action Update Time 1h (DB SQL parameter missing).

Fix Enter key to submit/filter a Search value.

Add HTML escaping.  Add to remove "<" and ">" from the schedule name.

Add additional filtering to Devices tab.
Copy link
Member

@netniV netniV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, firstly, let me thank you for your contribution, it's appreciated and I'm sure others will use the functionality without even realising the changes required 👍

However, there are a few things I've highlighted that I think we need to address or discuss before I commit this to the repo. Some are minor points and I can live without changing but some are quite important especially the security related ones.

If you are unsure what we mean, see the recent XSS fixes we have been applying to the core with regards to <script>alert('1');</script> in many fields to show up improper escaping that has crept in even from our own developers.

maint.php Outdated
</td>
<td>
<select id='location'>
<option value='<?php print HOST_FILTER_LOC_ANY ?>' <?php if (get_request_var('location') == HOST_FILTER_LOC_ANY) {?> selected<?php }?>><?php print __('Any');?></option>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than explicitly output Any as a location. Why not add it to the locations array further down so the same piece of code is used for all output. That way, if there are any changes to the URL or display of it, they are all handled at once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That code was misplaced and not working. Thank you. It is added to the SQL; I do not know how to add it to the array.

@netniV netniV self-assigned this Jun 7, 2020
@netniV netniV merged commit d46be82 into Cacti:develop Jun 28, 2021
@netniV
Copy link
Member

netniV commented Jun 28, 2021

Sorry it took so long, completely forgot about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants