From 453ca8d0954dfe481fbb387743bef357af1b6426 Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Mon, 10 Nov 2025 19:03:20 -0600 Subject: [PATCH 01/13] Add length validation for rename_replacement parameter in snapshot restore --- .../snapshots/RestoreSnapshotIT.java | 59 +++++++++++++++++++ .../restore/RestoreSnapshotRequest.java | 13 ++++ .../restore/RestoreSnapshotRequestTests.java | 16 +++++ 3 files changed, 88 insertions(+) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index 1dabfb5a0edd5..daf1d6b90110b 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1102,4 +1102,63 @@ public void testExplainUnassigableDuringRestore() { ); } } + + public void testRenameReplacementTooLongRejected() 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(10_000_000)) + .setWaitForCompletion(true) + .get() + ); + + assertThat(exception.getMessage(), containsString("rename_replacement")); + assertThat(exception.getMessage(), containsString("exceeds maximum")); + + logger.info("--> attempt restore with 256 character rename_replacement (should also fail)"); + exception = expectThrows( + IllegalArgumentException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) + .setIndices(indexName) + .setRenamePattern("b") + .setRenameReplacement("a".repeat(256)) + .setWaitForCompletion(true) + .get() + ); + + assertThat(exception.getMessage(), containsString("rename_replacement")); + + logger.info("--> restore with valid 255 character rename_replacement (should succeed)"); + RestoreSnapshotResponse restoreResponse = client().admin() + .cluster() + .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) + .setIndices(indexName) + .setRenamePattern("b+") // Match all the b's + .setRenameReplacement("a".repeat(255)) + .setWaitForCompletion(true) + .get(); + + assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0)); + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index e852c49dc2e3f..d008aa70cdeec 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -26,6 +26,7 @@ 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; @@ -35,6 +36,7 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue; +import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES; /** * Restore snapshot request @@ -152,6 +154,17 @@ public ActionRequestValidationException validate() { if (featureStates == null) { validationException = addValidationError("featureStates is missing", validationException); } + if (renameReplacement != null) { + int byteLength = renameReplacement.getBytes(StandardCharsets.UTF_8).length; + + if(renameReplacement.getBytes(StandardCharsets.UTF_8).length > MAX_INDEX_NAME_BYTES) { + validationException = addValidationError( + "rename_replacement UTF-8 byte length [" + byteLength + + "] exceeds maximum allowed length [" + MAX_INDEX_NAME_BYTES + "] bytes", + validationException + ); + } + } if (indexSettings == null) { validationException = addValidationError("indexSettings are missing", validationException); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index 2be8989216d71..91231613f328c 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -9,10 +9,12 @@ 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; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.AbstractWireSerializingTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ToXContent; @@ -29,6 +31,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 { private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) { @@ -176,6 +180,18 @@ public void testToStringWillIncludeSkipOperatorOnlyState() { assertThat(original.toString(), containsString("skipOperatorOnlyState")); } + public void testRenameReplacementNameTooLong() { + RestoreSnapshotRequest request = new RestoreSnapshotRequest(TimeValue.THIRTY_SECONDS, "repo", "snapshot"); + request.indices("b".repeat(255)); + request.renamePattern("b"); + request.renameReplacement("1".repeat(10_000_000)); + + ActionRequestValidationException validation = request.validate(); + assertNotNull(validation); + assertThat(validation.getMessage(), containsString("rename_replacement")); + } + + private Map convertRequestToMap(RestoreSnapshotRequest request) throws IOException { XContentBuilder builder = request.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap())); try ( From b9123ed1e3cbf56ce9b10e923cf69ccc1aaab47c Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 11 Nov 2025 01:11:57 +0000 Subject: [PATCH 02/13] [CI] Auto commit changes from spotless --- .../snapshots/restore/RestoreSnapshotRequest.java | 11 +++++++---- .../restore/RestoreSnapshotRequestTests.java | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index d008aa70cdeec..4193f68d9266e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -34,9 +34,9 @@ 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; -import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES; /** * Restore snapshot request @@ -157,10 +157,13 @@ public ActionRequestValidationException validate() { if (renameReplacement != null) { int byteLength = renameReplacement.getBytes(StandardCharsets.UTF_8).length; - if(renameReplacement.getBytes(StandardCharsets.UTF_8).length > MAX_INDEX_NAME_BYTES) { + if (renameReplacement.getBytes(StandardCharsets.UTF_8).length > MAX_INDEX_NAME_BYTES) { validationException = addValidationError( - "rename_replacement UTF-8 byte length [" + byteLength + - "] exceeds maximum allowed length [" + MAX_INDEX_NAME_BYTES + "] bytes", + "rename_replacement UTF-8 byte length [" + + byteLength + + "] exceeds maximum allowed length [" + + MAX_INDEX_NAME_BYTES + + "] bytes", validationException ); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index 91231613f328c..08216a940b93a 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -191,7 +191,6 @@ public void testRenameReplacementNameTooLong() { assertThat(validation.getMessage(), containsString("rename_replacement")); } - private Map convertRequestToMap(RestoreSnapshotRequest request) throws IOException { XContentBuilder builder = request.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap())); try ( From e69ee3155173eff9aff37e8cf7bab9a200823c1e Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Tue, 11 Nov 2025 13:07:02 -0600 Subject: [PATCH 03/13] Update RestoreSnapshotIT.java --- .../java/org/elasticsearch/snapshots/RestoreSnapshotIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index daf1d6b90110b..fc1ade26d6ef8 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1154,7 +1154,7 @@ public void testRenameReplacementTooLongRejected() throws Exception { .cluster() .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) .setIndices(indexName) - .setRenamePattern("b+") // Match all the b's + .setRenamePattern("b+") .setRenameReplacement("a".repeat(255)) .setWaitForCompletion(true) .get(); From 6571a86b107b493c1f57b1f8167091254b3d0878 Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Tue, 11 Nov 2025 19:13:31 -0600 Subject: [PATCH 04/13] Update docs/changelog/137859.yaml --- docs/changelog/137859.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/137859.yaml diff --git a/docs/changelog/137859.yaml b/docs/changelog/137859.yaml new file mode 100644 index 0000000000000..c0e8e13f9f94b --- /dev/null +++ b/docs/changelog/137859.yaml @@ -0,0 +1,6 @@ +pr: 137859 +summary: Add length validation for `rename_replacement` parameter in snapshot restore + request +area: Snapshot/Restore +type: bug +issues: [] From 1aa25894f8138ec2251e5c0fa1ae507157ed475e Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Wed, 12 Nov 2025 17:44:19 -0600 Subject: [PATCH 05/13] Fixup integ testing # Conflicts: # server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java --- .../snapshots/RestoreSnapshotIT.java | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index fc1ade26d6ef8..03b4294dddd7b 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1103,7 +1103,7 @@ public void testExplainUnassigableDuringRestore() { } } - public void testRenameReplacementTooLongRejected() throws Exception { + public void testRenameReplacementValidation() throws Exception { String repoName = "test-repo"; createRepository(repoName, "fs"); @@ -1126,11 +1126,10 @@ public void testRenameReplacementTooLongRejected() throws Exception { .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) .setIndices(indexName) .setRenamePattern("b") - .setRenameReplacement("1".repeat(10_000_000)) + .setRenameReplacement("1".repeat(randomIntBetween(256, 10_000_000))) .setWaitForCompletion(true) .get() ); - assertThat(exception.getMessage(), containsString("rename_replacement")); assertThat(exception.getMessage(), containsString("exceeds maximum")); @@ -1146,7 +1145,6 @@ public void testRenameReplacementTooLongRejected() throws Exception { .setWaitForCompletion(true) .get() ); - assertThat(exception.getMessage(), containsString("rename_replacement")); logger.info("--> restore with valid 255 character rename_replacement (should succeed)"); @@ -1158,7 +1156,27 @@ public void testRenameReplacementTooLongRejected() throws Exception { .setRenameReplacement("a".repeat(255)) .setWaitForCompletion(true) .get(); - assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0)); + assertTrue("Renamed index should exist", indexExists("test-restored-1")); + ensureGreen("test-restored-1"); + assertDocCount("test-restored-1", 10L); + + // Clean up before next test + cluster().wipeIndices("test-restored-1"); + + logger.info("--> restore with rename pattern that creates too-long index name (should fail)"); + InvalidIndexNameException invalidIndexNameException = expectThrows( + InvalidIndexNameException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) + .setIndices(indexName) + .setRenamePattern("b") // Matches each b individually + .setRenameReplacement("aa") // Would create 510 chars + .setWaitForCompletion(true) + .get() + ); + assertThat(invalidIndexNameException.getMessage(), containsString("Invalid index name")); + } } From 1e029374190a2e3aa21d7e8000f7ff592813f14f Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Wed, 12 Nov 2025 17:51:32 -0600 Subject: [PATCH 06/13] Fix integration tests --- .../snapshots/RestoreSnapshotIT.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index 03b4294dddd7b..5e64f4d5bec3f 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1157,12 +1157,13 @@ public void testRenameReplacementValidation() throws Exception { .setWaitForCompletion(true) .get(); assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0)); - assertTrue("Renamed index should exist", indexExists("test-restored-1")); - ensureGreen("test-restored-1"); - assertDocCount("test-restored-1", 10L); - // Clean up before next test - cluster().wipeIndices("test-restored-1"); + String expectedIndexName = "a".repeat(255); + assertTrue("Renamed index should exist", indexExists(expectedIndexName)); + ensureGreen(expectedIndexName); + assertDocCount(expectedIndexName, 10L); + + cluster().wipeIndices(expectedIndexName); logger.info("--> restore with rename pattern that creates too-long index name (should fail)"); InvalidIndexNameException invalidIndexNameException = expectThrows( @@ -1171,8 +1172,8 @@ public void testRenameReplacementValidation() throws Exception { .cluster() .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) .setIndices(indexName) - .setRenamePattern("b") // Matches each b individually - .setRenameReplacement("aa") // Would create 510 chars + .setRenamePattern("b") + .setRenameReplacement("aa") .setWaitForCompletion(true) .get() ); From dadf20b86afe446eea0bb8035aa2e400bdc8758a Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Wed, 12 Nov 2025 17:54:06 -0600 Subject: [PATCH 07/13] Add new IT test case for long replacement --- .../java/org/elasticsearch/snapshots/RestoreSnapshotIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index 5e64f4d5bec3f..47f6aacc693e7 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1173,11 +1173,10 @@ public void testRenameReplacementValidation() throws Exception { .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) .setIndices(indexName) .setRenamePattern("b") - .setRenameReplacement("aa") + .setRenameReplacement("a".repeat(255)) .setWaitForCompletion(true) .get() ); assertThat(invalidIndexNameException.getMessage(), containsString("Invalid index name")); - } } From 9ed2e5954cb84f6f4afe88b04f80b1f1868958e9 Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Thu, 13 Nov 2025 17:11:44 -0600 Subject: [PATCH 08/13] Add buffer to rename_replacement size check; add safeRenameIndex() --- .../snapshots/RestoreSnapshotIT.java | 67 +++++++++++-------- .../restore/RestoreSnapshotRequest.java | 5 +- .../snapshots/RestoreService.java | 25 ++++++- 3 files changed, 67 insertions(+), 30 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index 47f6aacc693e7..556e181f76de7 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1126,57 +1126,70 @@ public void testRenameReplacementValidation() throws Exception { .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) .setIndices(indexName) .setRenamePattern("b") - .setRenameReplacement("1".repeat(randomIntBetween(256, 10_000_000))) + .setRenameReplacement("1".repeat(randomIntBetween(266, 10_000))) .setWaitForCompletion(true) .get() ); - assertThat(exception.getMessage(), containsString("rename_replacement")); - assertThat(exception.getMessage(), containsString("exceeds maximum")); + assertThat(exception.getMessage(), containsString("rename_replacement UTF-8 byte length")); + assertThat(exception.getMessage(), containsString("exceeds maximum allowed length")); - logger.info("--> attempt restore with 256 character rename_replacement (should also fail)"); - exception = expectThrows( + 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("a".repeat(256)) + .setRenameReplacement("aa") .setWaitForCompletion(true) .get() ); - assertThat(exception.getMessage(), containsString("rename_replacement")); + assertThat(exception2.getMessage(), containsString("index name would exceed")); + assertThat(exception2.getMessage(), containsString("bytes after rename")); + - logger.info("--> restore with valid 255 character rename_replacement (should succeed)"); + 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("a".repeat(255)) + .setRenameReplacement("restored") .setWaitForCompletion(true) .get(); assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0)); + assertTrue("Renamed index should exist", indexExists("restored")); + ensureGreen("restored"); - String expectedIndexName = "a".repeat(255); - assertTrue("Renamed index should exist", indexExists(expectedIndexName)); - ensureGreen(expectedIndexName); - assertDocCount(expectedIndexName, 10L); + cluster().wipeIndices("restored"); - cluster().wipeIndices(expectedIndexName); + 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); - logger.info("--> restore with rename pattern that creates too-long index name (should fail)"); - InvalidIndexNameException invalidIndexNameException = expectThrows( - InvalidIndexNameException.class, - () -> client().admin() - .cluster() - .prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName) - .setIndices(indexName) - .setRenamePattern("b") - .setRenameReplacement("a".repeat(255)) - .setWaitForCompletion(true) - .get() - ); - assertThat(invalidIndexNameException.getMessage(), containsString("Invalid index name")); + 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)); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 4193f68d9266e..dbdbebbdc25e7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -157,12 +157,13 @@ public ActionRequestValidationException validate() { if (renameReplacement != null) { int byteLength = renameReplacement.getBytes(StandardCharsets.UTF_8).length; - if (renameReplacement.getBytes(StandardCharsets.UTF_8).length > MAX_INDEX_NAME_BYTES) { + // 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 + + (MAX_INDEX_NAME_BYTES + 10) + "] bytes", validationException ); diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 5e7287f9b8841..2c7a8ad57b1f8 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -104,6 +104,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; @@ -115,6 +116,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.repositories.ProjectRepo.projectRepoString; @@ -1074,7 +1076,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; } @@ -1968,6 +1970,27 @@ private void ensureValidIndexName( createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false); } + private 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, From bc4012efd9a5fd7d3c325f1ff6e2a9dfcd49bd13 Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Thu, 13 Nov 2025 17:26:28 -0600 Subject: [PATCH 09/13] Randomize rename_replacement in testRenameReplacementNameTooLong() --- .../cluster/snapshots/restore/RestoreSnapshotRequestTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index 08216a940b93a..f3c6fbfba63c6 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -184,7 +184,7 @@ public void testRenameReplacementNameTooLong() { RestoreSnapshotRequest request = new RestoreSnapshotRequest(TimeValue.THIRTY_SECONDS, "repo", "snapshot"); request.indices("b".repeat(255)); request.renamePattern("b"); - request.renameReplacement("1".repeat(10_000_000)); + request.renameReplacement("1".repeat(randomIntBetween(266, 10_000))); ActionRequestValidationException validation = request.validate(); assertNotNull(validation); From de81fabf03ec50ba6a48532c56441e9b6fb603e2 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 13 Nov 2025 23:33:19 +0000 Subject: [PATCH 10/13] [CI] Auto commit changes from spotless --- .../java/org/elasticsearch/snapshots/RestoreSnapshotIT.java | 1 - .../main/java/org/elasticsearch/snapshots/RestoreService.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java index 556e181f76de7..8eae7a867375a 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java @@ -1148,7 +1148,6 @@ public void testRenameReplacementValidation() throws Exception { 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() diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 2c7a8ad57b1f8..23b4fd4360c36 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -1981,8 +1981,7 @@ private static String safeRenameIndex(String index, String renamePattern, String } 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"); + throw new IllegalArgumentException("index name would exceed " + MAX_INDEX_NAME_BYTES + " bytes after rename"); } matcher.appendTail(sb); return sb.toString(); From 6c3552f2fb243f4d89ca04d95b51ebb05d0d11f6 Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Fri, 14 Nov 2025 21:03:38 -0600 Subject: [PATCH 11/13] Add unit test for safeRenameIndex() --- .../snapshots/RestoreService.java | 3 +- .../snapshots/RestoreServiceTests.java | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 23b4fd4360c36..b53dedbbe9444 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -1970,7 +1970,8 @@ private void ensureValidIndexName( createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false); } - private static String safeRenameIndex(String index, String renamePattern, String renameReplacement) { + // 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) { diff --git a/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java b/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java index 6551484257aba..cafcf874aadd2 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java @@ -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; @@ -301,6 +302,46 @@ 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( From 6274e480733915ae53a5479aff28bb101312a8f1 Mon Sep 17 00:00:00 2001 From: Graeme Mjehovich <59065536+gmjehovich@users.noreply.github.com> Date: Fri, 14 Nov 2025 21:07:59 -0600 Subject: [PATCH 12/13] fix testRenameReplacementNameTooLong() to use createTestInstance() --- .../cluster/snapshots/restore/RestoreSnapshotRequestTests.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index f3c6fbfba63c6..3d6f5e80d5474 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.AbstractWireSerializingTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ToXContent; @@ -181,7 +180,7 @@ public void testToStringWillIncludeSkipOperatorOnlyState() { } public void testRenameReplacementNameTooLong() { - RestoreSnapshotRequest request = new RestoreSnapshotRequest(TimeValue.THIRTY_SECONDS, "repo", "snapshot"); + RestoreSnapshotRequest request = createTestInstance(); request.indices("b".repeat(255)); request.renamePattern("b"); request.renameReplacement("1".repeat(randomIntBetween(266, 10_000))); From d35e36b658c44f126c206d900724fe1a588988b3 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Sat, 15 Nov 2025 03:18:09 +0000 Subject: [PATCH 13/13] [CI] Auto commit changes from spotless --- .../org/elasticsearch/snapshots/RestoreServiceTests.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java b/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java index cafcf874aadd2..e3495fc486600 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java @@ -319,10 +319,7 @@ public void testSafeRenameIndex() { assertEquals("test_123", result); // Test back-reference that would be too long - e = expectThrows( - IllegalArgumentException.class, - () -> RestoreService.safeRenameIndex("a".repeat(200), "(a+)", "$1$1") - ); + e = expectThrows(IllegalArgumentException.class, () -> RestoreService.safeRenameIndex("a".repeat(200), "(a+)", "$1$1")); assertThat(e.getMessage(), containsString("exceed")); // Test no match - returns original