Fix #200 - Clarify error messages for queries with missing parameter

irrd/server/whois/query_parser.py

@@ -110,10 +110,17 @@ def handle_irrd_command(self, full_command: str) -> WhoisQueryResponse:
         response_type = WhoisQueryResponseType.SUCCESS
         result = None
 
+        # A is not tested here because it is already handled in handle_irrd_routes_for_as_set
+        queries_with_parameter = list('TG6IJMNORS')
+        if command in queries_with_parameter and not parameter:
+            raise WhoisQueryParserException(f'Missing parameter for {command} query')
+
         if command == '!':
             self.multiple_command_mode = True
             result = None
             response_type = WhoisQueryResponseType.NO_RESPONSE
+        elif command == 'V':
+            result = self.handle_irrd_version()
         elif command == 'T':
             self.handle_irrd_timeout_update(parameter)
         elif command == 'G':
@@ -150,8 +157,6 @@ def handle_irrd_command(self, full_command: str) -> WhoisQueryResponse:
                 response_type = WhoisQueryResponseType.KEY_NOT_FOUND
         elif command == 'S':
             result = self.handle_irrd_sources_list(parameter)
-        elif command == 'V':
-            result = self.handle_irrd_version()
         else:
             raise WhoisQueryParserException(f'Unrecognised command: {command}')
 
@@ -211,6 +216,9 @@ def handle_irrd_routes_for_as_set(self, set_name: str) -> str:
         else:
             object_classes = ['route', 'route6']
 
+        if not set_name:
+            raise WhoisQueryParserException(f'Missing required set name for A query')
+
         self._current_set_root_object_class = 'as-set'
 
         members = self._recursive_set_resolve({set_name})

irrd/server/whois/tests/test_query_parser.py

@@ -447,6 +447,18 @@ def test_invalid_command(self, prepare_parser):
 
         assert not mock_dq.mock_calls
 
+    def test_parameter_required(self, prepare_parser):
+        mock_dq, mock_dh, parser = prepare_parser
+
+        queries_with_parameter = list('TG6IJMNORS')
+        for query in queries_with_parameter:
+            response = parser.handle_query(f'!{query}')
+            assert response.response_type == WhoisQueryResponseType.ERROR
+            assert response.mode == WhoisQueryResponseMode.IRRD
+            assert response.result == f'Missing parameter for {query} query'
+
+        assert not mock_dq.mock_calls
+
     def test_multiple_command_mode(self, prepare_parser):
         mock_dq, mock_dh, parser = prepare_parser
 
@@ -530,6 +542,14 @@ def test_routes_for_origin_invalid(self, prepare_parser):
     def test_handle_irrd_routes_for_as_set(self, prepare_parser, monkeypatch):
         mock_dq, mock_dh, parser = prepare_parser
 
+        for parameter in ['', '4', '6']:
+            response = parser.handle_query(f'!a{parameter}')
+            assert response.response_type == WhoisQueryResponseType.ERROR
+            assert response.mode == WhoisQueryResponseMode.IRRD
+            assert response.result == 'Missing required set name for A query'
+
+        assert not mock_dq.mock_calls
+
         monkeypatch.setattr(
             'irrd.server.whois.query_parser.WhoisQueryParser._recursive_set_resolve',
             lambda self, set_name: {'AS65547', 'AS65548'}
@@ -947,11 +967,6 @@ def test_sources_list(self, prepare_parser):
         assert response.mode == WhoisQueryResponseMode.IRRD
         assert response.result == 'TEST1,TEST2'
 
-        response = parser.handle_query('!s')
-        assert response.response_type == WhoisQueryResponseType.ERROR
-        assert response.mode == WhoisQueryResponseMode.IRRD
-        assert response.result == 'One or more selected sources are unavailable.'
-
         response = parser.handle_query('!sTEST3')
         assert response.response_type == WhoisQueryResponseType.ERROR
         assert response.mode == WhoisQueryResponseMode.IRRD