add validation on number of characters in query context path (1024)

Signed-off-by: HenryL27 <[email protected]>
opensearch-project · Jan 29, 2024 · 66d2efd · 66d2efd
1 parent 9a6fe41
commit 66d2efd
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 5 deletions.
diff --git a/.../java/org/opensearch/neuralsearch/processor/rerank/context/QueryContextSourceFetcher.java b/.../java/org/opensearch/neuralsearch/processor/rerank/context/QueryContextSourceFetcher.java
@@ -38,6 +38,8 @@ public class QueryContextSourceFetcher implements ContextSourceFetcher {
     public static final String QUERY_TEXT_FIELD = "query_text";
     public static final String QUERY_TEXT_PATH_FIELD = "query_text_path";
 
+    public static final Integer MAX_QUERY_PATH_STRLEN = 1024;
+
     private final Environment environment;
 
     @Override
@@ -76,9 +78,10 @@ public void fetchContext(
                     throw new IllegalArgumentException(
                         String.format(
                             Locale.ROOT,
-                            "%s exceeded the maximum path length of %d",
+                            "%s exceeded the maximum path length of %d nested fields or %d characters",
                             QUERY_TEXT_PATH_FIELD,
-                            MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(environment.settings())
+                            MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(environment.settings()),
+                            MAX_QUERY_PATH_STRLEN
                         )
                     );
                 }
@@ -129,6 +132,9 @@ private boolean validatePath(final String path) {
         if (path == null || path.isEmpty()) {
             return true;
         }
+        if (path.length() > MAX_QUERY_PATH_STRLEN) {
+            return false;
+        }
         return path.split("\\.").length <= MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(environment.settings());
     }
 }
diff --git a/...t/java/org/opensearch/neuralsearch/processor/rerank/MLOpenSearchRerankProcessorTests.java b/...t/java/org/opensearch/neuralsearch/processor/rerank/MLOpenSearchRerankProcessorTests.java
@@ -230,7 +230,30 @@ public void testRerankContext_whenQueryTextPathIsBadPointer_thenFail() throws IO
             .equals(QueryContextSourceFetcher.QUERY_TEXT_PATH_FIELD + " must point to a string field"));
     }
 
-    public void testRerankContext_whenQueryTextPathIsExceeedinglyLong_thenFail() throws IOException {
+    public void testRerankContext_whenQueryTextPathIsExceeedinglyManyCharacters_thenFail() throws IOException {
+        // "eighteencharacters" * 60 = 1080 character string > max len of 1024
+        setupParams(Map.of(QueryContextSourceFetcher.QUERY_TEXT_PATH_FIELD, "eighteencharacters".repeat(60)));
+        setupSearchResults();
+        @SuppressWarnings("unchecked")
+        ActionListener<Map<String, Object>> listener = mock(ActionListener.class);
+        processor.generateRerankingContext(request, response, listener);
+        ArgumentCaptor<Exception> argCaptor = ArgumentCaptor.forClass(Exception.class);
+        verify(listener, times(1)).onFailure(argCaptor.capture());
+        assert (argCaptor.getValue() instanceof IllegalArgumentException);
+        assert (argCaptor.getValue()
+            .getMessage()
+            .equals(
+                String.format(
+                    Locale.ROOT,
+                    "%s exceeded the maximum path length of %d nested fields or %d characters",
+                    QueryContextSourceFetcher.QUERY_TEXT_PATH_FIELD,
+                    MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(environment.settings()),
+                    QueryContextSourceFetcher.MAX_QUERY_PATH_STRLEN
+                )
+            ));
+    }
+
+    public void textRerankContext_whenQueryTextPathIsExceeedinglyDeeplyNested_ThenFail() throws IOException {
         setupParams(Map.of(QueryContextSourceFetcher.QUERY_TEXT_PATH_FIELD, "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.w.x.y.z"));
         setupSearchResults();
         @SuppressWarnings("unchecked")
@@ -244,9 +267,10 @@ public void testRerankContext_whenQueryTextPathIsExceeedinglyLong_thenFail() thr
             .equals(
                 String.format(
                     Locale.ROOT,
-                    "%s exceeded the maximum path length of %d",
+                    "%s exceeded the maximum path length of %d nested fields or %d characters",
                     QueryContextSourceFetcher.QUERY_TEXT_PATH_FIELD,
-                    MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(environment.settings())
+                    MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(environment.settings()),
+                    QueryContextSourceFetcher.MAX_QUERY_PATH_STRLEN
                 )
             ));
     }