Merge pull request #270 from jjnicola/get-scans-id

Make scan_id attribute mandatory for get_scans
greenbone · May 13, 2020 · e37a45c · e37a45c
2 parents 4bfffd0 + 1cde59f
commit e37a45c
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   directly be importable in the virtual python environment. [#252](https://github.com/greenbone/ospd/pull/252)
 - Progress bar calculation does not take in account the dead hosts. [#266](https://github.com/greenbone/ospd/pull/266)
 - Show progress as integer for get_scans. [#269](https://github.com/greenbone/ospd/pull/269)
+- Make scan_id attribute mandatory for get_scans. [#270](https://github.com/greenbone/ospd/pull/270)
 
 ### Fixed
 - Fix stop scan. Wait for the scan process to be stopped before delete it from the process table. [#204](https://github.com/greenbone/ospd/pull/204)

diff --git a/ospd/command/command.py b/ospd/command/command.py
@@ -409,6 +409,9 @@ def handle_xml(self, xml: Element) -> bytes:
         """
 
         scan_id = xml.get('scan_id')
+        if scan_id is None or scan_id == '':
+            raise OspdCommandError('No scan_id attribute', 'get_scans')
+
         details = xml.get('details')
         pop_res = xml.get('pop_results')
         max_res = xml.get('max_results')
@@ -426,22 +429,15 @@ def handle_xml(self, xml: Element) -> bytes:
         progress = progress and progress == '1'
 
         responses = []
-        if scan_id and scan_id in self._daemon.scan_collection.ids_iterator():
+        if scan_id in self._daemon.scan_collection.ids_iterator():
             self._daemon.check_scan_process(scan_id)
             scan = self._daemon.get_scan_xml(
                 scan_id, details, pop_res, max_res, progress
             )
             responses.append(scan)
-        elif scan_id:
+        else:
             text = "Failed to find scan '{0}'".format(scan_id)
             return simple_response_str('get_scans', 404, text)
-        else:
-            for scan_id in self._daemon.scan_collection.ids_iterator():
-                self._daemon.check_scan_process(scan_id)
-                scan = self._daemon.get_scan_xml(
-                    scan_id, details, pop_res, max_res, progress
-                )
-                responses.append(scan)
 
         return simple_response_str('get_scans', 200, 'OK', responses)
 

diff --git a/tests/test_scan_and_result.py b/tests/test_scan_and_result.py
@@ -645,7 +645,10 @@ def test_get_scan_pop(self):
         )
 
         fs = FakeStream()
-        daemon.handle_command('<get_scans details="0" pop_results="1"/>', fs)
+        daemon.handle_command(
+            '<get_scans scan_id="%s" details="0" pop_results="1"/>' % scan_id,
+            fs,
+        )
         response = fs.get_response()
 
         self.assertEqual(response.findtext('scan/results/result'), None)
@@ -877,6 +880,29 @@ def test_calculate_progress_without_current_hosts(self):
         progress = daemon.get_scan_progress(scan_id)
         self.assertEqual(progress, 33)
 
+    def test_get_scan_without_scanid(self):
+        daemon = DummyWrapper([])
+
+        fs = FakeStream()
+        daemon.handle_command(
+            '<start_scan parallel="2">'
+            '<scanner_params />'
+            '<targets><target>'
+            '<hosts>localhost1, localhost2, localhost3, localhost4</hosts>'
+            '<ports>22</ports>'
+            '</target></targets>'
+            '</start_scan>',
+            fs,
+        )
+
+        fs = FakeStream()
+        self.assertRaises(
+            OspdCommandError,
+            daemon.handle_command,
+            '<get_scans details="0" progress="1"/>',
+            fs,
+        )
+
     def test_get_scan_progress_xml(self):
         daemon = DummyWrapper([])
 
@@ -903,7 +929,7 @@ def test_get_scan_progress_xml(self):
 
         fs = FakeStream()
         daemon.handle_command(
-            '<get_scans details="0" progress="1"/>', fs,
+            '<get_scans scan_id="%s" details="0" progress="1"/>' % scan_id, fs,
         )
         response = fs.get_response()
 
@@ -1012,7 +1038,9 @@ def test_result_order(self):
         hosts = ['a', 'c', 'b']
 
         fs = FakeStream()
-        daemon.handle_command('<get_scans details="1"/>', fs)
+        daemon.handle_command(
+            '<get_scans scan_id="%s" details="1"/>' % scan_id, fs
+        )
         response = fs.get_response()
 
         results = response.findall("scan/results/")
@@ -1047,7 +1075,9 @@ def test_batch_result(self):
         hosts = ['a', 'c', 'b']
 
         fs = FakeStream()
-        daemon.handle_command('<get_scans details="1"/>', fs)
+        daemon.handle_command(
+            '<get_scans scan_id="%s" details="1"/>' % scan_id, fs
+        )
         response = fs.get_response()
 
         results = response.findall("scan/results/")