Skip to content

Commit 6b59792

Browse files
gmjehovichelasticsearchmachine
gmjehovich
and
elasticsearchmachine
authored
Add length validation for rename_replacement parameter in snapshot restore request (elastic#137859)
* 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]>
1 parent 6e34147 commit 6b59792

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
@@ -1102,4 +1102,93 @@ public void testExplainUnassigableDuringRestore() {
11021102
);
11031103
}
11041104
}
1105+
1106+
public void testRenameReplacementValidation() throws Exception {
1107+
String repoName = "test-repo";
1108+
createRepository(repoName, "fs");
1109+
1110+
// Create an index with a long name (255 characters of 'b')
1111+
String indexName = "b".repeat(255);
1112+
createIndex(indexName);
1113+
indexRandomDocs(indexName, 10);
1114+
1115+
String snapshotName = "test-snap";
1116+
createSnapshot(repoName, snapshotName, Collections.singletonList(indexName));
1117+
1118+
logger.info("--> delete the index");
1119+
cluster().wipeIndices(indexName);
1120+
1121+
logger.info("--> attempt restore with excessively long rename_replacement (should fail validation)");
1122+
IllegalArgumentException exception = expectThrows(
1123+
IllegalArgumentException.class,
1124+
() -> client().admin()
1125+
.cluster()
1126+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1127+
.setIndices(indexName)
1128+
.setRenamePattern("b")
1129+
.setRenameReplacement("1".repeat(randomIntBetween(266, 10_000)))
1130+
.setWaitForCompletion(true)
1131+
.get()
1132+
);
1133+
assertThat(exception.getMessage(), containsString("rename_replacement UTF-8 byte length"));
1134+
assertThat(exception.getMessage(), containsString("exceeds maximum allowed length"));
1135+
1136+
logger.info("--> restore with rename pattern that creates too-long index name (should fail)");
1137+
IllegalArgumentException exception2 = expectThrows(
1138+
IllegalArgumentException.class,
1139+
() -> client().admin()
1140+
.cluster()
1141+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1142+
.setIndices(indexName)
1143+
.setRenamePattern("b")
1144+
.setRenameReplacement("aa")
1145+
.setWaitForCompletion(true)
1146+
.get()
1147+
);
1148+
assertThat(exception2.getMessage(), containsString("index name would exceed"));
1149+
assertThat(exception2.getMessage(), containsString("bytes after rename"));
1150+
1151+
logger.info("--> restore with valid simple rename (should succeed)");
1152+
RestoreSnapshotResponse restoreResponse = client().admin()
1153+
.cluster()
1154+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1155+
.setIndices(indexName)
1156+
.setRenamePattern("b+")
1157+
.setRenameReplacement("restored")
1158+
.setWaitForCompletion(true)
1159+
.get();
1160+
assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0));
1161+
assertTrue("Renamed index should exist", indexExists("restored"));
1162+
ensureGreen("restored");
1163+
1164+
cluster().wipeIndices("restored");
1165+
1166+
logger.info("--> restore with back-reference in replacement (should succeed)");
1167+
RestoreSnapshotResponse restoreResponseBackRef = client().admin()
1168+
.cluster()
1169+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1170+
.setIndices(indexName)
1171+
.setRenamePattern("(b{100}).*")
1172+
.setRenameReplacement("$1-restored")
1173+
.setWaitForCompletion(true)
1174+
.get();
1175+
assertThat(restoreResponseBackRef.getRestoreInfo().failedShards(), equalTo(0));
1176+
String backRefIndex = "b".repeat(100) + "-restored";
1177+
assertTrue("Back-ref index should exist", indexExists(backRefIndex));
1178+
ensureGreen(backRefIndex);
1179+
1180+
cluster().wipeIndices(backRefIndex);
1181+
1182+
logger.info("--> restore with non-matching pattern (should leave name unchanged)");
1183+
RestoreSnapshotResponse restoreResponseNoMatch = client().admin()
1184+
.cluster()
1185+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, repoName, snapshotName)
1186+
.setIndices(indexName)
1187+
.setRenamePattern("z")
1188+
.setRenameReplacement("replaced")
1189+
.setWaitForCompletion(true)
1190+
.get();
1191+
assertThat(restoreResponseNoMatch.getRestoreInfo().failedShards(), equalTo(0));
1192+
assertTrue("Original index name should exist when pattern doesn't match", indexExists(indexName));
1193+
}
11051194
}

‎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
@@ -104,6 +104,7 @@
104104
import java.util.concurrent.atomic.AtomicReference;
105105
import java.util.function.BiConsumer;
106106
import java.util.function.Function;
107+
import java.util.regex.Pattern;
107108
import java.util.stream.Collectors;
108109
import java.util.stream.Stream;
109110

@@ -115,6 +116,7 @@
115116
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
116117
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
117118
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
119+
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES;
118120
import static org.elasticsearch.core.Strings.format;
119121
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
120122
import static org.elasticsearch.repositories.ProjectRepo.projectRepoString;
@@ -1074,7 +1076,7 @@ private static String renameIndex(String index, RestoreSnapshotRequest request,
10741076
if (prefix != null) {
10751077
index = index.substring(prefix.length());
10761078
}
1077-
renamedIndex = index.replaceAll(request.renamePattern(), request.renameReplacement());
1079+
renamedIndex = safeRenameIndex(index, request.renamePattern(), request.renameReplacement());
10781080
if (prefix != null) {
10791081
renamedIndex = prefix + renamedIndex;
10801082
}
@@ -1968,6 +1970,27 @@ private void ensureValidIndexName(
19681970
createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false);
19691971
}
19701972

1973+
// package-private for unit testing
1974+
static String safeRenameIndex(String index, String renamePattern, String renameReplacement) {
1975+
final var matcher = Pattern.compile(renamePattern).matcher(index);
1976+
var found = matcher.find();
1977+
if (found) {
1978+
final var sb = new StringBuilder();
1979+
do {
1980+
matcher.appendReplacement(sb, renameReplacement);
1981+
found = matcher.find();
1982+
} while (found && sb.length() <= MAX_INDEX_NAME_BYTES);
1983+
1984+
if (sb.length() > MAX_INDEX_NAME_BYTES) {
1985+
throw new IllegalArgumentException("index name would exceed " + MAX_INDEX_NAME_BYTES + " bytes after rename");
1986+
}
1987+
matcher.appendTail(sb);
1988+
return sb.toString();
1989+
} else {
1990+
return index;
1991+
}
1992+
}
1993+
19711994
private static void ensureSearchableSnapshotsRestorable(
19721995
final ClusterState currentState,
19731996
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;
@@ -301,6 +302,43 @@ public void testNotAllowToRestoreGlobalStateFromSnapshotWithoutOne() {
301302
);
302303
}
303304

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

0 commit comments

Comments
 (0)