Skip to content

Commit 5b9f60a

Browse files
gmjehovichgmjehovich
authored
Add length validation for rename_replacement parameter in snapshot restore request (elastic#137859) (elastic#138323)
* Add length validation for rename_replacement parameter in snapshot restore * [CI] Auto commit changes from spotless * Update RestoreSnapshotIT.java * Update docs/changelog/137859.yaml * Fixup integ testing # Conflicts: # server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java * Fix integration tests * Add new IT test case for long replacement * Add buffer to rename_replacement size check; add safeRenameIndex() * Randomize rename_replacement in testRenameReplacementNameTooLong() * [CI] Auto commit changes from spotless * Add unit test for safeRenameIndex() * fix testRenameReplacementNameTooLong() to use createTestInstance() * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit 6b59792) # Conflicts: # server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java
1 parent 8ca8bcc commit 5b9f60a

File tree

6 files changed

+188
-1
lines changed

6 files changed

+188
-1
lines changed

‎docs/changelog/137859.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 137859
2+
summary: Add length validation for `rename_replacement` parameter in snapshot restore
3+
request
4+
area: Snapshot/Restore
5+
type: bug
6+
issues: []

‎server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,4 +1025,93 @@ public void testNoWarningsOnRestoreOverClosedIndex() throws IllegalAccessExcepti
10251025
mockLog.assertAllExpectationsMatched();
10261026
}
10271027
}
1028+
1029+
public void testRenameReplacementValidation() throws Exception {
1030+
String repoName = "test-repo";
1031+
createRepository(repoName, "fs");
1032+
1033+
// Create an index with a long name (255 characters of 'b')
1034+
String indexName = "b".repeat(255);
1035+
createIndex(indexName);
1036+
indexRandomDocs(indexName, 10);
1037+
1038+
String snapshotName = "test-snap";
1039+
createSnapshot(repoName, snapshotName, Collections.singletonList(indexName));
1040+
1041+
logger.info("--> delete the index");
1042+
cluster().wipeIndices(indexName);
1043+
1044+
logger.info("--> attempt restore with excessively long rename_replacement (should fail validation)");
1045+
IllegalArgumentException exception = expectThrows(
1046+
IllegalArgumentException.class,
1047+
() -> client().admin()
1048+
.cluster()
1049+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1050+
.setIndices(indexName)
1051+
.setRenamePattern("b")
1052+
.setRenameReplacement("1".repeat(randomIntBetween(266, 10_000)))
1053+
.setWaitForCompletion(true)
1054+
.get()
1055+
);
1056+
assertThat(exception.getMessage(), containsString("rename_replacement UTF-8 byte length"));
1057+
assertThat(exception.getMessage(), containsString("exceeds maximum allowed length"));
1058+
1059+
logger.info("--> restore with rename pattern that creates too-long index name (should fail)");
1060+
IllegalArgumentException exception2 = expectThrows(
1061+
IllegalArgumentException.class,
1062+
() -> client().admin()
1063+
.cluster()
1064+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1065+
.setIndices(indexName)
1066+
.setRenamePattern("b")
1067+
.setRenameReplacement("aa")
1068+
.setWaitForCompletion(true)
1069+
.get()
1070+
);
1071+
assertThat(exception2.getMessage(), containsString("index name would exceed"));
1072+
assertThat(exception2.getMessage(), containsString("bytes after rename"));
1073+
1074+
logger.info("--> restore with valid simple rename (should succeed)");
1075+
RestoreSnapshotResponse restoreResponse = client().admin()
1076+
.cluster()
1077+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1078+
.setIndices(indexName)
1079+
.setRenamePattern("b+")
1080+
.setRenameReplacement("restored")
1081+
.setWaitForCompletion(true)
1082+
.get();
1083+
assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0));
1084+
assertTrue("Renamed index should exist", indexExists("restored"));
1085+
ensureGreen("restored");
1086+
1087+
cluster().wipeIndices("restored");
1088+
1089+
logger.info("--> restore with back-reference in replacement (should succeed)");
1090+
RestoreSnapshotResponse restoreResponseBackRef = client().admin()
1091+
.cluster()
1092+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1093+
.setIndices(indexName)
1094+
.setRenamePattern("(b{100}).*")
1095+
.setRenameReplacement("$1-restored")
1096+
.setWaitForCompletion(true)
1097+
.get();
1098+
assertThat(restoreResponseBackRef.getRestoreInfo().failedShards(), equalTo(0));
1099+
String backRefIndex = "b".repeat(100) + "-restored";
1100+
assertTrue("Back-ref index should exist", indexExists(backRefIndex));
1101+
ensureGreen(backRefIndex);
1102+
1103+
cluster().wipeIndices(backRefIndex);
1104+
1105+
logger.info("--> restore with non-matching pattern (should leave name unchanged)");
1106+
RestoreSnapshotResponse restoreResponseNoMatch = client().admin()
1107+
.cluster()
1108+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1109+
.setIndices(indexName)
1110+
.setRenamePattern("z")
1111+
.setRenameReplacement("replaced")
1112+
.setWaitForCompletion(true)
1113+
.get();
1114+
assertThat(restoreResponseNoMatch.getRestoreInfo().failedShards(), equalTo(0));
1115+
assertTrue("Original index name should exist when pattern doesn't match", indexExists(indexName));
1116+
}
10281117
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,15 @@
2626
import org.elasticsearch.xcontent.XContentType;
2727

