Limit size of "message" field in PostCheck to a default of 10k
The limit can be set in gerrit.config as plugin.checks.messageSizeLimit.
Change-Id: I71d707c34049a81cfe62d50a2b53769c79aeae1d
diff --git a/java/com/google/gerrit/plugins/checks/api/PostCheck.java b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
index aaad490..cf7cbd3 100644
--- a/java/com/google/gerrit/plugins/checks/api/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
@@ -36,6 +36,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import java.io.IOException;
@@ -55,6 +56,7 @@
private final Checks checks;
private final Provider<ChecksUpdate> checksUpdate;
private final CheckJson.Factory checkJsonFactory;
+ private final PluginConfigFactory pluginConfigFactory;
@Inject
PostCheck(
@@ -64,7 +66,8 @@
Checkers checkers,
Checks checks,
@UserInitiated Provider<ChecksUpdate> checksUpdate,
- CheckJson.Factory checkJsonFactory) {
+ CheckJson.Factory checkJsonFactory,
+ PluginConfigFactory pluginConfigFactory) {
this.self = self;
this.permissionBackend = permissionBackend;
this.permission = permission;
@@ -72,6 +75,7 @@
this.checks = checks;
this.checksUpdate = checksUpdate;
this.checkJsonFactory = checkJsonFactory;
+ this.pluginConfigFactory = pluginConfigFactory;
}
@Override
@@ -102,6 +106,7 @@
CheckKey key = CheckKey.create(rsrc.getProject(), rsrc.getPatchSet().id(), checkerUuid);
Optional<Check> check = checks.getCheck(key, GetCheckOptions.defaults());
Check updatedCheck;
+ CheckUpdate checkUpdate = toCheckUpdate(input);
if (!check.isPresent()) {
checkers
.getChecker(checkerUuid)
@@ -110,19 +115,15 @@
new UnprocessableEntityException(
String.format("checker %s not found", checkerUuid)));
updatedCheck =
- checksUpdate
- .get()
- .createCheck(key, toCheckUpdate(input), input.notify, input.notifyDetails);
+ checksUpdate.get().createCheck(key, checkUpdate, input.notify, input.notifyDetails);
} else {
updatedCheck =
- checksUpdate
- .get()
- .updateCheck(key, toCheckUpdate(input), input.notify, input.notifyDetails);
+ checksUpdate.get().updateCheck(key, checkUpdate, input.notify, input.notifyDetails);
}
return Response.ok(checkJsonFactory.noOptions().format(updatedCheck));
}
- private static CheckUpdate toCheckUpdate(CheckInput input) throws BadRequestException {
+ private CheckUpdate toCheckUpdate(CheckInput input) throws BadRequestException {
CheckUpdate.Builder checkUpdateBuilder = CheckUpdate.builder();
if (input.state != null) {
@@ -130,7 +131,9 @@
}
if (input.message != null) {
- checkUpdateBuilder.setMessage(input.message.trim());
+ String message = input.message.trim();
+ checkMessageSizeLimit(message.length());
+ checkUpdateBuilder.setMessage(message);
}
if (input.url != null) {
@@ -147,4 +150,14 @@
return checkUpdateBuilder.build();
}
+
+ private void checkMessageSizeLimit(int messageSize) throws BadRequestException {
+ int messageSizeLimit =
+ pluginConfigFactory.getFromGerritConfig("checks").getInt("messageSizeLimit", 10_000);
+ if (messageSize > messageSizeLimit) {
+ throw new BadRequestException(
+ String.format(
+ "Field \"message\" exceeds size limit (%d > %d)", messageSize, messageSizeLimit));
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
index ba01396..c763abc 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.PatchSet;
@@ -98,6 +99,31 @@
}
@Test
+ public void updateMessage_rejected_tooLong() {
+ CheckInput input = new CheckInput();
+ input.message = new String(new char[10_001]).replace('\0', 'x');
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input));
+ assertThat(thrown).hasMessageThat().contains("exceeds size limit (10001 > 10000)");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.checks.messageSizeLimit", value = "5")
+ public void updateMessage_rejected_configureLimit() {
+ CheckInput input = new CheckInput();
+ input.message = "foobar";
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input));
+ assertThat(thrown).hasMessageThat().contains("exceeds size limit (6 > 5)");
+ }
+
+ @Test
public void unsetMessage() throws Exception {
checkOperations.check(checkKey).forUpdate().message("some message").upsert();
diff --git a/resources/Documentation/rest-api-checks.md b/resources/Documentation/rest-api-checks.md
index 0d35ac8..c400b34 100644
--- a/resources/Documentation/rest-api-checks.md
+++ b/resources/Documentation/rest-api-checks.md
@@ -182,7 +182,7 @@
| `patch_set_id` | | The patch set that this check applies to.
| `checker_uuid` | | The [UUID](./rest-api-checkers.md#checker-id) of the checker that reported this check.
| `state` | | The state as string-serialized form of [CheckState](#check-state)
-| `message` | optional | Short message explaining the check state.
+| `message` | optional | Short message explaining the check state. Size limit is 10k by default, configured via plugin.checks.messageSizeLimit.
| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
| `started` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check started processing.
| `finished` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check finished processing.
