Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CCR] Enforce auto follow pattern name restrictions #35197

Merged
merged 4 commits into from
Nov 7, 2018
Merged

[CCR] Enforce auto follow pattern name restrictions #35197

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
162f36f
[CCR] Enforce auto follow pattern name restrictions
martijnvg Nov 2, 2018
5a8e5f7
Merge remote-tracking branch 'es/master' into ccr_auto_follow_pattern…
martijnvg Nov 6, 2018
d42d53a
Merge remote-tracking branch 'es/master' into ccr_auto_follow_pattern…
martijnvg Nov 7, 2018
d608563
split tests and added a test that a name can contain a non leading un…
martijnvg Nov 7, 2018
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
29 changes: 29 additions & 0 deletions ...cr/src/test/java/org/elasticsearch/xpack/ccr/action/PutAutoFollowPatternRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,33 @@ public void testValidate() {
validationException = request.validate();
assertThat(validationException, nullValue());
}

public void testValidateName() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you break each of the separate test cases into a separate test? testValidateNameComma, testValidateNameLeadingUnderscore, ...

Also, I'm not seeing a test that a name can otherwise contain an underscore as long as it's not the leader character.

PutAutoFollowPatternAction.Request request = new PutAutoFollowPatternAction.Request();
request.setRemoteCluster("_alias");
request.setLeaderIndexPatterns(Collections.singletonList("logs-*"));

request.setName("name1,name2");
ActionRequestValidationException validationException = request.validate();
assertThat(validationException, notNullValue());
assertThat(validationException.getMessage(), containsString("name must not contain a ','"));

request.setName("_name");
validationException = request.validate();
assertThat(validationException, notNullValue());
assertThat(validationException.getMessage(), containsString("name must not start with '_'"));

StringBuilder stringBuilder = new StringBuilder();
for (int i = 0; i < 256; i++) {
stringBuilder.append('x');
}
request.setName(stringBuilder.toString());
validationException = request.validate();
assertThat(validationException, notNullValue());
assertThat(validationException.getMessage(), containsString("name is too long (256 > 255)"));

request.setName("name");
validationException = request.validate();
assertThat(validationException, nullValue());
}
}
18 changes: 17 additions & 1 deletion ...ore/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutAutoFollowPatternAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.xpack.core.ccr.AutoFollowMetadata.AutoFollowPattern;

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

Expand All @@ -44,8 +45,8 @@ public AcknowledgedResponse newResponse() {
public static class Request extends AcknowledgedRequest<Request> implements ToXContentObject {

private static final ObjectParser<Request, String> PARSER = new ObjectParser<>("put_auto_follow_pattern_request", Request::new);

private static final ParseField NAME_FIELD = new ParseField("name");
private static final int MAX_NAME_BYTES = 255;

static {
PARSER.declareString(Request::setName, NAME_FIELD);
Expand Down Expand Up @@ -119,6 +120,21 @@ public ActionRequestValidationException validate() {
if (name == null) {
validationException = addValidationError("[" + NAME_FIELD.getPreferredName() + "] is missing", validationException);
}
if (name != null) {
if (name.contains(",")) {
validationException = addValidationError("[" + NAME_FIELD.getPreferredName() + "] name must not contain a ','",
validationException);
}
if (name.startsWith("_")) {
validationException = addValidationError("[" + NAME_FIELD.getPreferredName() + "] name must not start with '_'",
validationException);
}
int byteCount = name.getBytes(StandardCharsets.UTF_8).length;
if (byteCount > MAX_NAME_BYTES) {
validationException = addValidationError("[" + NAME_FIELD.getPreferredName() + "] name is too long (" +
byteCount + " > " + MAX_NAME_BYTES + ")", validationException);
}
}
if (remoteCluster == null) {
validationException = addValidationError("[" + REMOTE_CLUSTER_FIELD.getPreferredName() +
"] is missing", validationException);
Expand Down