http-api: T5263: simplify form errors

1 files changed, 42 insertions, 66 deletions

diff --git a/src/services/vyos-http-api-server b/src/services/vyos-http-api-server
index 89c685f32..dda137943 100755
--- a/src/services/vyos-http-api-server
+++ b/src/services/vyos-http-api-server
@@ -260,18 +260,15 @@ def auth_required(data: ApiModel):
 # the explicit validation may be dropped, if desired, in favor of native
 # validation by FastAPI/Pydantic, as is used for application/json requests
 class MultipartRequest(Request):
-    ERR_MISSING_KEY = False
-    ERR_MISSING_DATA = False
-    ERR_NOT_JSON = False
-    ERR_NOT_DICT = False
-    ERR_NO_OP = False
-    ERR_NO_PATH = False
-    ERR_EMPTY_PATH = False
-    ERR_PATH_NOT_LIST = False
-    ERR_VALUE_NOT_STRING = False
-    ERR_PATH_NOT_LIST_OF_STR = False
-    offending_command = {}
-    exception = None
+    _form_err = ()
+    @property
+    def form_err(self):
+        return self._form_err
+
+    @form_err.setter
+    def form_err(self, val):
+        if not self._form_err:
+            self._form_err = val
 
     @property
     def orig_headers(self):
@@ -310,19 +307,20 @@ class MultipartRequest(Request):
 
             form_data = await self.form()
             if form_data:
+                endpoint = self.url.path
                 logger.debug("processing form data")
                 for k, v in form_data.multi_items():
                     forms[k] = v
 
                 if 'data' not in forms:
-                    self.ERR_MISSING_DATA = True
+                    self.form_err = (422, "Non-empty data field is required")
+                    return self._body
                 else:
                     try:
                         tmp = json.loads(forms['data'])
                     except json.JSONDecodeError as e:
-                        self.ERR_NOT_JSON = True
-                        self.exception = e
-                        tmp = {}
+                        self.form_err = (400, f'Failed to parse JSON: {e}')
+                        return self._body
                     if isinstance(tmp, list):
                         merge['commands'] = tmp
                     else:
@@ -336,29 +334,33 @@ class MultipartRequest(Request):
 
                 for c in cmds:
                     if not isinstance(c, dict):
-                        self.ERR_NOT_DICT = True
-                        self.offending_command = c
-                    elif 'op' not in c:
-                        self.ERR_NO_OP = True
-                        self.offending_command = c
-                    elif 'path' not in c:
-                        self.ERR_NO_PATH = True
-                        self.offending_command = c
-                    elif not c['path']:
-                        self.ERR_EMPTY_PATH = True
-                        self.offending_command = c
-                    elif not isinstance(c['path'], list):
-                        self.ERR_PATH_NOT_LIST = True
-                        self.offending_command = c
-                    elif not all(isinstance(el, str) for el in c['path']):
-                        self.ERR_PATH_NOT_LIST_OF_STR = True
-                        self.offending_command = c
-                    elif 'value' in c and not isinstance(c['value'], str):
-                        self.ERR_VALUE_NOT_STRING = True
-                        self.offending_command = c
+                        self.form_err = (400,
+                        f"Malformed command '{c}': any command must be JSON of dict")
+                        return self._body
+                    if 'op' not in c:
+                        self.form_err = (400,
+                        f"Malformed command '{c}': missing 'op' field")
+                    if endpoint not in ('/config-file', '/container-image',
+                                        '/image'):
+                        if 'path' not in c:
+                            self.form_err = (400,
+                            f"Malformed command '{c}': missing 'path' field")
+                        elif not isinstance(c['path'], list):
+                            self.form_err = (400,
+                            f"Malformed command '{c}': 'path' field must be a list")
+                        elif not all(isinstance(el, str) for el in c['path']):
+                            self.form_err = (400,
+                            f"Malformed command '{0}': 'path' field must be a list of strings")
+                    if endpoint in ('/configure'):
+                        if not c['path']:
+                            self.form_err = (400,
+                            f"Malformed command '{c}': 'path' list must be non-empty")
+                        if 'value' in c and not isinstance(c['value'], str):
+                            self.form_err = (400,
+                            f"Malformed command '{c}': 'value' field must be a string")
 
                 if 'key' not in forms and 'key' not in merge:
-                    self.ERR_MISSING_KEY = True
+                    self.form_err = (401, "Valid API key is required")
                 if 'key' in forms and 'key' not in merge:
                     merge['key'] = forms['key']
 
@@ -374,40 +376,14 @@ class MultipartRoute(APIRoute):
 
         async def custom_route_handler(request: Request) -> Response:
             request = MultipartRequest(request.scope, request.receive)
-            endpoint = request.url.path
             try:
                 response: Response = await original_route_handler(request)
             except HTTPException as e:
                 return error(e.status_code, e.detail)
             except Exception as e:
-                if request.ERR_MISSING_KEY:
-                    return error(401, "Valid API key is required")
-                if request.ERR_MISSING_DATA:
-                    return error(422, "Non-empty data field is required")
-                if request.ERR_NOT_JSON:
-                    return error(400, "Failed to parse JSON: {0}".format(request.exception))
-                if endpoint == '/configure':
-                    if request.ERR_NOT_DICT:
-                        return error(400, "Malformed command \"{0}\": any command must be a dict".format(json.dumps(request.offending_command)))
-                    if request.ERR_NO_OP:
-                        return error(400, "Malformed command \"{0}\": missing \"op\" field".format(json.dumps(request.offending_command)))
-                    if request.ERR_NO_PATH:
-                        return error(400, "Malformed command \"{0}\": missing \"path\" field".format(json.dumps(request.offending_command)))
-                    if request.ERR_EMPTY_PATH:
-                        return error(400, "Malformed command \"{0}\": empty path".format(json.dumps(request.offending_command)))
-                    if request.ERR_PATH_NOT_LIST:
-                        return error(400, "Malformed command \"{0}\": \"path\" field must be a list".format(json.dumps(request.offending_command)))
-                    if request.ERR_VALUE_NOT_STRING:
-                        return error(400, "Malformed command \"{0}\": \"value\" field must be a string".format(json.dumps(request.offending_command)))
-                    if request.ERR_PATH_NOT_LIST_OF_STR:
-                        return error(400, "Malformed command \"{0}\": \"path\" field must be a list of strings".format(json.dumps(request.offending_command)))
-                if endpoint in ('/retrieve','/generate','/show','/reset'):
-                    if request.ERR_NO_OP or request.ERR_NO_PATH:
-                        return error(400, "Missing required field. \"op\" and \"path\" fields are required")
-                if endpoint in ('/config-file', '/image', '/container-image'):
-                    if request.ERR_NO_OP:
-                        return error(400, "Missing required field \"op\"")
-
+                form_err = request.form_err
+                if form_err:
+                    return error(*form_err)
                 raise e
 
             return response