Limit single comment size to 16k by default (configurable)

16k seems enough to paste e.g. a stack trace or tool output. Limiting
comment size is important because the code makes simplifying
assumptions, e.g. that all comments for a change fit into memory.

Change-Id: Ib870c7c73eec737eabe596b66fb422475075b0a9

java/com/google/gerrit/server/config/GerritGlobalModule.java

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);

java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java

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());
+  }
+}

javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java

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();
+  }
 }