SONAR-10460 do not allow null character in scanner for issue message

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
index 72bbca69018095af3bc7e60be8cf73e12614b963..fbb20c0353e676dd84061cc3b0b1ad92bb4e639f 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
@@ -20,14 +20,18 @@
 package org.sonar.api.batch.sensor.issue.internal;
 
 import com.google.common.base.Preconditions;
-import org.apache.commons.lang.StringUtils;
 import org.sonar.api.batch.fs.InputComponent;
 import org.sonar.api.batch.fs.TextRange;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.sensor.issue.IssueLocation;
 import org.sonar.api.batch.sensor.issue.NewIssueLocation;
 
+import javax.annotation.Nullable;
+
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
+import static org.apache.commons.lang.StringUtils.abbreviate;
+import static org.apache.commons.lang.StringUtils.trim;
 
 public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
 
@@ -37,7 +41,7 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
 
   @Override
   public DefaultIssueLocation on(InputComponent component) {
-    Preconditions.checkArgument(component != null, "Component can't be null");
+    checkArgument(component != null, "Component can't be null");
     Preconditions.checkState(this.component == null, "on() already called");
     this.component = component;
     return this;
@@ -56,10 +60,21 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
   @Override
   public DefaultIssueLocation message(String message) {
     requireNonNull(message, "Message can't be null");
-    this.message = StringUtils.abbreviate(StringUtils.trim(message), MESSAGE_MAX_SIZE);
+    if (message.contains("\u0000")) {
+      throw new IllegalArgumentException(unsupportedCharacterError(message, component));
+    }
+    this.message = abbreviate(trim(message), MESSAGE_MAX_SIZE);
     return this;
   }
 
+  private static String unsupportedCharacterError(String message, @Nullable InputComponent component) {
+    String error = "Character \\u0000 is not supported in issue message '" + message + "'";
+    if (component != null) {
+      error += ", on component: " + component.toString();
+    }
+    return error;
+  }
+
   @Override
   public InputComponent inputComponent() {
     return this.component;
@@ -75,4 +90,10 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
     return this.message;
   }
 
+  public static void main (String[] args) {
+
+    new DefaultIssueLocation().message("pipo");
+
+  }
+
 }

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
index fc032947f9231ca66d2298e078f11effe53cc846..190c22f6c9e155ba9283e92458eb8e6ccf91cedf 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
@@ -20,6 +20,9 @@
 package org.sonar.api.batch.sensor.issue.internal;
 
 import org.apache.commons.lang.StringUtils;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeMatcher;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -27,16 +30,26 @@ import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.rules.ExpectedException.none;
 
 public class DefaultIssueLocationTest {
 
   @Rule
-  public ExpectedException thrown = ExpectedException.none();
+  public ExpectedException thrown = none();
 
   private InputFile inputFile = new TestInputFileBuilder("foo", "src/Foo.php")
     .initMetadata("Foo\nBar\n")
     .build();
 
+  @Test
+  public void should_build() {
+    assertThat(new DefaultIssueLocation()
+      .on(inputFile)
+      .message("pipo bimbo")
+      .message()
+    ).isEqualTo("pipo bimbo");
+  }
+
   @Test
   public void not_allowed_to_call_on_twice() {
     thrown.expect(IllegalStateException.class);
@@ -58,4 +71,37 @@ public class DefaultIssueLocationTest {
       .message(StringUtils.repeat("a", 4001)).message()).hasSize(4000);
   }
 
+  @Test
+  public void prevent_null_character_in_message_text() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Character \\u0000 is not supported in issue message");
+
+    new DefaultIssueLocation()
+      .message("pipo " + '\u0000' + " bimbo");
+  }
+
+  @Test
+  public void prevent_null_character_in_message_text_when_builder_has_been_initialized() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage(customMatcher("Character \\u0000 is not supported in issue message", ", on component: src/Foo.php"));
+
+    new DefaultIssueLocation()
+      .on(inputFile)
+      .message("pipo " + '\u0000' + " bimbo");
+  }
+
+  private Matcher<String> customMatcher(String startWith, String endWith) {
+    return new TypeSafeMatcher<String>() {
+      @Override
+      public void describeTo(Description description) {
+        description.appendText("Invalid message");
+      }
+
+      @Override
+      protected boolean matchesSafely(final String item) {
+        return item.startsWith(startWith) && item.endsWith(endWith);
+      }
+    };
+  }
+
 }

diff --git a/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationTest.java b/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationTest.java
index e369330a291836123218f09a07fb917a1fce0fc6..3112caf8eac871325b2f73b542cbe07449b7689c 100644 (file)
--- a/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationTest.java
+++ b/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationTest.java
@@ -21,17 +21,23 @@ package org.sonarqube.tests.issue;
 
 import org.apache.commons.lang.StringUtils;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.wsclient.issue.Issue;
 import org.sonar.wsclient.issue.IssueQuery;
 import org.sonar.wsclient.issue.Issues;
 import util.ItUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.rules.ExpectedException.none;
 import static util.ItUtils.runProjectAnalysis;
 
 public class IssueCreationTest extends AbstractIssueTest {
 
+  @Rule
+  public ExpectedException thrown = none();
+
   private static final String SAMPLE_PROJECT_KEY = "sample";
   private static final int ISSUE_MESSAGE_MAX_LENGTH = 1_333;
 
@@ -60,6 +66,17 @@ public class IssueCreationTest extends AbstractIssueTest {
     assertThat(issue.message()).isEqualTo("Issue With Custom Message");
   }
 
+  @Test
+  public void fail_if_message_contains_null_character(){
+    ORCHESTRATOR.getServer().provisionProject(SAMPLE_PROJECT_KEY, SAMPLE_PROJECT_KEY);
+    ItUtils.restoreProfile(ORCHESTRATOR, getClass().getResource("/issue/IssueCreationTest/with-custom-message.xml"));
+    ORCHESTRATOR.getServer().associateProjectToQualityProfile(SAMPLE_PROJECT_KEY, "xoo", "with-custom-message");
+
+    thrown.expect(java.lang.IllegalStateException.class);
+
+    runProjectAnalysis(ORCHESTRATOR, "shared/xoo-sample", "sonar.customMessage.message", "a \u0000 0message");
+  }
+
   @Test
   public void plugin_can_override_profile_severity() {
     ORCHESTRATOR.getServer().provisionProject(SAMPLE_PROJECT_KEY, SAMPLE_PROJECT_KEY);