From 66c544821f019a30de47f38686e0ce5a3a08cfca Mon Sep 17 00:00:00 2001 From: Rabi Panda Date: Mon, 21 Nov 2022 12:15:08 -0800 Subject: [PATCH] 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 * Rename method as per code review comment Signed-off-by: Rabi Panda * Address review comments Signed-off-by: Rabi Panda Signed-off-by: Rabi Panda --- CHANGELOG.md | 3 ++- .../action/bulk/BulkRequestParser.java | 12 +++++++++ .../action/bulk/BulkRequestParserTests.java | 26 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 295ed68636f3a..6267afb55ee2c 100644 --- 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 index 212450515b57e..af0408453e652 100644 --- 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 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 index d3da77112408b..32a0b3723f7ae 100644 --- 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() + ); + } }