Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enclose system properties containing white space with quotation marks #1958

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

package org.eclipse.m2e.jdt.tests;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -37,6 +38,7 @@
import org.eclipse.m2e.jdt.internal.UnitTestSupport;
import org.eclipse.m2e.jdt.internal.launch.MavenRuntimeClasspathProvider;
import org.eclipse.m2e.tests.common.AbstractMavenProjectTestCase;
import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -53,13 +55,22 @@ public class UnitTestLaunchConfigConfigurationTest extends AbstractMavenProjectT
private static final String ROOT_PATH = "/projects/surefireFailsafeToTestLaunchSettings";
private static ILaunchManager LAUNCH_MANAGER = DebugPlugin.getDefault().getLaunchManager();

/*
* XML allows encoding set of control characters: space (U+0020), carriage
* return (U+000d), line feed (U+000a) and horizontal tab (U+0009).
* https://www.w3.org/TR/xml-entity-names/000.html
*/
private static final String SUREFIRE_ARGS_SET = """
<configuration>
<argLine>
--argLineItem=surefireArgLineValue --undefinedArgLineItem=${undefinedProperty}
</argLine>
<systemPropertyVariables>
<surefireProp1>surefireProp1Value</surefireProp1>
<surefirePropWithSpaces>surefire Prop&#x20;With Spaces</surefirePropWithSpaces>
<surefirePropWithTab>surefirePropWith&#x09;Tab</surefirePropWithTab>
<surefirePropWithCR>has&#x0d;CR</surefirePropWithCR>
<surefirePropWithLF>has&#x0a;LF</surefirePropWithLF>
<surefireEmptyProp>${undefinedProperty}</surefireEmptyProp>
</systemPropertyVariables>
<environmentVariables>
Expand All @@ -76,6 +87,10 @@ public class UnitTestLaunchConfigConfigurationTest extends AbstractMavenProjectT
<systemPropertyVariables>
<failsafeProp1>failsafeProp1Value</failsafeProp1>
<failsafeEmptyProp>${undefiniedProperty}</failsafeEmptyProp>
<failsafePropWithSpaces>failsafe Prop&#x20;With Spaces</failsafePropWithSpaces>
<failsafePropWithTab>failsafePropWith&#x09;Tab</failsafePropWithTab>
<failsafePropWithCR>has&#x0d;CR</failsafePropWithCR>
<failsafePropWithLF>has&#x0a;LF</failsafePropWithLF>
</systemPropertyVariables>
<environmentVariables>
<failsafeEnvironmentVariables1>failsafeEnvironmentVariables1Value</failsafeEnvironmentVariables1>
Expand Down Expand Up @@ -149,6 +164,12 @@ public void test_configuration_must_be_updated_with_surefire_config()
// check systemPropertyVariables
assertTrue(argLine.contains("-DsurefireProp1=surefireProp1Value"));

// check systemPropertyVariables with white space in values have values quoted
assertThat(argLine, Matchers.containsString("-DsurefirePropWithSpaces=\"surefire Prop With Spaces\""));
assertThat(argLine, Matchers.containsString("-DsurefirePropWithTab=\"surefirePropWith\tTab\""));
assertThat(argLine, Matchers.containsString("-DsurefirePropWithCR=\"has\rCR\""));
assertThat(argLine, Matchers.containsString("-DsurefirePropWithLF=\"has\nLF\""));

// check systemPropertyVariables with null value aren't set
assertTrue(!argLine.contains("-DsurefireEmptyProp="));

Expand Down Expand Up @@ -204,6 +225,13 @@ public void test_configuration_must_be_updated_with_failsafe_config()

// check systemPropertyVariables with null value aren't set
assertTrue(!argLine.contains("-DfailsafeEmptyProp="));

// check systemPropertyVariables with white space in values have values quoted
assertThat(argLine, Matchers.containsString("-DfailsafePropWithSpaces=\"failsafe Prop With Spaces\""));
assertThat(argLine, Matchers.containsString("-DfailsafePropWithTab=\"failsafePropWith\tTab\""));
assertThat(argLine, Matchers.containsString("-DfailsafePropWithCR=\"has\rCR\""));
assertThat(argLine, Matchers.containsString("-DfailsafePropWithLF=\"has\nLF\""));

}

@Test
Expand Down Expand Up @@ -246,6 +274,13 @@ public void test_configuration_must_be_updated_with_surefire_config_when_created

// check systemPropertyVariables
assertTrue(argLine.contains("-DsurefireProp1=surefireProp1Value"));

// check systemPropertyVariables with white space in values have values quoted
assertThat(argLine, Matchers.containsString("-DsurefirePropWithSpaces=\"surefire Prop With Spaces\""));
assertThat(argLine, Matchers.containsString("-DsurefirePropWithTab=\"surefirePropWith\tTab\""));
assertThat(argLine, Matchers.containsString("-DsurefirePropWithCR=\"has\rCR\""));
assertThat(argLine, Matchers.containsString("-DsurefirePropWithLF=\"has\nLF\""));

}

@Test
Expand Down Expand Up @@ -288,6 +323,13 @@ public void test_configuration_must_be_updated_with_failSafe_config_when_created

// check systemPropertyVariables
assertTrue(argLine.contains("-DfailsafeProp1=failsafeProp1Value"));

// check systemPropertyVariables with white space in values have values quoted
assertThat(argLine, Matchers.containsString("-DfailsafePropWithSpaces=\"failsafe Prop With Spaces\""));
assertThat(argLine, Matchers.containsString("-DfailsafePropWithTab=\"failsafePropWith\tTab\""));
assertThat(argLine, Matchers.containsString("-DfailsafePropWithCR=\"has\rCR\""));
assertThat(argLine, Matchers.containsString("-DfailsafePropWithLF=\"has\nLF\""));

}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ private void defineConfigurationValues(IProject project, ILaunchConfiguration co
if(args.systemPropertyVariables() != null) {
args.systemPropertyVariables().entrySet().stream() //
.filter(e -> e.getKey() != null && e.getValue() != null)
.forEach(e -> launchArguments.add("-D" + e.getKey() + "=" + e.getValue()));
.forEach(e -> launchArguments.add("-D" + e.getKey() + "=" + escapeValue(e.getValue())));
}
copy.setAttribute(LAUNCH_CONFIG_VM_ARGUMENTS, launchArguments.toString());

Expand All @@ -306,6 +306,13 @@ private void defineConfigurationValues(IProject project, ILaunchConfiguration co
copy.doSave();
}

private static String escapeValue(String raw) {
if(raw.contains(" ") || raw.contains("\t") || raw.contains("\r") || raw.contains("\n")) {
return "\"" + raw + "\"";
}
return raw;
}

private TestLaunchArguments getTestLaunchArguments(ILaunchConfiguration configuration, IMavenProjectFacade facade,
IProgressMonitor monitor) throws CoreException {

Expand Down