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.

Sign up for GitHub

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

Jump to bottom

Do not expose table related exception if feature is not enabled. #1469

Open
Yuqi-Du opened this issue Sep 27, 2024 · 2 comments · May be fixed by #1470
Open

Do not expose table related exception if feature is not enabled. #1469

Yuqi-Du opened this issue Sep 27, 2024 · 2 comments · May be fixed by #1470
Assignees
@tatu-at-datastax

Comments

@Yuqi-Du
Copy link
Contributor

Yuqi-Du commented Sep 27, 2024 •
edited
Loading

So if the column validation failed before the feature flag check, it won’t return error like TABLE_FEATURE_NOT_ENABLED, instead it will return table related stuff. We should do feature checks before these validations happen.

For example, with Feature-Flag-tables set as False

{
    "createTable": {
        "name": "allcolumns",
        "definition": {
            "columns": {
                "id": {
                    "type": "text"
                },
                "age": {
                    "type": "int"
                },
                "image_data": {
                    "type": "blob"
                }

            },
            "primaryKey": "id"
        }
    }
}


{
    "errors": [
        {
            "message": "Unsupported column types: Invalid column type: blob",
            "errorCode": "TABLE_COLUMN_TYPE_UNSUPPORTED"
        }
    ]
}
@tatu-at-datastax tatu-at-datastax self-assigned this Sep 27, 2024
@tatu-at-datastax
Copy link
Contributor

I can reproduce this, but the problem unfortunately seems to difficult to resolve. The problem is that this "validation" is part of JSON deserialization of CreateTableCommand by Quarkus (using Jackson) which converts from column type String into ColumnDataType via ColumnType. And lookup for ColumnType throws the exception.

ApiFeature checking can only be done when in KeyspaceResource.postCommand(), but to get there, KeyspaceCommand must have been deserialized; doing which throws exception.

Not sure if or how this could be resolved: will create a PR simply reproducing the issue first.

tatu-at-datastax added a commit that referenced this issue Sep 27, 2024
@tatu-at-datastax
Add failing test to show #1469
260d568
@tatu-at-datastax tatu-at-datastax linked a pull request Sep 27, 2024 that will close this issue
Draft
4 tasks
@tatu-at-datastax
Copy link
Contributor

I have a PR for fix (#1470) but not 100% sure if we want to pursue that approach; to be discussed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tatu-at-datastax tatu-at-datastax

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@tatu-at-datastax @Yuqi-Du