Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Percent-encode invalid flint index characters #215

Merged
merged 5 commits into from
Jan 9, 2024
Merged

Percent-encode invalid flint index characters #215

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc708ff
percent-encode invalid flint index characters
seankao-az Jan 9, 2024
3f82e67
formatting
seankao-az Jan 9, 2024
a26e779
move encoding logic to FlintOpenSearchClient
seankao-az Jan 9, 2024
4b7cd5e
minor arg fix and string format fix
seankao-az Jan 9, 2024
d8e1748
fix import order
seankao-az Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 43 additions & 7 deletions flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Logger;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.http.HttpHost;
import org.apache.http.auth.AuthScope;
Expand Down Expand Up @@ -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<Character> INVALID_INDEX_NAME_CHARS =
Set.of(' ', ',', ':', '"', '+', '/', '\\', '|', '?', '#', '>', '<');

/**
* Metadata log index name prefix
*/
Expand Down Expand Up @@ -121,7 +129,7 @@ public void createIndex(String indexName, FlintMetadata metadata) {

protected void createIndex(String indexName, String mapping, Option<String> 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);
Expand All @@ -137,7 +145,7 @@ protected void createIndex(String indexName, String mapping, Option<String> 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) {
Expand All @@ -148,7 +156,7 @@ public boolean exists(String indexName) {
@Override
public List<FlintMetadata> 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);
Expand All @@ -166,7 +174,7 @@ public List<FlintMetadata> 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);
Expand All @@ -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);

Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -287,4 +295,32 @@ private String toLowercase(String indexName) {

return indexName.toLowerCase(Locale.ROOT);
}

/*
* Percent-encode invalid OpenSearch index name characters.
*/
private String percentEncode(String indexName, Set<Character> charsToEncode) {
seankao-az marked this conversation as resolved.
Show resolved Hide resolved
Objects.requireNonNull(indexName);
Objects.requireNonNull(charsToEncode);

StringBuilder builder = new StringBuilder(indexName.length());
for (char ch : indexName.toCharArray()) {
if (charsToEncode.contains(ch)) {
builder.append("%").append(String.format("%02X", (int) ch));
Copy link
Collaborator Author

@seankao-az seankao-az Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly prefer this over builder.append(String.format("%%%02X", (int) ch)). suggestions are welcome

edit: did a bit more reading and changed my mind

} 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, INVALID_INDEX_NAME_CHARS);
return toLowercase(encoded);
}
}
24 changes: 24 additions & 0 deletions integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading