Merge "Limit single comment size to 16k by default (configurable)"
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index edf8864..7df3902 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -128,6 +128,7 @@
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.receive.ReceiveCommitsModule;
+import com.google.gerrit.server.git.validators.CommentLimitsValidator;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.MergeValidationListener;
import com.google.gerrit.server.git.validators.MergeValidators;
@@ -408,6 +409,10 @@
factory(UploadValidators.Factory.class);
DynamicSet.setOf(binder(), UploadValidationListener.class);
+ bind(CommentValidator.class)
+ .annotatedWith(Exports.named(CommentLimitsValidator.class.getSimpleName()))
+ .to(CommentLimitsValidator.class);
+
DynamicMap.mapOf(binder(), DynamicOptions.DynamicBean.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeHasOperandFactory.class);
diff --git a/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java b/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
new file mode 100644
index 0000000..8237e69
--- /dev/null
+++ b/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
@@ -0,0 +1,46 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git.validators;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationFailure;
+import com.google.gerrit.extensions.validators.CommentValidator;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import org.eclipse.jgit.lib.Config;
+
+/** Limits the size of comments to prevent large comments from causing issues. */
+public class CommentLimitsValidator implements CommentValidator {
+ private final int maxCommentLength;
+
+ @Inject
+ CommentLimitsValidator(@GerritServerConfig Config serverConfig) {
+ maxCommentLength = serverConfig.getInt("change", null, "maxCommentLength", 16 << 10);
+ }
+
+ @Override
+ public ImmutableList<CommentValidationFailure> validateComments(
+ ImmutableList<CommentForValidation> comments) {
+ return comments.stream()
+ .filter(c -> c.getText().length() > maxCommentLength)
+ .map(
+ c ->
+ c.failValidation(
+ String.format(
+ "Comment too large (%d > %d)", c.getText().length(), maxCommentLength)))
+ .collect(ImmutableList.toImmutableList());
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
index 84bf759c..d8b65b7 100644
--- a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.git.receive;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -24,6 +25,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.client.Side;
@@ -46,6 +48,8 @@
@Inject private CommentValidator mockCommentValidator;
@Inject private TestCommentHelper testCommentHelper;
+ private static final int MAX_COMMENT_LENGTH = 666;
+
private static final String COMMENT_TEXT = "The comment text";
@Captor private ArgumentCaptor<ImmutableList<CommentForValidation>> capture;
@@ -135,4 +139,23 @@
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, draftFile.message));
}
+
+ @Test
+ @GerritConfig(name = "change.maxCommentLength", value = "" + MAX_COMMENT_LENGTH)
+ public void validateComments_enforceLimits_commentTooLarge() throws Exception {
+ when(mockCommentValidator.validateComments(any())).thenReturn(ImmutableList.of());
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ int commentLength = MAX_COMMENT_LENGTH + 1;
+ DraftInput comment =
+ testCommentHelper.newDraft(new String(new char[commentLength]).replace("\0", "x"));
+ testCommentHelper.addDraft(changeId, result.getCommit().getName(), comment);
+
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
+ Result amendResult = amendChange(changeId, "refs/for/master%publish-comments", admin, testRepo);
+ amendResult.assertOkStatus();
+ amendResult.assertMessage(
+ String.format("Comment too large (%d > %d)", commentLength, MAX_COMMENT_LENGTH));
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
+ }
}