opendev/gerrit
Code Issues Proposed changes

Restrict size of a robot comment

Browse Source 
Robot comments whose JSON representation exceeds the specified amount
of bytes are rejected on addition. This precautionary measure is
introduced because those comments are typically generated by tools
which might easily generate a huge comment erroneously.

The limit can be specified in the gerrit.config. The default limit is
1024kB. Zero or negative values allow robot comments to be of unlimited
size.

Change-Id: I75b3dfd02266d4223cc0c7e46d3b3f1d8c8b0572
This commit is contained in:
Alice Kober-Sotzek
2017-03-21 14:19:54 +01:00
parent a8060587e0
commit 5ccf7078ab
3 changed files with 128 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
View File

@@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
@@ -173,6 +174,75 @@ public class RobotCommentsIT extends AbstractDaemonTest {
assertRobotComment(comment, in, false);
}
@Test
public void hugeRobotCommentIsRejected() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
int defaultSizeLimit = 1024 * 1024;
int sizeOfRest = 451;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit - sizeOfRest + 1);
exception.expect(BadRequestException.class);
exception.expectMessage("limit");
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
public void reasonablyLargeRobotCommentIsAccepted() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
int defaultSizeLimit = 1024 * 1024;
int sizeOfRest = 451;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit - sizeOfRest);
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThat(robotCommentInfos).hasSize(1);
}
@Test
@GerritConfig(name = "change.robotCommentSizeLimit", value = "10k")
public void maximumAllowedSizeOfRobotCommentCanBeAdjusted() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
int sizeLimit = 10 * 1024;
fixReplacementInfo.replacement = getStringFor(sizeLimit);
exception.expect(BadRequestException.class);
exception.expectMessage("limit");
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
@GerritConfig(name = "change.robotCommentSizeLimit", value = "0")
public void zeroForMaximumAllowedSizeOfRobotCommentRemovesRestriction() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
int defaultSizeLimit = 1024 * 1024;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit);
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThat(robotCommentInfos).hasSize(1);
}
@Test
@GerritConfig(name = "change.robotCommentSizeLimit", value = "-1")
public void negativeValueForMaximumAllowedSizeOfRobotCommentRemovesRestriction()
throws Exception {
assume().that(notesMigration.enabled()).isTrue();
int defaultSizeLimit = 1024 * 1024;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit);
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThat(robotCommentInfos).hasSize(1);
}
@Test
public void addedFixSuggestionCanBeRetrieved() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
@@ -1061,6 +1131,13 @@ public class RobotCommentsIT extends AbstractDaemonTest {
}
}
private static String getStringFor(int numberOfBytes) {
char[] chars = new char[numberOfBytes];
// 'a' will require one byte even when mapped to a JSON string
Arrays.fill(chars, 'a');
return new String(chars);
}
private static List<String> getFixIds(List<RobotCommentInfo> robotComments) {
assertThatList(robotComments).isNotNull();
return robotComments