Skip to content

Commit a8ec260

Browse files
gmjehovichelasticsearchmachine
gmjehovich
and
elasticsearchmachine
authored
[8.19] Add length validation for rename_replacement parameter in snapshot restore request (elastic#137859) (elastic#138320)
* 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]> (cherry picked from commit 6b59792) # Conflicts: # server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java # server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent f2dae0f commit a8ec260

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

‎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
@@ -100,6 +100,7 @@
100100
import java.util.concurrent.atomic.AtomicReference;
101101
import java.util.function.BiConsumer;
102102
import java.util.function.Function;
103+
import java.util.regex.Pattern;
103104
import java.util.stream.Collectors;
104105
import java.util.stream.Stream;
105106

@@ -111,6 +112,7 @@
111112
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
112113
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
113114
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
115+
import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.MAX_INDEX_NAME_BYTES;
114116
import static org.elasticsearch.core.Strings.format;
115117
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
116118
import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_DELETE_SNAPSHOT_ON_INDEX_DELETION;
@@ -981,7 +983,7 @@ private static String renameIndex(String index, RestoreSnapshotRequest request,
981983
if (prefix != null) {
982984
index = index.substring(prefix.length());
983985
}
984-
renamedIndex = index.replaceAll(request.renamePattern(), request.renameReplacement());
986+
renamedIndex = safeRenameIndex(index, request.renamePattern(), request.renameReplacement());
985987
if (prefix != null) {
986988
renamedIndex = prefix + renamedIndex;
987989
}
@@ -1849,6 +1851,27 @@ private void ensureValidIndexName(
18491851
createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false);
18501852
}
18511853

1854+
// package-private for unit testing
1855+
static String safeRenameIndex(String index, String renamePattern, String renameReplacement) {
1856+
final var matcher = Pattern.compile(renamePattern).matcher(index);
1857+
var found = matcher.find();
1858+
if (found) {
1859+
final var sb = new StringBuilder();
1860+
do {
1861+
matcher.appendReplacement(sb, renameReplacement);
1862+
found = matcher.find();
1863+
} while (found && sb.length() <= MAX_INDEX_NAME_BYTES);
1864+
1865+
if (sb.length() > MAX_INDEX_NAME_BYTES) {
1866+
throw new IllegalArgumentException("index name would exceed " + MAX_INDEX_NAME_BYTES + " bytes after rename");
1867+
}
1868+
matcher.appendTail(sb);
1869+
return sb.toString();
1870+
} else {
1871+
return index;
1872+
}
1873+
}
1874+
18521875
private static void ensureSearchableSnapshotsRestorable(
18531876
final ClusterState currentState,
18541877
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
@@ -36,6 +36,7 @@
3636
import java.util.Set;
3737
import java.util.concurrent.atomic.AtomicBoolean;
3838

39+
import static org.hamcrest.Matchers.containsString;
3940
import static org.hamcrest.Matchers.empty;
4041
import static org.hamcrest.Matchers.equalTo;
4142
import static org.mockito.ArgumentMatchers.any;
@@ -245,6 +246,43 @@ public void testNotAllowToRestoreGlobalStateFromSnapshotWithoutOne() {
245246
);
246247
}
247248

249+
public void testSafeRenameIndex() {
250+
// Test normal rename
251+
String result = RestoreService.safeRenameIndex("test-index", "test", "prod");
252+
assertEquals("prod-index", result);
253+
254+
// Test pattern that creates too-long name (255×255 case)
255+
IllegalArgumentException e = expectThrows(
256+
IllegalArgumentException.class,
257+
() -> RestoreService.safeRenameIndex("b".repeat(255), "b", "aa")
258+
);
259+
assertThat(e.getMessage(), containsString("exceed"));
260+
261+
// Test back-reference
262+
result = RestoreService.safeRenameIndex("test-123", "(test)-(\\d+)", "$1_$2");
263+
assertEquals("test_123", result);
264+
265+
// Test back-reference that would be too long
266+
e = expectThrows(IllegalArgumentException.class, () -> RestoreService.safeRenameIndex("a".repeat(200), "(a+)", "$1$1"));
267+
assertThat(e.getMessage(), containsString("exceed"));
268+
269+
// Test no match - returns original
270+
result = RestoreService.safeRenameIndex("test", "xyz", "replacement");
271+
assertEquals("test", result);
272+
273+
// Test exactly at limit (255 chars)
274+
result = RestoreService.safeRenameIndex("b".repeat(255), "b+", "a".repeat(255));
275+
assertEquals("a".repeat(255), result);
276+
277+
// Test empty replacement
278+
result = RestoreService.safeRenameIndex("test-index", "test-", "");
279+
assertEquals("index", result);
280+
281+
// Test multiple matches accumulating
282+
result = RestoreService.safeRenameIndex("a-b-c", "-", "_");
283+
assertEquals("a_b_c", result);
284+
}
285+
248286
private static SnapshotInfo createSnapshotInfo(Snapshot snapshot, Boolean includeGlobalState) {
249287
var shards = randomIntBetween(0, 100);
250288
return new SnapshotInfo(

0 commit comments

Comments
 (0)