From c9b61e7d88352ca17a36ec67e7a7a7af36638e58 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 10 Oct 2024 10:46:28 -0700 Subject: [PATCH 1/4] Add reproducer for malformed cursor handling Signed-off-by: Simeon Widdis --- .../test/java/org/opensearch/sql/sql/PaginationIT.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java index fbe1e378e2..383008f959 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java @@ -253,6 +253,16 @@ public void testAlias() throws Exception { assertEquals(aliasFilteredResponse.getInt("size"), 4); } + @Test + public void testMalformedCursorGracefullyHandled() throws IOException { + ResponseException ex = assertThrows( + "Expected query with malformed cursor to raise error, but didn't", + ResponseException.class, + () -> executeCursorQuery("d:a11b4db33f") + ); + assertEquals(ex.getResponse().getStatusLine().getStatusCode(), 400); + } + private String executeFetchQuery(String query, int fetchSize, String requestType, String filter) throws IOException { String endpoint = "/_plugins/_sql?format=" + requestType; From ec239d2761153430839ef4e0049402602edc134b Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 10 Oct 2024 11:58:46 -0700 Subject: [PATCH 2/4] Handle malformed query cursors Signed-off-by: Simeon Widdis --- .../java/org/opensearch/sql/sql/PaginationIT.java | 5 +++-- .../legacy/executor/cursor/CursorResultExecutor.java | 11 ++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java index 383008f959..c971ca50ba 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java @@ -255,12 +255,13 @@ public void testAlias() throws Exception { @Test public void testMalformedCursorGracefullyHandled() throws IOException { - ResponseException ex = assertThrows( + ResponseException result = assertThrows( "Expected query with malformed cursor to raise error, but didn't", ResponseException.class, () -> executeCursorQuery("d:a11b4db33f") ); - assertEquals(ex.getResponse().getStatusLine().getStatusCode(), 400); + assertTrue(result.getMessage().contains("Malformed cursor")); + assertEquals(result.getResponse().getStatusLine().getStatusCode(), 400); } private String executeFetchQuery(String query, int fetchSize, String requestType, String filter) diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java index 0af3ca243b..a5a97d5102 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java @@ -20,6 +20,7 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; import org.opensearch.common.unit.TimeValue; +import org.opensearch.core.rest.RestStatus; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.search.SearchHit; @@ -36,6 +37,7 @@ import org.opensearch.sql.legacy.pit.PointInTimeHandler; import org.opensearch.sql.legacy.pit.PointInTimeHandlerImpl; import org.opensearch.sql.legacy.rewriter.matchtoterm.VerificationException; +import org.opensearch.sql.opensearch.response.error.ErrorMessageFactory; public class CursorResultExecutor implements CursorRestExecutor { @@ -58,7 +60,14 @@ public void execute(Client client, Map params, RestChannel chann } catch (IllegalArgumentException | JSONException e) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_CUS).increment(); LOG.error("Error parsing the cursor", e); - channel.sendResponse(new BytesRestResponse(channel, e)); + channel.sendResponse(new BytesRestResponse( + RestStatus.BAD_REQUEST, + "application/json; charset=UTF-8", + ErrorMessageFactory.createErrorMessage( + new IllegalArgumentException("Malformed cursor: unable to extract cursor information"), + RestStatus.BAD_REQUEST.getStatus() + ).toString() + )); } catch (OpenSearchException e) { int status = (e.status().getStatus()); if (status > 399 && status < 500) { From 30ce0e8f67eda0c2d201892f584fc3b87ab9d0bb Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 10 Oct 2024 12:13:48 -0700 Subject: [PATCH 3/4] Move malformed cursor test to correct file Signed-off-by: Simeon Widdis --- .../test/java/org/opensearch/sql/legacy/CursorIT.java | 11 +++++++++++ .../java/org/opensearch/sql/sql/PaginationIT.java | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java index d0c2f19f42..03a85f4004 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java @@ -440,6 +440,17 @@ public void noPaginationWithNonJDBCFormat() throws IOException { assertThat(rows.length, equalTo(1000)); } + @Test + public void testMalformedCursorGracefullyHandled() throws IOException { + ResponseException result = assertThrows( + "Expected query with malformed cursor to raise error, but didn't", + ResponseException.class, + () -> executeCursorQuery("d:a11b4db33f") + ); + assertTrue(result.getMessage().contains("Malformed cursor")); + assertEquals(result.getResponse().getStatusLine().getStatusCode(), 400); + } + public void verifyWithAndWithoutPaginationResponse( String sqlQuery, String cursorQuery, int fetch_size, boolean shouldFallBackToV1) throws IOException { diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java index c971ca50ba..fbe1e378e2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java @@ -253,17 +253,6 @@ public void testAlias() throws Exception { assertEquals(aliasFilteredResponse.getInt("size"), 4); } - @Test - public void testMalformedCursorGracefullyHandled() throws IOException { - ResponseException result = assertThrows( - "Expected query with malformed cursor to raise error, but didn't", - ResponseException.class, - () -> executeCursorQuery("d:a11b4db33f") - ); - assertTrue(result.getMessage().contains("Malformed cursor")); - assertEquals(result.getResponse().getStatusLine().getStatusCode(), 400); - } - private String executeFetchQuery(String query, int fetchSize, String requestType, String filter) throws IOException { String endpoint = "/_plugins/_sql?format=" + requestType; From 01e2f1e1e40c5c0c23d03b5d99ce77bb49709e64 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 10 Oct 2024 13:04:44 -0700 Subject: [PATCH 4/4] Apply spotless Signed-off-by: Simeon Widdis --- .../test/java/org/opensearch/sql/legacy/CursorIT.java | 6 +++--- .../legacy/executor/cursor/CursorResultExecutor.java | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java index 03a85f4004..2bcb2902a2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java @@ -442,11 +442,11 @@ public void noPaginationWithNonJDBCFormat() throws IOException { @Test public void testMalformedCursorGracefullyHandled() throws IOException { - ResponseException result = assertThrows( + ResponseException result = + assertThrows( "Expected query with malformed cursor to raise error, but didn't", ResponseException.class, - () -> executeCursorQuery("d:a11b4db33f") - ); + () -> executeCursorQuery("d:a11b4db33f")); assertTrue(result.getMessage().contains("Malformed cursor")); assertEquals(result.getResponse().getStatusLine().getStatusCode(), 400); } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java index a5a97d5102..4947d06b2f 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java @@ -60,14 +60,15 @@ public void execute(Client client, Map params, RestChannel chann } catch (IllegalArgumentException | JSONException e) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_CUS).increment(); LOG.error("Error parsing the cursor", e); - channel.sendResponse(new BytesRestResponse( + channel.sendResponse( + new BytesRestResponse( RestStatus.BAD_REQUEST, "application/json; charset=UTF-8", ErrorMessageFactory.createErrorMessage( - new IllegalArgumentException("Malformed cursor: unable to extract cursor information"), - RestStatus.BAD_REQUEST.getStatus() - ).toString() - )); + new IllegalArgumentException( + "Malformed cursor: unable to extract cursor information"), + RestStatus.BAD_REQUEST.getStatus()) + .toString())); } catch (OpenSearchException e) { int status = (e.status().getStatus()); if (status > 399 && status < 500) {