From 3dbb4cb0c5a091d92e32f49595344ffd87e021c7 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Tue, 9 Jan 2024 15:36:38 -0800 Subject: [PATCH] Percent-encode invalid flint index characters (#215) (#216) * percent-encode invalid flint index characters Signed-off-by: Sean Kao * formatting Signed-off-by: Sean Kao * move encoding logic to FlintOpenSearchClient Signed-off-by: Sean Kao * minor arg fix and string format fix Signed-off-by: Sean Kao * fix import order Signed-off-by: Sean Kao --------- Signed-off-by: Sean Kao (cherry picked from commit 3528b6660f7f1ddb2fbb9d0af8a0748e545424e3) --- .../core/storage/FlintOpenSearchClient.java | 49 ++++++++++++++++--- .../core/FlintOpenSearchClientSuite.scala | 24 +++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java b/flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java index 22badfbf9..4e549df2b 100644 --- a/flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java +++ b/flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -71,6 +72,13 @@ public class FlintOpenSearchClient implements FlintClient { new NamedXContentRegistry(new SearchModule(Settings.builder().build(), new ArrayList<>()).getNamedXContents()); + /** + * Invalid index name characters to percent-encode, + * excluding '*' because it's reserved for pattern matching. + */ + private final static Set INVALID_INDEX_NAME_CHARS = + Set.of(' ', ',', ':', '"', '+', '/', '\\', '|', '?', '#', '>', '<'); + /** * Metadata log index name prefix */ @@ -121,7 +129,7 @@ public void createIndex(String indexName, FlintMetadata metadata) { protected void createIndex(String indexName, String mapping, Option settings) { LOG.info("Creating Flint index " + indexName); - String osIndexName = toLowercase(indexName); + String osIndexName = sanitizeIndexName(indexName); try (RestHighLevelClient client = createClient()) { CreateIndexRequest request = new CreateIndexRequest(osIndexName); request.mapping(mapping, XContentType.JSON); @@ -137,7 +145,7 @@ protected void createIndex(String indexName, String mapping, Option sett @Override public boolean exists(String indexName) { LOG.info("Checking if Flint index exists " + indexName); - String osIndexName = toLowercase(indexName); + String osIndexName = sanitizeIndexName(indexName); try (RestHighLevelClient client = createClient()) { return client.indices().exists(new GetIndexRequest(osIndexName), RequestOptions.DEFAULT); } catch (IOException e) { @@ -148,7 +156,7 @@ public boolean exists(String indexName) { @Override public List getAllIndexMetadata(String indexNamePattern) { LOG.info("Fetching all Flint index metadata for pattern " + indexNamePattern); - String osIndexNamePattern = toLowercase(indexNamePattern); + String osIndexNamePattern = sanitizeIndexName(indexNamePattern); try (RestHighLevelClient client = createClient()) { GetIndexRequest request = new GetIndexRequest(osIndexNamePattern); GetIndexResponse response = client.indices().get(request, RequestOptions.DEFAULT); @@ -166,7 +174,7 @@ public List getAllIndexMetadata(String indexNamePattern) { @Override public FlintMetadata getIndexMetadata(String indexName) { LOG.info("Fetching Flint index metadata for " + indexName); - String osIndexName = toLowercase(indexName); + String osIndexName = sanitizeIndexName(indexName); try (RestHighLevelClient client = createClient()) { GetIndexRequest request = new GetIndexRequest(osIndexName); GetIndexResponse response = client.indices().get(request, RequestOptions.DEFAULT); @@ -182,7 +190,7 @@ public FlintMetadata getIndexMetadata(String indexName) { @Override public void deleteIndex(String indexName) { LOG.info("Deleting Flint index " + indexName); - String osIndexName = toLowercase(indexName); + String osIndexName = sanitizeIndexName(indexName); try (RestHighLevelClient client = createClient()) { DeleteIndexRequest request = new DeleteIndexRequest(osIndexName); @@ -211,7 +219,7 @@ public FlintReader createReader(String indexName, String query) { queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(parser); } return new OpenSearchScrollReader(createClient(), - toLowercase(indexName), + sanitizeIndexName(indexName), new SearchSourceBuilder().query(queryBuilder), options); } catch (IOException e) { @@ -221,7 +229,7 @@ public FlintReader createReader(String indexName, String query) { public FlintWriter createWriter(String indexName) { LOG.info("Creating Flint index writer for " + indexName); - return new OpenSearchWriter(createClient(), toLowercase(indexName), options.getRefreshPolicy()); + return new OpenSearchWriter(createClient(), sanitizeIndexName(indexName), options.getRefreshPolicy()); } @Override @@ -287,4 +295,31 @@ private String toLowercase(String indexName) { return indexName.toLowerCase(Locale.ROOT); } + + /* + * Percent-encode invalid OpenSearch index name characters. + */ + private String percentEncode(String indexName) { + Objects.requireNonNull(indexName); + + StringBuilder builder = new StringBuilder(indexName.length()); + for (char ch : indexName.toCharArray()) { + if (INVALID_INDEX_NAME_CHARS.contains(ch)) { + builder.append(String.format("%%%02X", (int) ch)); + } else { + builder.append(ch); + } + } + return builder.toString(); + } + + /* + * Sanitize index name to comply with OpenSearch index name restrictions. + */ + private String sanitizeIndexName(String indexName) { + Objects.requireNonNull(indexName); + + String encoded = percentEncode(indexName); + return toLowercase(encoded); + } } diff --git a/integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala b/integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala index 7da67051d..85be9bbb8 100644 --- a/integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala +++ b/integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala @@ -114,6 +114,30 @@ class FlintOpenSearchClientSuite extends AnyFlatSpec with OpenSearchSuite with M flintClient.exists(indexName) shouldBe false } + it should "percent-encode invalid index name characters" in { + val indexName = "test ,:\"+/\\|?#><" + flintClient.createIndex( + indexName, + FlintMetadata("""{"properties": {"test": { "type": "integer" } } }""")) + + flintClient.exists(indexName) shouldBe true + flintClient.getIndexMetadata(indexName) should not be null + flintClient.getAllIndexMetadata("test *") should not be empty + + // Read write test + val writer = flintClient.createWriter(indexName) + writer.write("""{"create":{}}""") + writer.write("\n") + writer.write("""{"test":1}""") + writer.write("\n") + writer.flush() + writer.close() + flintClient.createReader(indexName, "").hasNext shouldBe true + + flintClient.deleteIndex(indexName) + flintClient.exists(indexName) shouldBe false + } + it should "return false if index not exist" in { flintClient.exists("non-exist-index") shouldBe false }