From 7477fd8e0a1cf46e31ac2b90e094720fe3d92598 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 10 Oct 2024 14:09:38 -0700 Subject: [PATCH] Improve error handling for malformed query cursors (#3066) * Add reproducer for malformed cursor handling Signed-off-by: Simeon Widdis * Handle malformed query cursors Signed-off-by: Simeon Widdis * Move malformed cursor test to correct file Signed-off-by: Simeon Widdis * Apply spotless Signed-off-by: Simeon Widdis --------- Signed-off-by: Simeon Widdis --- .../java/org/opensearch/sql/legacy/CursorIT.java | 11 +++++++++++ .../legacy/executor/cursor/CursorResultExecutor.java | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) 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..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 @@ -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/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..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 @@ -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,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, 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) {