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

SQL: supplement input checks on received request parameters #52229

Merged
merged 3 commits into from
Feb 12, 2020

Conversation

bpintea
Copy link
Contributor

@bpintea bpintea commented Feb 11, 2020

This PR adds two necessary verifications on received parameters:

  • it checks the validity of the parameter's data type: if the declared
    data type is resolved to an ES or Java type;
  • it checks if the returned converter is non-null (i.e. a conversion is
    possible) and generates an appropriate exception otherwise.

This commit adds two necessary verifications on received parameters:
- it checks the validity of the parameter's data type: if the declared
data type is resolved to an ES or Java type;
- it checks if the returned converter is non-null (i.e. a conversion is
possible) and generates an appropriate exception otherwise.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/SQL)

@bpintea bpintea requested review from costin, matriv and astefan February 11, 2020 19:17
Remove unused "import" resulted from incorrect master merging into
current branch.
Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -129,7 +129,8 @@

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.elasticsearch.xpack.ql.type.DataTypeConverter.converterFor;
import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.canConvert;
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@bpintea bpintea merged commit eda30ac into elastic:master Feb 12, 2020
@bpintea bpintea deleted the fix/sql_converter_for_literals branch February 12, 2020 16:35
bpintea added a commit to bpintea/elasticsearch that referenced this pull request Feb 12, 2020
…52229)

* Add more checks around parameter conversions

This commit adds two necessary verifications on received parameters:
- it checks the validity of the parameter's data type: if the declared
data type is resolved to an ES or Java type;
- it checks if the returned converter is non-null (i.e. a conversion is
possible) and generates an appropriate exception otherwise.

(cherry picked from commit eda30ac)
bpintea added a commit that referenced this pull request Feb 12, 2020
…52277)

* Add more checks around parameter conversions

This commit adds two necessary verifications on received parameters:
- it checks the validity of the parameter's data type: if the declared
data type is resolved to an ES or Java type;
- it checks if the returned converter is non-null (i.e. a conversion is
possible) and generates an appropriate exception otherwise.

(cherry picked from commit eda30ac)
bpintea added a commit that referenced this pull request Feb 12, 2020
…52276)

* SQL: supplement input checks on received request parameters (#52229)

* Add more checks around parameter conversions

This commit adds two necessary verifications on received parameters:
- it checks the validity of the parameter's data type: if the declared
data type is resolved to an ES or Java type;
- it checks if the returned converter is non-null (i.e. a conversion is
possible) and generates an appropriate exception otherwise.

(cherry picked from commit eda30ac)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants