fix: do not use Marshmallow validation in partial params validation

apache · Jun 17, 2021 · 7b370b1 · 7b370b1
1 parent 243af7f
commit 7b370b1
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 17 deletions.
diff --git a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx
@@ -207,3 +207,11 @@ The submitted payload has the incorrect schema.
 ```
 
 Please check that the request payload has the expected schema.
+
+## Issue 1021
+
+```
+The database port provided is invalid.
+```
+
+Please check that the provided database port is an integer between 0 and 65535 (inclusive).
diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts
@@ -33,6 +33,7 @@ export const ErrorTypeEnum = {
   CONNECTION_INVALID_PASSWORD_ERROR: 'CONNECTION_INVALID_PASSWORD_ERROR',
   CONNECTION_INVALID_HOSTNAME_ERROR: 'CONNECTION_INVALID_HOSTNAME_ERROR',
   CONNECTION_PORT_CLOSED_ERROR: 'CONNECTION_PORT_CLOSED_ERROR',
+  CONNECTION_INVALID_PORT_ERROR: 'CONNECTION_INVALID_PORT_ERROR',
   CONNECTION_HOST_DOWN_ERROR: 'CONNECTION_HOST_DOWN_ERROR',
   CONNECTION_ACCESS_DENIED_ERROR: 'CONNECTION_ACCESS_DENIED_ERROR',
   CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR',

diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
@@ -346,17 +346,6 @@ class Meta:  # pylint: disable=too-few-public-methods
         description=configuration_method_description,
     )
 
-    @validates_schema
-    def validate_parameters(  # pylint: disable=no-self-use
-        self, data: Dict[str, Any], **kwargs: Any  # pylint: disable=unused-argument
-    ) -> None:
-        """
-        Validate the DB engine spec specific parameters schema.
-        """
-        # TODO (aafghahi): Move this onCreate instead of validation
-        engine_spec = get_engine_spec(data.get("engine") or data.get("backend"))
-        engine_spec.parameters_schema.load(data["parameters"])  # type: ignore
-
 
 class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
     class Meta:  # pylint: disable=too-few-public-methods

diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
@@ -1445,7 +1445,16 @@ def validate_parameters(
         port = parameters.get("port", None)
         if not port:
             return errors
-        if not is_port_open(host, port):
+        if not (isinstance(port, int) and 0 <= port < 2 ** 16):
+            errors.append(
+                SupersetError(
+                    message="The port must be an integer between 0 and 65535 (inclusive).",
+                    error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR,
+                    level=ErrorLevel.ERROR,
+                    extra={"invalid": ["port"]},
+                ),
+            )
+        elif not is_port_open(host, port):
             errors.append(
                 SupersetError(
                     message="The port is closed.",

diff --git a/superset/errors.py b/superset/errors.py
@@ -44,6 +44,7 @@ class SupersetErrorType(str, Enum):
     CONNECTION_INVALID_PASSWORD_ERROR = "CONNECTION_INVALID_PASSWORD_ERROR"
     CONNECTION_INVALID_HOSTNAME_ERROR = "CONNECTION_INVALID_HOSTNAME_ERROR"
     CONNECTION_PORT_CLOSED_ERROR = "CONNECTION_PORT_CLOSED_ERROR"
+    CONNECTION_INVALID_PORT_ERROR = "CONNECTION_INVALID_PORT_ERROR"
     CONNECTION_HOST_DOWN_ERROR = "CONNECTION_HOST_DOWN_ERROR"
     CONNECTION_ACCESS_DENIED_ERROR = "CONNECTION_ACCESS_DENIED_ERROR"
     CONNECTION_UNKNOWN_DATABASE_ERROR = "CONNECTION_UNKNOWN_DATABASE_ERROR"
@@ -156,6 +157,9 @@ class SupersetErrorType(str, Enum):
     SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR: [
         {"code": 1008, "message": _("Issue 1008 - The port is closed.")},
     ],
+    SupersetErrorType.CONNECTION_INVALID_PORT_ERROR: [
+        {"code": 1021, "message": _("Issue 1021 - Port number is invalid.")},
+    ],
     SupersetErrorType.CONNECTION_HOST_DOWN_ERROR: [
         {
             "code": 1009,

diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py
@@ -1807,18 +1807,18 @@ def test_validate_parameters_invalid_port(self):
         assert response == {
             "errors": [
                 {
-                    "message": "Not a valid integer.",
-                    "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR",
-                    "level": "error",
+                    "error_type": "CONNECTION_INVALID_PORT_ERROR",
                     "extra": {
                         "invalid": ["port"],
                         "issue_codes": [
                             {
-                                "code": 1020,
-                                "message": "Issue 1020 - The submitted payload has the incorrect schema.",
+                                "code": 1021,
+                                "message": "Issue 1021 - Port number is invalid.",
                             }
                         ],
                     },
+                    "level": "error",
+                    "message": "The port must be an integer between 0 and 65535 (inclusive).",
                 }
             ]
         }