From: Guillaume Jambet Date: Wed, 6 Jun 2018 12:43:17 +0000 (+0200) Subject: SONAR-10460 do not allow null character in scanner for issue message X-Git-Tag: 7.5~1061 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6f6f00faf62ea79215031534671bc4f1d9a55856;p=sonarqube.git 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 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,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 customMatcher(String startWith, String endWith) { + return new TypeSafeMatcher() { + @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 e369330a291..3112caf8eac 100644 --- 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);