2828
import java.io.IOException;
29+
import java.nio.charset.StandardCharsets;
2930
import java.util.ArrayList;
3031
import java.util.Arrays;
3132
import java.util.List;
3233
import java.util.Map;
3334
import java.util.Objects;
3435

3536
import static org.elasticsearch.action.ValidateActions.addValidationError;
37+
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES;
3638
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
3739
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;
3840

@@ -152,6 +154,21 @@ public ActionRequestValidationException validate() {
152154
if (featureStates == null) {
153155
validationException = addValidationError("featureStates is missing", validationException);
154156
}
157+
if (renameReplacement != null) {
158+
int byteLength = renameReplacement.getBytes(StandardCharsets.UTF_8).length;
159+
160+
// Allow small buffer for cases where back-references or escapes result in shorter output
161+
if (renameReplacement.getBytes(StandardCharsets.UTF_8).length > MAX_INDEX_NAME_BYTES + 10) {
162+
validationException = addValidationError(
163+
"rename_replacement UTF-8 byte length ["
164+
+ byteLength
165+
+ "] exceeds maximum allowed length ["
166+
+ (MAX_INDEX_NAME_BYTES + 10)
167+
+ "] bytes",
168+
validationException
169+
);
170+
}
171+
}
155172
if (indexSettings == null) {
156173
validationException = addValidationError("indexSettings are missing", validationException);
157174
}

‎server/src/main/java/org/elasticsearch/snapshots/RestoreService.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@
105105
import java.util.concurrent.atomic.AtomicReference;
106106
import java.util.function.BiConsumer;
107107
import java.util.function.Function;
108+
import java.util.regex.Pattern;
108109
import java.util.stream.Collectors;
109110
import java.util.stream.Stream;
110111

@@ -116,6 +117,7 @@
116117
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
117118
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
118119
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
120+
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES;
119121
import static org.elasticsearch.core.Strings.format;
120122
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
121123
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION;
@@ -1038,7 +1040,7 @@ private static String renameIndex(String index, RestoreSnapshotRequest request,
10381040
if (prefix != null) {
10391041
index = index.substring(prefix.length());
10401042
}
1041-
renamedIndex = index.replaceAll(request.renamePattern(), request.renameReplacement());
1043+
renamedIndex = safeRenameIndex(index, request.renamePattern(), request.renameReplacement());
10421044
if (prefix != null) {
10431045
renamedIndex = prefix + renamedIndex;
10441046
}
@@ -1909,6 +1911,27 @@ private void ensureValidIndexName(
19091911
createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false);
19101912
}
19111913

1914+
// package-private for unit testing
1915+
static String safeRenameIndex(String index, String renamePattern, String renameReplacement) {
1916+
final var matcher = Pattern.compile(renamePattern).matcher(index);
1917+
var found = matcher.find();
1918+
if (found) {
1919+
final var sb = new StringBuilder();
1920+
do {
1921+
matcher.appendReplacement(sb, renameReplacement);
1922+
found = matcher.find();
1923+
} while (found && sb.length() <= MAX_INDEX_NAME_BYTES);
1924+
1925+
if (sb.length() > MAX_INDEX_NAME_BYTES) {
1926+
throw new IllegalArgumentException("index name would exceed " + MAX_INDEX_NAME_BYTES + " bytes after rename");
1927+
}
1928+
matcher.appendTail(sb);
1929+
return sb.toString();
1930+
} else {
1931+
return index;
1932+
}
1933+
}
1934+
19121935
private static void ensureSearchableSnapshotsRestorable(
19131936
final ClusterState currentState,
19141937
final SnapshotInfo snapshotInfo,

‎server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

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

12+
import org.elasticsearch.action.ActionRequestValidationException;
1213
import org.elasticsearch.action.support.IndicesOptions;
1314
import org.elasticsearch.common.bytes.BytesReference;
1415
import org.elasticsearch.common.io.stream.BytesStreamOutput;
@@ -29,6 +30,8 @@
2930
import java.util.Map;
3031

3132
import static org.hamcrest.Matchers.containsString;
33+
import static org.junit.Assert.assertFalse;
34+
import static org.junit.Assert.assertNotNull;
3235

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

182+
public void testRenameReplacementNameTooLong() {
183+
RestoreSnapshotRequest request = createTestInstance();
184+
request.indices("b".repeat(255));
185+
request.renamePattern("b");
186+
request.renameReplacement("1".repeat(randomIntBetween(266, 10_000)));
187+
188+
ActionRequestValidationException validation = request.validate();
189+
assertNotNull(validation);
190+
assertThat(validation.getMessage(), containsString("rename_replacement"));
191+
}
192+
179193
private Map<String, Object> convertRequestToMap(RestoreSnapshotRequest request) throws IOException {
180194
XContentBuilder builder = request.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap()));
181195
try (

‎server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.concurrent.atomic.AtomicBoolean;
3939

4040
import static org.elasticsearch.core.Strings.format;
41+
import static org.hamcrest.Matchers.containsString;
4142
import static org.hamcrest.Matchers.empty;
4243
import static org.hamcrest.Matchers.equalTo;
4344
import static org.mockito.ArgumentMatchers.any;
@@ -291,6 +292,43 @@ public void testNotAllowToRestoreGlobalStateFromSnapshotWithoutOne() {
291292
);
292293
}
293294

295+
public void testSafeRenameIndex() {
296+
// Test normal rename
297+
String result = RestoreService.safeRenameIndex("test-index", "test", "prod");
298+
assertEquals("prod-index", result);
299+
300+
// Test pattern that creates too-long name (255×255 case)
301+
IllegalArgumentException e = expectThrows(
302+
IllegalArgumentException.class,
303+
() -> RestoreService.safeRenameIndex("b".repeat(255), "b", "aa")
304+
);
305+
assertThat(e.getMessage(), containsString("exceed"));
306+
307+
// Test back-reference
308+
result = RestoreService.safeRenameIndex("test-123", "(test)-(\\d+)", "$1_$2");
309+
assertEquals("test_123", result);
310+
311+
// Test back-reference that would be too long
312+
e = expectThrows(IllegalArgumentException.class, () -> RestoreService.safeRenameIndex("a".repeat(200), "(a+)", "$1$1"));
313+
assertThat(e.getMessage(), containsString("exceed"));
314+
315+
// Test no match - returns original
316+
result = RestoreService.safeRenameIndex("test", "xyz", "replacement");
317+
assertEquals("test", result);
318+
319+
// Test exactly at limit (255 chars)
320+
result = RestoreService.safeRenameIndex("b".repeat(255), "b+", "a".repeat(255));
321+
assertEquals("a".repeat(255), result);
322+
323+
// Test empty replacement
324+
result = RestoreService.safeRenameIndex("test-index", "test-", "");
325+
assertEquals("index", result);
326+
327+
// Test multiple matches accumulating
328+
result = RestoreService.safeRenameIndex("a-b-c", "-", "_");
329+
assertEquals("a_b_c", result);
330+
}
331+
294332
private static SnapshotInfo createSnapshotInfo(Snapshot snapshot, Boolean includeGlobalState) {
295333
var shards = randomIntBetween(0, 100);
296334
return new SnapshotInfo(

0 commit comments

Comments
 (0)