Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions docs/changelog/137859.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 137859
summary: Add length validation for `rename_replacement` parameter in snapshot restore
request
area: Snapshot/Restore
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -1025,4 +1025,93 @@ public void testNoWarningsOnRestoreOverClosedIndex() throws IllegalAccessExcepti
mockLog.assertAllExpectationsMatched();
}
}

public void testRenameReplacementValidation() throws Exception {
String repoName = "test-repo";
createRepository(repoName, "fs");

// Create an index with a long name (255 characters of 'b')
String indexName = "b".repeat(255);
createIndex(indexName);
indexRandomDocs(indexName, 10);

String snapshotName = "test-snap";
createSnapshot(repoName, snapshotName, Collections.singletonList(indexName));

logger.info("--> delete the index");
cluster().wipeIndices(indexName);

logger.info("--> attempt restore with excessively long rename_replacement (should fail validation)");
IllegalArgumentException exception = expectThrows(
IllegalArgumentException.class,
() -> client().admin()
.cluster()
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
.setIndices(indexName)
.setRenamePattern("b")
.setRenameReplacement("1".repeat(randomIntBetween(266, 10_000)))
.setWaitForCompletion(true)
.get()
);
assertThat(exception.getMessage(), containsString("rename_replacement UTF-8 byte length"));
assertThat(exception.getMessage(), containsString("exceeds maximum allowed length"));

logger.info("--> restore with rename pattern that creates too-long index name (should fail)");
IllegalArgumentException exception2 = expectThrows(
IllegalArgumentException.class,
() -> client().admin()
.cluster()
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
.setIndices(indexName)
.setRenamePattern("b")
.setRenameReplacement("aa")
.setWaitForCompletion(true)
.get()
);
assertThat(exception2.getMessage(), containsString("index name would exceed"));
assertThat(exception2.getMessage(), containsString("bytes after rename"));

logger.info("--> restore with valid simple rename (should succeed)");
RestoreSnapshotResponse restoreResponse = client().admin()
.cluster()
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
.setIndices(indexName)
.setRenamePattern("b+")
.setRenameReplacement("restored")
.setWaitForCompletion(true)
.get();
assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0));
assertTrue("Renamed index should exist", indexExists("restored"));
ensureGreen("restored");

cluster().wipeIndices("restored");

logger.info("--> restore with back-reference in replacement (should succeed)");
RestoreSnapshotResponse restoreResponseBackRef = client().admin()
.cluster()
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
.setIndices(indexName)
.setRenamePattern("(b{100}).*")
.setRenameReplacement("$1-restored")
.setWaitForCompletion(true)
.get();
assertThat(restoreResponseBackRef.getRestoreInfo().failedShards(), equalTo(0));
String backRefIndex = "b".repeat(100) + "-restored";
assertTrue("Back-ref index should exist", indexExists(backRefIndex));
ensureGreen(backRefIndex);

cluster().wipeIndices(backRefIndex);

logger.info("--> restore with non-matching pattern (should leave name unchanged)");
RestoreSnapshotResponse restoreResponseNoMatch = client().admin()
.cluster()
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
.setIndices(indexName)
.setRenamePattern("z")
.setRenameReplacement("replaced")
.setWaitForCompletion(true)
.get();
assertThat(restoreResponseNoMatch.getRestoreInfo().failedShards(), equalTo(0));
assertTrue("Original index name should exist when pattern doesn't match", indexExists(indexName));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
import org.elasticsearch.xcontent.XContentType;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import static org.elasticsearch.action.ValidateActions.addValidationError;
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES;
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;

