Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not use Marshmallow validation in partial params validation #15236

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/src/pages/docs/Miscellaneous/issue_codes.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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).
1 change: 1 addition & 0 deletions superset-frontend/src/components/ErrorMessage/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
11 changes: 0 additions & 11 deletions superset/databases/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,19 @@ 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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we stringify the json? @hughhhh

Screen Shot 2021-06-18 at 11 18 45 AM

Screen Shot 2021-06-18 at 11 20 24 AM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This number should be an int, let me look on the frontend

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.",
Expand Down
4 changes: 4 additions & 0 deletions superset/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
34 changes: 24 additions & 10 deletions tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).",
}
]
}
Expand Down Expand Up @@ -1909,18 +1909,32 @@ def test_validate_parameters_invalid_port_range(self, is_hostname_valid):
assert response == {
"errors": [
{
"message": "Must be greater than or equal to 0 and less than 65536.",
"error_type": "INVALID_PAYLOAD_SCHEMA_ERROR",
"message": "One or more parameters are missing: database, username",
"error_type": "CONNECTION_MISSING_PARAMETERS_ERROR",
"level": "warning",
"extra": {
"missing": ["database", "username"],
"issue_codes": [
{
"code": 1018,
"message": "Issue 1018 - One or more parameters needed to configure a database are missing.",
}
],
},
},
{
"message": "The port must be an integer between 0 and 65535 (inclusive).",
"error_type": "CONNECTION_INVALID_PORT_ERROR",
"level": "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.",
}
],
},
}
},
]
}