From 0b7335e7128458cdba491ec34ec8da9de1d6f06e Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 2 Nov 2017 09:20:09 +0100 Subject: [PATCH] SONAR-10040 add length validation to Request api and api/projects/create --- .../db/component/ComponentValidator.java | 2 +- .../sonar/server/project/ws/CreateAction.java | 4 + .../org/sonar/server/ws/ws/ListAction.java | 4 + .../org/sonar/server/ws/ws/list-example.json | 1 + .../server/project/ws/CreateActionTest.java | 73 ++++++++++++++----- .../sonar/server/ws/ws/WebServicesWsTest.java | 3 +- .../org/sonar/api/server/ws/WebService.java | 23 +++++- .../server/ws/internal/ValidatingRequest.java | 36 ++++----- .../org/sonar/api/server/ws/RequestTest.java | 39 ++++++++++ .../sonar/api/server/ws/WebServiceTest.java | 14 ++++ 10 files changed, 161 insertions(+), 38 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentValidator.java b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentValidator.java index e5c66883d72..9b6da588042 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentValidator.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentValidator.java @@ -24,7 +24,7 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static org.sonar.core.component.ComponentKeys.MAX_COMPONENT_KEY_LENGTH; public class ComponentValidator { - private static final int MAX_COMPONENT_NAME_LENGTH = 2000; + public static final int MAX_COMPONENT_NAME_LENGTH = 2000; private static final int MAX_COMPONENT_QUALIFIER_LENGTH = 10; private ComponentValidator() { diff --git a/server/sonar-server/src/main/java/org/sonar/server/project/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/project/ws/CreateAction.java index 90f4ffd2e8c..56e8d823332 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/project/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/project/ws/CreateAction.java @@ -34,6 +34,8 @@ import org.sonarqube.ws.WsProjects.CreateWsResponse; import org.sonarqube.ws.client.project.CreateRequest; import static org.sonar.api.resources.Qualifiers.PROJECT; +import static org.sonar.core.component.ComponentKeys.MAX_COMPONENT_KEY_LENGTH; +import static org.sonar.db.component.ComponentValidator.MAX_COMPONENT_NAME_LENGTH; import static org.sonar.db.permission.OrganizationPermission.PROVISION_PROJECTS; import static org.sonar.server.component.NewComponent.newComponentBuilder; import static org.sonar.server.project.ws.ProjectsWsSupport.PARAM_ORGANIZATION; @@ -79,11 +81,13 @@ public class CreateAction implements ProjectsWsAction { .setDescription("Key of the project") .setDeprecatedKey(DEPRECATED_PARAM_KEY, "6.3") .setRequired(true) + .setMaximumLength(MAX_COMPONENT_KEY_LENGTH) .setExampleValue(KEY_PROJECT_EXAMPLE_001); action.createParam(PARAM_NAME) .setDescription("Name of the project") .setRequired(true) + .setMaximumLength(MAX_COMPONENT_NAME_LENGTH) .setExampleValue("SonarQube"); action.createParam(PARAM_BRANCH) diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/ws/ListAction.java b/server/sonar-server/src/main/java/org/sonar/server/ws/ws/ListAction.java index 85f1a9ad824..8f91b4d0279 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/ws/ListAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/ws/ListAction.java @@ -138,6 +138,10 @@ public class ListAction implements WebServicesWsAction { if (possibleValues != null) { writer.name("possibleValues").beginArray().values(possibleValues).endArray(); } + Integer maximumLength = param.maximumLength(); + if (maximumLength != null) { + writer.prop("maximumLength", maximumLength); + } writer.endObject(); } diff --git a/server/sonar-server/src/main/resources/org/sonar/server/ws/ws/list-example.json b/server/sonar-server/src/main/resources/org/sonar/server/ws/ws/list-example.json index f53de56a18b..2734b17d98c 100644 --- a/server/sonar-server/src/main/resources/org/sonar/server/ws/ws/list-example.json +++ b/server/sonar-server/src/main/resources/org/sonar/server/ws/ws/list-example.json @@ -35,6 +35,7 @@ }, { "key": "severity", + "maximumLength": 20, "description": "Severity", "required": false, "internal": false, diff --git a/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java index 742c38385f4..5f707193151 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.project.ws; -import org.assertj.core.api.Assertions; import org.assertj.core.api.AssertionsForClassTypes; import org.junit.Rule; import org.junit.Test; @@ -48,7 +47,8 @@ import org.sonarqube.ws.WsProjects.CreateWsResponse; import org.sonarqube.ws.WsProjects.CreateWsResponse.Project; import org.sonarqube.ws.client.project.CreateRequest; -import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; +import static joptsimple.internal.Strings.repeat; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doThrow; @@ -88,8 +88,7 @@ public class CreateActionTest { new ProjectsWsSupport(db.getDbClient(), defaultOrganizationProvider, billingValidations), db.getDbClient(), userSession, new ComponentUpdater(db.getDbClient(), i18n, system2, mock(PermissionTemplateService.class), new FavoriteUpdater(db.getDbClient()), - projectIndexers) - )); + projectIndexers))); @Test public void create_project() throws Exception { @@ -265,6 +264,8 @@ public class CreateActionTest { @Test public void fail_when_missing_project_parameter() throws Exception { + userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization()); + expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'project' parameter is missing"); @@ -273,6 +274,8 @@ public class CreateActionTest { @Test public void fail_when_missing_name_parameter() throws Exception { + userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization()); + expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'name' parameter is missing"); @@ -286,6 +289,32 @@ public class CreateActionTest { call(CreateRequest.builder().setKey(DEFAULT_PROJECT_KEY).setName(DEFAULT_PROJECT_NAME).build()); } + @Test + public void fail_when_project_parameter_is_longer_than_400() { + userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization()); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'project' length (401) is longer than the maximum authorized (400)"); + + ws.newRequest().setMethod(POST.name()) + .setParam("project", repeat('a', 401)) + .setParam("name", DEFAULT_PROJECT_NAME) + .execute(); + } + + @Test + public void fail_when_name_parameter_is_longer_than_2000() { + userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization()); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("'name' length (2001) is longer than the maximum authorized (2000)"); + + ws.newRequest().setMethod(POST.name()) + .setParam("project", "key") + .setParam("name", repeat('a', 2001)) + .execute(); + } + @Test public void test_example() { userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization()); @@ -302,12 +331,12 @@ public class CreateActionTest { public void definition() { WebService.Action definition = ws.getDef(); - Assertions.assertThat(definition.key()).isEqualTo("create"); - Assertions.assertThat(definition.since()).isEqualTo("4.0"); - Assertions.assertThat(definition.isInternal()).isFalse(); - Assertions.assertThat(definition.responseExampleAsString()).isNotEmpty(); + assertThat(definition.key()).isEqualTo("create"); + assertThat(definition.since()).isEqualTo("4.0"); + assertThat(definition.isInternal()).isFalse(); + assertThat(definition.responseExampleAsString()).isNotEmpty(); - Assertions.assertThat(definition.params()).extracting(WebService.Param::key).containsExactlyInAnyOrder( + assertThat(definition.params()).extracting(WebService.Param::key).containsExactlyInAnyOrder( PARAM_VISIBILITY, PARAM_ORGANIZATION, PARAM_NAME, @@ -315,17 +344,25 @@ public class CreateActionTest { PARAM_BRANCH); WebService.Param organization = definition.param(PARAM_ORGANIZATION); - Assertions.assertThat(organization.description()).isEqualTo("The key of the organization"); - Assertions.assertThat(organization.isInternal()).isTrue(); - Assertions.assertThat(organization.isRequired()).isFalse(); - Assertions.assertThat(organization.since()).isEqualTo("6.3"); + assertThat(organization.description()).isEqualTo("The key of the organization"); + assertThat(organization.isInternal()).isTrue(); + assertThat(organization.isRequired()).isFalse(); + assertThat(organization.since()).isEqualTo("6.3"); WebService.Param isPrivate = definition.param(PARAM_VISIBILITY); - Assertions.assertThat(isPrivate.description()).isNotEmpty(); - Assertions.assertThat(isPrivate.isInternal()).isTrue(); - Assertions.assertThat(isPrivate.isRequired()).isFalse(); - Assertions.assertThat(isPrivate.since()).isEqualTo("6.4"); - Assertions.assertThat(isPrivate.possibleValues()).containsExactlyInAnyOrder("private", "public"); + assertThat(isPrivate.description()).isNotEmpty(); + assertThat(isPrivate.isInternal()).isTrue(); + assertThat(isPrivate.isRequired()).isFalse(); + assertThat(isPrivate.since()).isEqualTo("6.4"); + assertThat(isPrivate.possibleValues()).containsExactlyInAnyOrder("private", "public"); + + WebService.Param project = definition.param(PARAM_PROJECT); + assertThat(project.isRequired()).isTrue(); + assertThat(project.maximumLength()).isEqualTo(400); + + WebService.Param name = definition.param(PARAM_NAME); + assertThat(name.isRequired()).isTrue(); + assertThat(name.maximumLength()).isEqualTo(2000); } private CreateWsResponse call(CreateRequest request) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/ws/WebServicesWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/ws/WebServicesWsTest.java index e868789d42c..dd8110eb008 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/ws/WebServicesWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/ws/WebServicesWsTest.java @@ -100,10 +100,10 @@ public class WebServicesWsTest { // action with a lot of overridden values NewAction create = newController.createAction("create") + .setPost(true) .setDescription("Create metric") .setSince("4.1") .setDeprecatedSince("5.3") - .setPost(true) .setResponseExample(Resources.getResource(getClass(), "WebServicesWsTest/metrics_example.json")) .setChangelog( new Change("4.5", "Deprecate database ID in response"), @@ -114,6 +114,7 @@ public class WebServicesWsTest { create .createParam("severity") + .setMaximumLength(20) .setDescription("Severity") .setSince("4.4") .setDeprecatedSince("5.2") diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java index f200cfe8cf2..9d07ee5307a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java @@ -633,7 +633,8 @@ public interface WebService extends Definable { private boolean required = false; private boolean internal = false; private Set possibleValues = null; - private Integer maxValuesAllowed = null; + private Integer maxValuesAllowed; + private Integer maximumLength; private NewParam(String key) { this.key = key; @@ -770,6 +771,14 @@ public interface WebService extends Definable { return this; } + /** + * @since 7.0 + */ + public NewParam setMaximumLength(@Nullable Integer maximumLength) { + this.maximumLength = maximumLength; + return this; + } + @Override public String toString() { return key; @@ -824,6 +833,7 @@ public interface WebService extends Definable { private final boolean required; private final boolean internal; private final Set possibleValues; + private final Integer maximumLength; private final Integer maxValuesAllowed; protected Param(Action action, NewParam newParam) { @@ -839,6 +849,7 @@ public interface WebService extends Definable { this.internal = newParam.internal; this.possibleValues = newParam.possibleValues; this.maxValuesAllowed = newParam.maxValuesAllowed; + this.maximumLength = newParam.maximumLength; checkArgument(!required || defaultValue == null, "Default value must not be set on parameter '%s?%s' as it's marked as required", action, key); } @@ -935,6 +946,16 @@ public interface WebService extends Definable { return maxValuesAllowed; } + /** + * Specify the maximum length of the value used in this parameter + * + * @since 7.0 + */ + @CheckForNull + public Integer maximumLength() { + return maximumLength; + } + @Override public String toString() { return key; diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java index 79e76d50182..495cd8e5d82 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java @@ -67,7 +67,22 @@ public abstract class ValidatingRequest extends Request { @Override @CheckForNull public String param(String key) { - return param(key, true); + WebService.Param definition = action.param(key); + String value = defaultString(readParam(key, definition), definition.defaultValue()); + + Integer maximumLength = definition.maximumLength(); + if (value != null && maximumLength != null) { + int valueLength = value.length(); + checkArgument(valueLength <= maximumLength, + "'%s' length (%s) is longer than the maximum authorized (%s)", + key, valueLength, maximumLength); + } + + String trimmedValue = value == null ? null : CharMatcher.WHITESPACE.trimFrom(value); + if (trimmedValue != null) { + validateValue(trimmedValue, definition); + } + return trimmedValue; } @Override @@ -89,22 +104,11 @@ public abstract class ValidatingRequest extends Request { return readPart(key); } - @CheckForNull - private String param(String key, boolean validateValue) { - WebService.Param definition = action.param(key); - String value = readParamOrDefaultValue(key, definition); - String trimmedValue = value == null ? null : CharMatcher.WHITESPACE.trimFrom(value); - if (trimmedValue != null && validateValue) { - validateValue(trimmedValue, definition); - } - return trimmedValue; - } - @CheckForNull @Override public List paramAsStrings(String key) { WebService.Param definition = action.param(key); - String value = readParamOrDefaultValue(key, definition); + String value = defaultString(readParam(key, definition), definition.defaultValue()); if (value == null) { return null; } @@ -125,12 +129,10 @@ public abstract class ValidatingRequest extends Request { } @CheckForNull - private String readParamOrDefaultValue(String key, @Nullable WebService.Param definition) { + private String readParam(String key, @Nullable WebService.Param definition) { checkArgument(definition != null, "BUG - parameter '%s' is undefined for action '%s'", key, action.key()); - String deprecatedKey = definition.deprecatedKey(); - String value = deprecatedKey != null ? defaultString(readParam(deprecatedKey), readParam(key)) : readParam(key); - return defaultString(value, definition.defaultValue()); + return deprecatedKey != null ? defaultString(readParam(deprecatedKey), readParam(key)) : readParam(key); } private List readMultiParamOrDefaultValue(String key, @Nullable WebService.Param definition) { diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java index d9395d5b598..e95cf3077db 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import javax.annotation.Nullable; import org.apache.commons.io.IOUtils; import org.junit.Before; @@ -43,7 +44,9 @@ import org.sonar.api.server.ws.internal.PartImpl; import org.sonar.api.server.ws.internal.ValidatingRequest; import org.sonar.api.utils.DateUtils; +import static com.google.common.base.Strings.repeat; import static com.google.common.collect.Lists.newArrayList; +import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.mock; @@ -96,6 +99,28 @@ public class RequestTest { assertThat(underTest.mandatoryParamAsEnum("a_required_enum", RuleStatus.class)).isEqualTo(RuleStatus.BETA); } + @Test + public void maximum_length_ok() { + String parameter = "maximum_length_param"; + defineParameterTestAction(newParam -> newParam.setMaximumLength(10), parameter); + String value = repeat("X", 10); + + String param = underTest.setParam(parameter, value).param(parameter); + + assertThat(param).isEqualTo(value); + } + + @Test + public void maximum_length_not_ok() { + String parameter = "maximum_length_param"; + defineParameterTestAction(newParam -> newParam.setMaximumLength(10), parameter); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(format("'%s' length (11) is longer than the maximum authorized (10)", parameter)); + + underTest.setParam(parameter, repeat("X", 11)).param(parameter); + } + @Test public void required_param_as_strings() { underTest.setParam("a_required_string", "foo,bar"); @@ -580,6 +605,20 @@ public class RequestTest { underTest.mandatoryParamAsPart("required_param"); } + private void defineParameterTestAction(Consumer newParam, String parameter) { + String controllerPath = "my_controller"; + String actionPath = "my_action"; + WebService.Context context = new WebService.Context(); + WebService.NewController controller = context.createController(controllerPath); + WebService.NewAction action = controller + .createAction(actionPath) + .setHandler(mock(RequestHandler.class)); + WebService.NewParam param = action.createParam(parameter); + newParam.accept(param); + controller.done(); + underTest.setAction(context.controller(controllerPath).action(actionPath)); + } + private static class FakeRequest extends ValidatingRequest { private final ListMultimap multiParams = ArrayListMultimap.create(); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java index b73ce7ef5e4..4b674ea392c 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java @@ -301,6 +301,20 @@ public class WebServiceTest { assertThat(action.param("status").possibleValues()).isNull(); } + @Test + public void param_with_maximum_length() { + ((WebService) context -> { + NewController newController = context.createController("api/custom_measures"); + NewAction create = newDefaultAction(newController, "create"); + create.createParam("string_value") + .setMaximumLength(24); + newController.done(); + }).define(context); + + WebService.Action action = context.controller("api/custom_measures").action("create"); + assertThat(action.param("string_value").maximumLength()).isEqualTo(24); + } + @Test public void fail_if_required_param_has_default_value() { expectedException.expect(IllegalArgumentException.class); -- 2.39.5