Expand Down Expand Up @@ -152,6 +154,21 @@ public ActionRequestValidationException validate() {
if (featureStates == null) {
validationException = addValidationError("featureStates is missing", validationException);
}
if (renameReplacement != null) {
int byteLength = renameReplacement.getBytes(StandardCharsets.UTF_8).length;

// Allow small buffer for cases where back-references or escapes result in shorter output
if (renameReplacement.getBytes(StandardCharsets.UTF_8).length > MAX_INDEX_NAME_BYTES + 10) {
validationException = addValidationError(
"rename_replacement UTF-8 byte length ["
+ byteLength
+ "] exceeds maximum allowed length ["
+ (MAX_INDEX_NAME_BYTES + 10)
+ "] bytes",
validationException
);
}
}
if (indexSettings == null) {
validationException = addValidationError("indexSettings are missing", validationException);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.function.Function;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -116,6 +117,7 @@
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES;
import static org.elasticsearch.core.Strings.format;
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION;
Expand Down Expand Up @@ -1038,7 +1040,7 @@ private static String renameIndex(String index, RestoreSnapshotRequest request,
if (prefix != null) {
index = index.substring(prefix.length());
}
renamedIndex = index.replaceAll(request.renamePattern(), request.renameReplacement());
renamedIndex = safeRenameIndex(index, request.renamePattern(), request.renameReplacement());
if (prefix != null) {
renamedIndex = prefix + renamedIndex;
}
Expand Down Expand Up @@ -1909,6 +1911,27 @@ private void ensureValidIndexName(
createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false);
}

// package-private for unit testing
static String safeRenameIndex(String index, String renamePattern, String renameReplacement) {
final var matcher = Pattern.compile(renamePattern).matcher(index);
var found = matcher.find();
if (found) {
final var sb = new StringBuilder();
do {
matcher.appendReplacement(sb, renameReplacement);
found = matcher.find();
} while (found && sb.length() <= MAX_INDEX_NAME_BYTES);

if (sb.length() > MAX_INDEX_NAME_BYTES) {
throw new IllegalArgumentException("index name would exceed " + MAX_INDEX_NAME_BYTES + " bytes after rename");
}
matcher.appendTail(sb);
return sb.toString();
} else {
return index;
}
}

private static void ensureSearchableSnapshotsRestorable(
final ClusterState currentState,
final SnapshotInfo snapshotInfo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

package org.elasticsearch.action.admin.cluster.snapshots.restore;

import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
Expand All @@ -29,6 +30,8 @@
import java.util.Map;

import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;

public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase<RestoreSnapshotRequest> {
private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) {
Expand Down Expand Up @@ -176,6 +179,17 @@ public void testToStringWillIncludeSkipOperatorOnlyState() {
assertThat(original.toString(), containsString("skipOperatorOnlyState"));
}

public void testRenameReplacementNameTooLong() {
RestoreSnapshotRequest request = createTestInstance();
request.indices("b".repeat(255));
request.renamePattern("b");
request.renameReplacement("1".repeat(randomIntBetween(266, 10_000)));

ActionRequestValidationException validation = request.validate();
assertNotNull(validation);
assertThat(validation.getMessage(), containsString("rename_replacement"));
}

private Map<String, Object> convertRequestToMap(RestoreSnapshotRequest request) throws IOException {
XContentBuilder builder = request.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap()));
try (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.concurrent.atomic.AtomicBoolean;

import static org.elasticsearch.core.Strings.format;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -291,6 +292,43 @@ public void testNotAllowToRestoreGlobalStateFromSnapshotWithoutOne() {
);
}

public void testSafeRenameIndex() {
// Test normal rename
String result = RestoreService.safeRenameIndex("test-index", "test", "prod");
assertEquals("prod-index", result);

// Test pattern that creates too-long name (255×255 case)
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> RestoreService.safeRenameIndex("b".repeat(255), "b", "aa")
);
assertThat(e.getMessage(), containsString("exceed"));

// Test back-reference
result = RestoreService.safeRenameIndex("test-123", "(test)-(\\d+)", "$1_$2");
assertEquals("test_123", result);

// Test back-reference that would be too long
e = expectThrows(IllegalArgumentException.class, () -> RestoreService.safeRenameIndex("a".repeat(200), "(a+)", "$1$1"));
assertThat(e.getMessage(), containsString("exceed"));

// Test no match - returns original
result = RestoreService.safeRenameIndex("test", "xyz", "replacement");
assertEquals("test", result);

// Test exactly at limit (255 chars)
result = RestoreService.safeRenameIndex("b".repeat(255), "b+", "a".repeat(255));
assertEquals("a".repeat(255), result);

// Test empty replacement
result = RestoreService.safeRenameIndex("test-index", "test-", "");
assertEquals("index", result);

// Test multiple matches accumulating
result = RestoreService.safeRenameIndex("a-b-c", "-", "_");
assertEquals("a_b_c", result);
}

private static SnapshotInfo createSnapshotInfo(Snapshot snapshot, Boolean includeGlobalState) {
var shards = randomIntBetween(0, 100);
return new SnapshotInfo(
Expand Down