[2.0] Fix VT Filter. #165
[2.0] Fix VT Filter. #165
Conversation
Codecov Report
@@ Coverage Diff @@
## ospd-2.0 #165 +/- ##
============================================
- Coverage 68.5% 68.48% -0.03%
============================================
Files 12 12
Lines 1759 1761 +2
============================================
+ Hits 1205 1206 +1
- Misses 554 555 +1
Continue to review full report at Codecov.
|
ospd/vtfilter.py
Outdated
| @@ -128,4 +128,4 @@ def get_filtered_vts_list(self, vts, vt_filter): | |||
| else: | |||
| _vts_aux.pop(vt_id) | |||
|
|
|||
| return _vts_aux | |||
| return _vts_aux if len(_vts_aux) else -1 | |||
There was a problem hiding this comment.
I had a look at the return value docstring and it says "Dictionary with filtered vulnerability tests.". In that case it would be better to return None here to indicate an error/special case. Also the docstring should be updated to reflect this change.
| return _vts_aux if len(_vts_aux) else -1 | |
| return _vts_aux if len(_vts_aux) else None |
There was a problem hiding this comment.
If you don't use a filter, the default filter is None. Then, instead of getting the complete VTs set , you get none.
We need to differentiate the "no filter" with the "no matches". A no matches returned an empty dictionary, and this was the initial problem, so when the filter does not have matches, it returned the complete set.
There was a problem hiding this comment.
Ok the None case when no filter isn't mentioned in the docs too. Therefore I did oversee it in the function code. But I still don't understand why you want to return -1 when the dict is empty instead of checking at the place where this is used for an empty dict as in my suggestion here https://github.com/greenbone/ospd/pull/165/files/05918e21c73c861ea88c3064ff0e6d3a555e2130#r339015657
ospd/ospd.py
Outdated
| @@ -1542,6 +1542,8 @@ def get_vts_xml(self, vt_id=None, filtered_vts=None): | |||
| """ | |||
|
|
|||
| vts_xml = Element('vts') | |||
| if filtered_vts == -1: | |||
There was a problem hiding this comment.
| if filtered_vts == -1: | |
| if filtered_vts is None: |
There was a problem hiding this comment.
Oh just recognized, wouldn't it be possible to just do a
| if filtered_vts == -1: | |
| if not filtered_vts: |
here and even drop the get_filtered_vts_list change?
jjnicola
force-pushed
the
fix-filter-2
branch
from
3ff97df
to
0363870
Compare
ospd/ospd.py
Outdated
| @@ -994,6 +994,11 @@ def handle_get_scans_command(self, scan_et): | |||
|
|
|||
| def handle_get_vts_command(self, vt_et): | |||
| """ Handles <get_vts> command. | |||
| The <get_vts> element accept two optional arguments. | |||
| vt_id argument receives a single vt id. | |||
| filter argument receives a filter select a sub set of vts. | |||
There was a problem hiding this comment.
| filter argument receives a filter select a sub set of vts. | |
| filter argument receives a filter selecting a sub set of vts. |
ospd/ospd.py
Outdated
| If no vt_id is specified, the collection will contain all vts or those | ||
| passed in filtered_vts. | ||
| If no vt_id is specified or filtered_vts is None (default), the | ||
| collection will contain all vts. Otherwise return those vts passed |
There was a problem hiding this comment.
| collection will contain all vts. Otherwise return those vts passed | |
| collection will contain all vts. Otherwise those vts passed |
ospd/ospd.py
Outdated
| passed in filtered_vts. | ||
| If no vt_id is specified or filtered_vts is None (default), the | ||
| collection will contain all vts. Otherwise return those vts passed | ||
| in filtered_vts or vt_id. In case of both vt_id and filtered_vts are |
There was a problem hiding this comment.
| in filtered_vts or vt_id. In case of both vt_id and filtered_vts are | |
| in filtered_vts or vt_id are returned. In case of both vt_id and filtered_vts are |
If no vts matches the filter, returns an empty element.
jjnicola
force-pushed
the
fix-filter-2
branch
from
0363870
to
fccd732
Compare
If no vts matches the filter, returns an empty element.