Skip to content

Commit eda30ac

Browse files
authored
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.
1 parent 0c7ae02 commit eda30ac

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

x-pack/plugin/sql/qa/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java

+16
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,20 @@ public void testErrorMessageForTranslatingSQLCommandStatement() throws IOExcepti
3939
containsString("Cannot generate a query DSL for a special SQL command " +
4040
"(e.g.: DESCRIBE, SHOW), sql statement: [SHOW FUNCTIONS]"));
4141
}
42+
43+
public void testErrorMessageForInvalidParamDataType() throws IOException {
44+
expectBadRequest(() -> runTranslateSql(
45+
"{\"query\":\"SELECT null WHERE 0 = ? \", \"mode\": \"odbc\", \"params\":[{\"type\":\"invalid\", \"value\":\"irrelevant\"}]}"
46+
),
47+
containsString("Invalid parameter data type [invalid]")
48+
);
49+
}
50+
51+
public void testErrorMessageForInvalidParamSpec() throws IOException {
52+
expectBadRequest(() -> runTranslateSql(
53+
"{\"query\":\"SELECT null WHERE 0 = ? \", \"mode\": \"odbc\", \"params\":[{\"type\":\"SHAPE\", \"value\":false}]}"
54+
),
55+
containsString("Cannot cast value [false] of type [BOOLEAN] to parameter type [SHAPE]")
56+
);
57+
}
4258
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@
129129

130130
import static java.util.Collections.emptyList;
131131
import static java.util.Collections.singletonList;
132-
import static org.elasticsearch.xpack.ql.type.DataTypeConverter.converterFor;
132+
import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.canConvert;
133+
import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.converterFor;
133134
import static org.elasticsearch.xpack.sql.util.DateUtils.asTimeOnly;
134135
import static org.elasticsearch.xpack.sql.util.DateUtils.dateOfEscapedLiteral;
135136
import static org.elasticsearch.xpack.sql.util.DateUtils.dateTimeOfEscapedLiteral;
@@ -700,6 +701,9 @@ public Literal visitParamLiteral(ParamLiteralContext ctx) {
700701
SqlTypedParamValue param = param(ctx.PARAM());
701702
DataType dataType = SqlDataTypes.fromTypeName(param.type);
702703
Source source = source(ctx);
704+
if (dataType == null) {
705+
throw new ParsingException(source, "Invalid parameter data type [{}]", param.type);
706+
}
703707
if (param.value == null) {
704708
// no conversion is required for null values
705709
return new Literal(source, null, dataType);
@@ -717,6 +721,10 @@ public Literal visitParamLiteral(ParamLiteralContext ctx) {
717721
}
718722
// otherwise we need to make sure that xcontent-serialized value is converted to the correct type
719723
try {
724+
if (canConvert(sourceType, dataType) == false) {
725+
throw new ParsingException(source, "Cannot cast value [{}] of type [{}] to parameter type [{}]", param.value, sourceType,
726+
dataType);
727+
}
720728
return new Literal(source, converterFor(sourceType, dataType).convert(param.value), dataType);
721729
} catch (QlIllegalArgumentException ex) {
722730
throw new ParsingException(ex, source, "Unexpected actual parameter type [{}] for type [{}]", sourceType, param.type);

0 commit comments

Comments
 (0)