Skip to content

Commit c07ed2a

Browse files
authored
[Bug-fix] Translate JSONException to 400 instead of 500 (opensearch-project#3833)
1 parent 774a3a2 commit c07ed2a

4 files changed

Lines changed: 57 additions & 25 deletions

File tree

integ-test/src/test/java/org/opensearch/sql/ppl/PPLPluginIT.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,17 @@ public void sqlEnableSettingsTest() throws IOException {
9696
updateClusterSettings(new ClusterSetting(PERSISTENT, "plugins.ppl.enabled", null));
9797
}
9898

99+
@Test
100+
public void testQueryEndpointShouldFailWithInvalidPayload() throws IOException {
101+
exceptionRule.expect(ResponseException.class);
102+
exceptionRule.expect(hasProperty("response", statusCode(400)));
103+
104+
Request post = new Request("POST", "/_plugins/_ppl");
105+
post.setJsonEntity("{ \"query\": [\"search source=test\"] }");
106+
107+
client().performRequest(post);
108+
}
109+
99110
protected Request makePPLRequest(String query) {
100111
Request post = new Request("POST", "/_plugins/_ppl");
101112
post.setJsonEntity(String.format(Locale.ROOT, "{\n" + " \"query\": \"%s\"\n" + "}", query));

integ-test/src/test/java/org/opensearch/sql/sql/QueryValidationIT.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,27 @@ public void aggregationFunctionInSelectGroupByMultipleFields() throws IOExceptio
8282
"SELECT SUM(age) FROM opensearch-sql_test_index_account GROUP BY state.keyword");
8383
}
8484

85+
@Test
86+
public void testSqlQueryWithArrayPayloadShouldFailWithBadRequest() throws IOException {
87+
expectResponseException()
88+
.hasStatusCode(BAD_REQUEST)
89+
.hasErrorType("IllegalArgumentException")
90+
.containsMessage("Failed to parse request payload");
91+
92+
whenExecuteMalformedPayload();
93+
}
94+
95+
private static void whenExecuteMalformedPayload() throws IOException {
96+
Request request = new Request("POST", QUERY_API_ENDPOINT);
97+
request.setJsonEntity("{ \"query\": [\"SELECT * FROM opensearch-sql_test_index_account\"] }");
98+
99+
RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
100+
restOptionsBuilder.addHeader("Content-Type", "application/json");
101+
request.setOptions(restOptionsBuilder);
102+
103+
client().performRequest(request);
104+
}
105+
85106
public ResponseExceptionAssertion expectResponseException() {
86107
return new ResponseExceptionAssertion(exceptionRule);
87108
}

legacy/src/main/java/org/opensearch/sql/legacy/request/SqlRequestFactory.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,19 @@ private static SqlRequest parseSqlRequestFromPayload(RestRequest restRequest) {
5353
if (jsonContent.has(SQL_CURSOR_FIELD_NAME)) {
5454
return new SqlRequest(jsonContent.getString(SQL_CURSOR_FIELD_NAME));
5555
}
56+
String sql = jsonContent.getString(SQL_FIELD_NAME);
57+
58+
if (jsonContent.has(PARAM_FIELD_NAME)) { // is a PreparedStatement
59+
JSONArray paramArray = jsonContent.getJSONArray(PARAM_FIELD_NAME);
60+
List<PreparedStatementRequest.PreparedStatementParameter> parameters =
61+
parseParameters(paramArray);
62+
return new PreparedStatementRequest(
63+
sql, validateAndGetFetchSize(jsonContent), jsonContent, parameters);
64+
}
65+
return new SqlRequest(sql, validateAndGetFetchSize(jsonContent), jsonContent);
5666
} catch (JSONException e) {
5767
throw new IllegalArgumentException("Failed to parse request payload", e);
5868
}
59-
String sql = jsonContent.getString(SQL_FIELD_NAME);
60-
61-
if (jsonContent.has(PARAM_FIELD_NAME)) { // is a PreparedStatement
62-
JSONArray paramArray = jsonContent.getJSONArray(PARAM_FIELD_NAME);
63-
List<PreparedStatementRequest.PreparedStatementParameter> parameters =
64-
parseParameters(paramArray);
65-
return new PreparedStatementRequest(
66-
sql, validateAndGetFetchSize(jsonContent), jsonContent, parameters);
67-
}
68-
return new SqlRequest(sql, validateAndGetFetchSize(jsonContent), jsonContent);
6969
}
7070

7171
private static Integer validateAndGetFetchSize(JSONObject jsonContent) {

plugin/src/main/java/org/opensearch/sql/plugin/request/PPLQueryRequestFactory.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,24 +59,24 @@ private static PPLQueryRequest parsePPLRequestFromPayload(RestRequest restReques
5959
boolean pretty = getPrettyOption(restRequest.params());
6060
try {
6161
jsonContent = new JSONObject(content);
62+
PPLQueryRequest pplRequest =
63+
new PPLQueryRequest(
64+
jsonContent.getString(PPL_FIELD_NAME),
65+
jsonContent,
66+
restRequest.path(),
67+
format.getFormatName());
68+
// set sanitize option if csv format
69+
if (format.equals(Format.CSV)) {
70+
pplRequest.sanitize(getSanitizeOption(restRequest.params()));
71+
}
72+
// set pretty option
73+
if (pretty) {
74+
pplRequest.style(JsonResponseFormatter.Style.PRETTY);
75+
}
76+
return pplRequest;
6277
} catch (JSONException e) {
6378
throw new IllegalArgumentException("Failed to parse request payload", e);
6479
}
65-
PPLQueryRequest pplRequest =
66-
new PPLQueryRequest(
67-
jsonContent.getString(PPL_FIELD_NAME),
68-
jsonContent,
69-
restRequest.path(),
70-
format.getFormatName());
71-
// set sanitize option if csv format
72-
if (format.equals(Format.CSV)) {
73-
pplRequest.sanitize(getSanitizeOption(restRequest.params()));
74-
}
75-
// set pretty option
76-
if (pretty) {
77-
pplRequest.style(JsonResponseFormatter.Style.PRETTY);
78-
}
79-
return pplRequest;
8080
}
8181

8282
private static Format getFormat(Map<String, String> requestParams, String path) {

0 commit comments

Comments
 (0)