Skip to content

Commit

Permalink
SQL: supplement input checks on received request parameters (#52229) (#…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
bpintea authored Feb 12, 2020
1 parent fc96464 commit 5dfe276
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,20 @@ public void testErrorMessageForTranslatingSQLCommandStatement() throws IOExcepti
containsString("Cannot generate a query DSL for a special SQL command " +
"(e.g.: DESCRIBE, SHOW), sql statement: [SHOW FUNCTIONS]"));
}

public void testErrorMessageForInvalidParamDataType() throws IOException {
expectBadRequest(() -> runTranslateSql(
"{\"query\":\"SELECT null WHERE 0 = ? \", \"mode\": \"odbc\", \"params\":[{\"type\":\"invalid\", \"value\":\"irrelevant\"}]}"
),
containsString("Invalid parameter data type [invalid]")
);
}

public void testErrorMessageForInvalidParamSpec() throws IOException {
expectBadRequest(() -> runTranslateSql(
"{\"query\":\"SELECT null WHERE 0 = ? \", \"mode\": \"odbc\", \"params\":[{\"type\":\"SHAPE\", \"value\":false}]}"
),
containsString("Cannot cast value [false] of type [BOOLEAN] to parameter type [SHAPE]")
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.converterFor;
import static org.elasticsearch.xpack.sql.util.DateUtils.asTimeOnly;
import static org.elasticsearch.xpack.sql.util.DateUtils.dateOfEscapedLiteral;
import static org.elasticsearch.xpack.sql.util.DateUtils.dateTimeOfEscapedLiteral;
Expand Down Expand Up @@ -700,6 +701,9 @@ public Literal visitParamLiteral(ParamLiteralContext ctx) {
SqlTypedParamValue param = param(ctx.PARAM());
DataType dataType = SqlDataTypes.fromTypeName(param.type);
Source source = source(ctx);
if (dataType == null) {
throw new ParsingException(source, "Invalid parameter data type [{}]", param.type);
}
if (param.value == null) {
// no conversion is required for null values
return new Literal(source, null, dataType);
Expand All @@ -717,6 +721,10 @@ public Literal visitParamLiteral(ParamLiteralContext ctx) {
}
// otherwise we need to make sure that xcontent-serialized value is converted to the correct type
try {
if (canConvert(sourceType, dataType) == false) {
throw new ParsingException(source, "Cannot cast value [{}] of type [{}] to parameter type [{}]", param.value, sourceType,
dataType);
}
return new Literal(source, converterFor(sourceType, dataType).convert(param.value), dataType);
} catch (QlIllegalArgumentException ex) {
throw new ParsingException(ex, source, "Unexpected actual parameter type [{}] for type [{}]", sourceType, param.type);
Expand Down

0 comments on commit 5dfe276

Please sign in to comment.