]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-10040 add length validation to Request api and api/projects/create
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 2 Nov 2017 08:20:09 +0000 (09:20 +0100)
committerGuillaume Jambet <guillaume.jambet@gmail.com>
Wed, 8 Nov 2017 12:51:31 +0000 (13:51 +0100)
server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentValidator.java
server/sonar-server/src/main/java/org/sonar/server/project/ws/CreateAction.java
server/sonar-server/src/main/java/org/sonar/server/ws/ws/ListAction.java
server/sonar-server/src/main/resources/org/sonar/server/ws/ws/list-example.json
server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java
server/sonar-server/src/test/java/org/sonar/server/ws/ws/WebServicesWsTest.java
sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java
sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java

index e5c66883d7227292aad4d37b13727c6fe0a37f51..9b6da588042130cabc29a5184c425fb18a6e31a0 100644 (file)
@@ -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() {
index 90f4ffd2e8c954bae57f3028e0f7b9e720be5f85..56e8d82333262f9701e9ca442f87f1a9de41e547 100644 (file)
@@ -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)
index 85f1a9ad8243c7b1c9f82689479e6365a3022671..8f91b4d02793a3ce8e9e7d01df76199e9512b88a 100644 (file)
@@ -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();
   }
 
index f53de56a18ba3ae970441d49446b3773ac5db9ad..2734b17d98c73aba9c1e980fbb20ab5d4dab338f 100644 (file)
@@ -35,6 +35,7 @@
             },
             {
               "key": "severity",
+              "maximumLength": 20,
               "description": "Severity",
               "required": false,
               "internal": false,
index 742c38385f45f2418f2e901e900b1d1d88ff0bf7..5f7071931518900ebbc58b4b81cc10afcb78af4a 100644 (file)
@@ -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) {
index e868789d42c4ee78c93e977961ff47513010f628..dd8110eb008f27a3e9d1c69bfaca5fa9846be30f 100644 (file)
@@ -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")
index f200cfe8cf28cab54df5d04ab79be1fc5a6ec3ea..9d07ee5307ad534d151c6a5541593856d6335ab0 100644 (file)
@@ -633,7 +633,8 @@ public interface WebService extends Definable<WebService.Context> {
     private boolean required = false;
     private boolean internal = false;
     private Set<String> 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<WebService.Context> {
       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<WebService.Context> {
     private final boolean required;
     private final boolean internal;
     private final Set<String> possibleValues;
+    private final Integer maximumLength;
     private final Integer maxValuesAllowed;
 
     protected Param(Action action, NewParam newParam) {
@@ -839,6 +849,7 @@ public interface WebService extends Definable<WebService.Context> {
       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<WebService.Context> {
       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;
index 79e76d501829cb42a51fb09efa3d90d32e4990a0..495cd8e5d821805ca5882ac6fb1b79d6a8c741f2 100644 (file)
@@ -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<String> 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<String> readMultiParamOrDefaultValue(String key, @Nullable WebService.Param definition) {
index d9395d5b598881f51beca2311ca69deb26f88a4f..e95cf3077db2f3e134422b2397062626314ed74d 100644 (file)
@@ -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<WebService.NewParam> 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<String, String> multiParams = ArrayListMultimap.create();
index b73ce7ef5e4914e4dcf46aa55e8f38f0dccddbe3..4b674ea392c52e465646d10233d25fca1973b6ea 100644 (file)
@@ -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);