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

2 files changed, 71 insertions, 4 deletions

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 72bbca69018..fbb20c0353e 100644
--- 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 fc032947f92..190c22f6c9e 100644
--- 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,17 +30,27 @@ 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);
     thrown.expectMessage("on() already called");
@@ -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);
+      }
+    };
+  }
+
 }