Reject bulk requests with invalid actions (#5302)

* Reject bulk requests with invalid actions The existing bulk api silently ignores bulk item requests that have an invalid action. This change rejects those requests. Signed-off-by: Rabi Panda <[email protected]> * Rename method as per code review comment Signed-off-by: Rabi Panda <[email protected]> * Address review comments Signed-off-by: Rabi Panda <[email protected]> Signed-off-by: Rabi Panda <[email protected]>
opensearch-project · Nov 21, 2022 · 66c5448 · 66c5448
1 parent a43030b
commit 66c5448
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -73,7 +73,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Fixed
 - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
 - Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
-- Add jvm option to allow security manager ([#5194](https://github.com/opensearch-project/OpenSearch/pull/5194))
+- Reject bulk requests with invalid actions ([#5299](https://github.com/opensearch-project/OpenSearch/issues/5299))
+
 ### Security
 
 ## [Unreleased 2.x]

diff --git a/server/src/main/java/org/opensearch/action/bulk/BulkRequestParser.java b/server/src/main/java/org/opensearch/action/bulk/BulkRequestParser.java
@@ -53,6 +53,7 @@
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
@@ -78,6 +79,8 @@ public final class BulkRequestParser {
     private static final ParseField IF_PRIMARY_TERM = new ParseField("if_primary_term");
     private static final ParseField REQUIRE_ALIAS = new ParseField(DocWriteRequest.REQUIRE_ALIAS);
 
+    private static final Set<String> VALID_ACTIONS = Set.of("create", "delete", "index", "update");
+
     private static int findNextMarker(byte marker, int from, BytesReference data) {
         final int res = data.indexOf(marker, from);
         if (res != -1) {
@@ -177,6 +180,15 @@ public void parse(
                     );
                 }
                 String action = parser.currentName();
+                if (action == null || VALID_ACTIONS.contains(action) == false) {
+                    throw new IllegalArgumentException(
+                        "Malformed action/metadata line ["
+                            + line
+                            + "], expected one of [create, delete, index, update] but found ["
+                            + action
+                            + "]"
+                    );
+                }
 
                 String index = defaultIndex;
                 String id = null;

diff --git a/server/src/test/java/org/opensearch/action/bulk/BulkRequestParserTests.java b/server/src/test/java/org/opensearch/action/bulk/BulkRequestParserTests.java
@@ -234,4 +234,30 @@ public void testParseDeduplicatesParameterStrings() throws IOException {
         assertSame(first.getPipeline(), second.getPipeline());
         assertSame(first.routing(), second.routing());
     }
+
+    public void testFailOnUnsupportedAction() {
+        BytesArray request = new BytesArray("{ \"baz\":{ \"_id\": \"bar\" } }\n{}\n");
+        BulkRequestParser parser = new BulkRequestParser();
+
+        IllegalArgumentException ex = expectThrows(
+            IllegalArgumentException.class,
+            () -> parser.parse(
+                request,
+                "foo",
+                null,
+                null,
+                null,
+                true,
+                false,
+                XContentType.JSON,
+                req -> fail(),
+                req -> fail(),
+                req -> fail()
+            )
+        );
+        assertEquals(
+            "Malformed action/metadata line [1], expected one of [create, delete, index, update] but found [baz]",
+            ex.getMessage()
+        );
+    }
 }