SONAR-13615 Prevent scanner failure in case of null character in issue message

2 files changed, 18 insertions, 28 deletions

diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
index c06a28dfb61..e49b672004a 100644
--- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
+++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
@@ -21,7 +21,7 @@ package org.sonar.api.batch.sensor.issue.internal;
 
 import java.util.ArrayList;
 import java.util.List;
-import javax.annotation.Nullable;
+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;
@@ -39,6 +39,9 @@ import static org.sonar.api.utils.Preconditions.checkState;
 
 public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
 
+  private static final String NULL_CHARACTER = "\u0000";
+  private static final String NULL_REPLACEMENT = "[NULL]";
+
   private InputComponent component;
   private TextRange textRange;
   private String message;
@@ -65,7 +68,8 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
   @Override
   public DefaultIssueLocation message(String message) {
     validateMessage(message);
-    this.message = abbreviate(trim(message), Issue.MESSAGE_MAX_SIZE);
+    String sanitizedMessage = sanitizeNulls(message);
+    this.message = abbreviate(trim(sanitizedMessage), Issue.MESSAGE_MAX_SIZE);
     return this;
   }
 
@@ -73,7 +77,8 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
   public DefaultIssueLocation message(String message, List<NewMessageFormatting> newMessageFormattings) {
     validateMessage(message);
     validateFormattings(newMessageFormattings, message);
-    this.message = abbreviate(message,  Issue.MESSAGE_MAX_SIZE);
+    String sanitizedMessage = sanitizeNulls(message);
+    this.message = abbreviate(sanitizedMessage, Issue.MESSAGE_MAX_SIZE);
 
     for (NewMessageFormatting newMessageFormatting : newMessageFormattings) {
       DefaultMessageFormatting messageFormatting = (DefaultMessageFormatting) newMessageFormatting;
@@ -98,11 +103,12 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
       .forEach(e -> e.validate(message));
   }
 
-  private void validateMessage(String message) {
+  private static void validateMessage(String message) {
     requireNonNull(message, "Message can't be null");
-    if (message.contains("\u0000")) {
-      throw new IllegalArgumentException(unsupportedCharacterError(message, component));
-    }
+  }
+
+  private static String sanitizeNulls(String message) {
+    return StringUtils.replace(message, NULL_CHARACTER, NULL_REPLACEMENT);
   }
 
   @Override
@@ -110,14 +116,6 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
     return new DefaultMessageFormatting();
   }
 
-  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;
diff --git a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
index a8e44a5bb4a..ef937b6ee60 100644
--- a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
+++ b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
@@ -34,7 +34,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.tuple;
 
 public class DefaultIssueLocationTest {
 
-  private InputFile inputFile = new TestInputFileBuilder("foo", "src/Foo.php")
+  private final InputFile inputFile = new TestInputFileBuilder("foo", "src/Foo.php")
     .initMetadata("Foo\nBar\n")
     .build();
 
@@ -109,21 +109,13 @@ public class DefaultIssueLocationTest {
   }
 
   @Test
-  public void prevent_null_character_in_message_text() {
-    assertThatThrownBy(() -> new DefaultIssueLocation()
-      .message("pipo " + '\u0000' + " bimbo"))
-      .isInstanceOf(IllegalArgumentException.class)
-      .hasMessageContaining("Character \\u0000 is not supported in issue message");
+  public void message_whenSettingMessage_shouldReplaceNullChar() {
+    assertThat(new DefaultIssueLocation().message("test " + '\u0000' + "123").message()).isEqualTo("test [NULL]123");
   }
 
   @Test
-  public void prevent_null_character_in_message_text_when_builder_has_been_initialized() {
-    assertThatThrownBy(() -> new DefaultIssueLocation()
-      .on(inputFile)
-      .message("pipo " + '\u0000' + " bimbo"))
-      .isInstanceOf(IllegalArgumentException.class)
-      .hasMessageStartingWith("Character \\u0000 is not supported in issue message")
-      .hasMessageEndingWith(", on component: src/Foo.php");
+  public void message_whenSettingMessageWithFormattings_shouldReplaceNullChar() {
+    assertThat(new DefaultIssueLocation().message("test " + '\u0000' + "123", Collections.emptyList()).message()).isEqualTo("test [NULL]123");
   }
 
   @Test