SONAR−9772 fail when changing mandatory JVM options through properties

5 files changed, 397 insertions, 83 deletions

diff --git a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/CeJvmOptions.java b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/CeJvmOptions.java
index 4b0fc9620eb..b9b4ff0b2cc 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/CeJvmOptions.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/CeJvmOptions.java
@@ -20,14 +20,19 @@
 package org.sonar.process.jmvoptions;
 
 import java.io.File;
-
-import static java.lang.String.format;
+import java.util.LinkedHashMap;
+import java.util.Map;
 
 public class CeJvmOptions extends JvmOptions<CeJvmOptions> {
-  private static final String[] MANDATORY_JVM_OPTIONS = {"-Djava.awt.headless=true", "-Dfile.encoding=UTF-8"};
-
   public CeJvmOptions(File tmpDir) {
-    super(MANDATORY_JVM_OPTIONS);
-    add(format("-Djava.io.tmpdir=%s", tmpDir.getAbsolutePath()));
+    super(mandatoryOptions(tmpDir));
+  }
+
+  private static Map<String, String> mandatoryOptions(File tmpDir) {
+    Map<String, String> res = new LinkedHashMap<>(3);
+    res.put("-Djava.awt.headless=", "true");
+    res.put("-Dfile.encoding=", "UTF-8");
+    res.put("-Djava.io.tmpdir=", tmpDir.getAbsolutePath());
+    return res;
   }
 }
diff --git a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/EsJvmOptions.java b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/EsJvmOptions.java
index 1c823e97056..61760016d2e 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/EsJvmOptions.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/EsJvmOptions.java
@@ -23,27 +23,11 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 public class EsJvmOptions extends JvmOptions<EsJvmOptions> {
-  private static final String[] MANDATORY_OPTIONS = {
-    "-XX:+UseConcMarkSweepGC",
-    "-XX:CMSInitiatingOccupancyFraction=75",
-    "-XX:+UseCMSInitiatingOccupancyOnly",
-    "-XX:+AlwaysPreTouch",
-    "-server",
-    "-Xss1m",
-    "-Djava.awt.headless=true",
-    "-Dfile.encoding=UTF-8",
-    "-Djna.nosys=true",
-    "-Djdk.io.permissionsUseCanonicalPath=true",
-    "-Dio.netty.noUnsafe=true",
-    "-Dio.netty.noKeySetOptimization=true",
-    "-Dio.netty.recycler.maxCapacityPerThread=0",
-    "-Dlog4j.shutdownHookEnabled=false",
-    "-Dlog4j2.disable.jmx=true",
-    "-Dlog4j.skipJansi=true"
-  };
   private static final String ELASTICSEARCH_JVM_OPTIONS_HEADER = "# This file has been automatically generated by SonarQube during startup.\n" +
     "# Please use the sonar.search.javaOpts in sonar.properties to specify jvm options for Elasticsearch\n" +
     "\n" +
@@ -51,7 +35,28 @@ public class EsJvmOptions extends JvmOptions<EsJvmOptions> {
     "\n";
 
   public EsJvmOptions() {
-    super(MANDATORY_OPTIONS);
+    super(mandatoryOptions());
+  }
+
+  private static Map<String, String> mandatoryOptions() {
+    Map<String, String> res = new LinkedHashMap<>(16);
+    res.put("-XX:+UseConcMarkSweepGC", "");
+    res.put("-XX:CMSInitiatingOccupancyFraction=", "75");
+    res.put("-XX:+UseCMSInitiatingOccupancyOnly", "");
+    res.put("-XX:+AlwaysPreTouch", "");
+    res.put("-server", "");
+    res.put("-Xss", "1m");
+    res.put("-Djava.awt.headless=", "true");
+    res.put("-Dfile.encoding=", "UTF-8");
+    res.put("-Djna.nosys=", "true");
+    res.put("-Djdk.io.permissionsUseCanonicalPath=", "true");
+    res.put("-Dio.netty.noUnsafe=", "true");
+    res.put("-Dio.netty.noKeySetOptimization=", "true");
+    res.put("-Dio.netty.recycler.maxCapacityPerThread=", "0");
+    res.put("-Dlog4j.shutdownHookEnabled=", "false");
+    res.put("-Dlog4j2.disable.jmx=", "true");
+    res.put("-Dlog4j.skipJansi=", "true");
+    return res;
   }
 
   public void writeToJvmOptionFile(File file) {
diff --git a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/JvmOptions.java b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/JvmOptions.java
index 15cc411273d..13560f9c50f 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/JvmOptions.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/JvmOptions.java
@@ -21,31 +21,90 @@ package org.sonar.process.jmvoptions;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
+import org.sonar.process.MessageException;
 import org.sonar.process.Props;
 
+import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.joining;
 
 public class JvmOptions<T extends JvmOptions> {
   private static final String JVM_OPTION_NOT_NULL_ERROR_MESSAGE = "a JVM option can't be null";
 
+  private final HashMap<String, String> mandatoryOptions = new HashMap<>();
   private final LinkedHashSet<String> options = new LinkedHashSet<>();
 
-  public JvmOptions(String... mandatoryJvmOptions) {
-    Arrays.stream(requireNonNull(mandatoryJvmOptions, JVM_OPTION_NOT_NULL_ERROR_MESSAGE))
-      .forEach(this::add);
+  public JvmOptions() {
+    this(Collections.emptyMap());
+  }
+
+  public JvmOptions(Map<String, String> mandatoryJvmOptions) {
+    requireNonNull(mandatoryJvmOptions, JVM_OPTION_NOT_NULL_ERROR_MESSAGE)
+      .entrySet()
+      .stream()
+      .filter(e -> {
+        requireNonNull(e.getKey(), "JVM option prefix can't be null");
+        if (e.getKey().trim().isEmpty()) {
+          throw new IllegalArgumentException("JVM option prefix can't be empty");
+        }
+        requireNonNull(e.getValue(), "JVM option value can't be null");
+        return true;
+      }).forEach(e -> {
+        String key = e.getKey().trim();
+        String value = e.getValue().trim();
+        mandatoryOptions.put(key, value);
+        add(key + value);
+      });
   }
 
   public T addFromMandatoryProperty(Props props, String propertyName) {
     String value = props.nonNullValue(propertyName);
     if (!value.isEmpty()) {
-      Arrays.stream(value.split(" (?=-)")).forEach(this::add);
+      List<String> jvmOptions = Arrays.stream(value.split(" (?=-)")).map(String::trim).collect(Collectors.toList());
+      checkOptionFormat(propertyName, jvmOptions);
+      checkMandatoryOptionOverwrite(propertyName, jvmOptions);
+      options.addAll(jvmOptions);
     }
 
     return castThis();
   }
 
+  private static void checkOptionFormat(String propertyName, List<String> jvmOptionsFromProperty) {
+    List<String> invalidOptions = jvmOptionsFromProperty.stream()
+      .filter(JvmOptions::isInvalidOption)
+      .collect(Collectors.toList());
+    if (!invalidOptions.isEmpty()) {
+      throw new MessageException(format(
+        "a JVM option can't be empty and must start with '-'. The following JVM options defined by property '%s' are invalid: %s",
+        propertyName,
+        invalidOptions.stream()
+          .collect(joining(", "))));
+    }
+  }
+
+  private void checkMandatoryOptionOverwrite(String propertyName, List<String> jvmOptionsFromProperty) {
+    List<Match> matches = jvmOptionsFromProperty.stream()
+      .map(jvmOption -> new Match(jvmOption, mandatoryOptionFor(jvmOption)))
+      .filter(match -> match.getMandatoryOption() != null)
+      .collect(Collectors.toList());
+    if (!matches.isEmpty()) {
+      throw new MessageException(format(
+        "a JVM option can't overwrite mandatory JVM options. The following JVM options defined by property '%s' are invalid: %s",
+        propertyName,
+        matches.stream()
+          .map(m -> m.getOption() + " overwrites " + m.mandatoryOption.getKey() + m.mandatoryOption.getValue())
+          .collect(joining(", "))));
+    }
+  }
+
   /**
    * Add an option.
    * Argument is trimmed before being added.
@@ -55,14 +114,37 @@ public class JvmOptions<T extends JvmOptions> {
   public T add(String str) {
     requireNonNull(str, JVM_OPTION_NOT_NULL_ERROR_MESSAGE);
     String value = str.trim();
-    if (value.isEmpty() || !value.startsWith("-")) {
+    if (isInvalidOption(value)) {
       throw new IllegalArgumentException("a JVM option can't be empty and must start with '-'");
     }
+    checkMandatoryOptionOverwrite(value);
     options.add(value);
 
     return castThis();
   }
 
+  private void checkMandatoryOptionOverwrite(String value) {
+    Map.Entry<String, String> overriddenMandatoryOption = mandatoryOptionFor(value);
+    if (overriddenMandatoryOption != null) {
+      throw new MessageException(String.format(
+        "a JVM option can't overwrite mandatory JVM options. %s overwrites %s",
+        value,
+        overriddenMandatoryOption.getKey() + overriddenMandatoryOption.getValue()));
+    }
+  }
+
+  @CheckForNull
+  private Map.Entry<String, String> mandatoryOptionFor(String jvmOption) {
+    return mandatoryOptions.entrySet().stream()
+      .filter(s -> jvmOption.startsWith(s.getKey()) && !jvmOption.equals(s.getKey() + s.getValue()))
+      .findFirst()
+      .orElse(null);
+  }
+
+  private static boolean isInvalidOption(String value) {
+    return value.isEmpty() || !value.startsWith("-");
+  }
+
   @SuppressWarnings("unchecked")
   private T castThis() {
     return (T) this;
@@ -76,4 +158,25 @@ public class JvmOptions<T extends JvmOptions> {
   public String toString() {
     return options.toString();
   }
+
+  private static final class Match {
+    private final String option;
+
+    private final Map.Entry<String, String> mandatoryOption;
+
+    private Match(String option, @Nullable Map.Entry<String, String> mandatoryOption) {
+      this.option = option;
+      this.mandatoryOption = mandatoryOption;
+    }
+
+    String getOption() {
+      return option;
+    }
+
+    @CheckForNull
+    Map.Entry<String, String> getMandatoryOption() {
+      return mandatoryOption;
+    }
+
+  }
 }
diff --git a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/WebJvmOptions.java b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/WebJvmOptions.java
index dda9856e613..a2a3f8f2d04 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/WebJvmOptions.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/jmvoptions/WebJvmOptions.java
@@ -20,14 +20,19 @@
 package org.sonar.process.jmvoptions;
 
 import java.io.File;
-
-import static java.lang.String.format;
+import java.util.LinkedHashMap;
+import java.util.Map;
 
 public class WebJvmOptions extends JvmOptions<WebJvmOptions> {
-  private static final String[] MANDATORY_OPTIONS = {"-Djava.awt.headless=true", "-Dfile.encoding=UTF-8"};
-
   public WebJvmOptions(File tmpDir) {
-    super(MANDATORY_OPTIONS);
-    add(format("-Djava.io.tmpdir=%s", tmpDir.getAbsolutePath()));
+    super(mandatoryOptions(tmpDir));
+  }
+
+  private static Map<String, String> mandatoryOptions(File tmpDir) {
+    Map<String, String> res = new LinkedHashMap<>(3);
+    res.put("-Djava.awt.headless=", "true");
+    res.put("-Dfile.encoding=", "UTF-8");
+    res.put("-Djava.io.tmpdir=", tmpDir.getAbsolutePath());
+    return res;
   }
 }
diff --git a/server/sonar-process/src/test/java/org/sonar/process/jmvoptions/JvmOptionsTest.java b/server/sonar-process/src/test/java/org/sonar/process/jmvoptions/JvmOptionsTest.java
index 7e42d307d68..84d54ed339e 100644
--- a/server/sonar-process/src/test/java/org/sonar/process/jmvoptions/JvmOptionsTest.java
+++ b/server/sonar-process/src/test/java/org/sonar/process/jmvoptions/JvmOptionsTest.java
@@ -19,22 +19,32 @@
  */
 package org.sonar.process.jmvoptions;
 
+import com.google.common.collect.ImmutableMap;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
-import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
 import java.util.Properties;
 import java.util.Random;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
-import org.apache.commons.lang.RandomStringUtils;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
+import org.sonar.process.MessageException;
 import org.sonar.process.Props;
 
+import static java.lang.String.valueOf;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 
 @RunWith(DataProviderRunner.class)
 public class JvmOptionsTest {
@@ -42,7 +52,9 @@ public class JvmOptionsTest {
   public ExpectedException expectedException = ExpectedException.none();
 
   private final Random random = new Random();
-  private final String randomPropertyName = RandomStringUtils.randomAlphanumeric(3);
+  private final String randomPropertyName = randomAlphanumeric(3);
+  private final String randomPrefix = "-" + randomAlphabetic(5).toLowerCase(Locale.ENGLISH);
+  private final String randomValue = randomAlphanumeric(4).toLowerCase(Locale.ENGLISH);
   private final Properties properties = new Properties();
   private final Props props = new Props(properties);
   private final JvmOptions underTest = new JvmOptions();
@@ -62,65 +74,72 @@ public class JvmOptionsTest {
   }
 
   @Test
-  public void constructor_throws_NPE_if_any_argument_is_null() {
-    ArrayList<String> nullList = new ArrayList<>();
-    nullList.add(null);
-    String[] arguments = Stream.of(
-      Stream.of("-S1"),
-      IntStream.range(0, random.nextInt(2)).mapToObj(i -> "-B" + i),
-      nullList.stream(),
-      IntStream.range(0, random.nextInt(2)).mapToObj(i -> "-A" + i)).flatMap(s -> s)
-      .toArray(String[]::new);
+  public void constructor_throws_NPE_if_any_option_prefix_is_null() {
+    Map<String, String> mandatoryJvmOptions = shuffleThenToMap(
+      Stream.of(
+        IntStream.range(0, random.nextInt(10)).mapToObj(i -> new Option("-B", valueOf(i))),
+        Stream.of(new Option(null, "value")))
+        .flatMap(s -> s));
 
-    expectJvmOptionNotNullNPE();
+    expectedException.expect(NullPointerException.class);
+    expectedException.expectMessage("JVM option prefix can't be null");
 
-    new JvmOptions(arguments);
+    new JvmOptions(mandatoryJvmOptions);
   }
 
   @Test
   @UseDataProvider("variousEmptyStrings")
-  public void constructor_throws_IAE_if_argument_is_empty(String emptyString) {
-    expectJvmOptionNotEmptyAndStartByDashIAE();
+  public void constructor_throws_IAE_if_any_option_prefix_is_empty(String emptyString) {
+    Map<String, String> mandatoryJvmOptions = shuffleThenToMap(
+      Stream.of(
+        IntStream.range(0, random.nextInt(10)).mapToObj(i -> new Option("-B", valueOf(i))),
+        Stream.of(new Option(emptyString, "value")))
+        .flatMap(s -> s));
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("JVM option prefix can't be empty");
 
-    new JvmOptions(emptyString);
+    new JvmOptions(mandatoryJvmOptions);
   }
 
   @Test
-  @UseDataProvider("variousEmptyStrings")
-  public void constructor_throws_IAE_if_any_argument_is_empty(String emptyString) {
-    String[] arguments = Stream.of(
-      Stream.of("-S1"),
-      IntStream.range(0, random.nextInt(2)).mapToObj(i -> "-B" + i),
-      Stream.of(emptyString),
-      IntStream.range(0, random.nextInt(2)).mapToObj(i -> "-A" + i))
-      .flatMap(s -> s)
-      .toArray(String[]::new);
+  public void constructor_throws_IAE_if_any_option_prefix_does_not_start_with_dash() {
+    String invalidPrefix = randomAlphanumeric(3);
+    Map<String, String> mandatoryJvmOptions = shuffleThenToMap(
+      Stream.of(
+        IntStream.range(0, random.nextInt(10)).mapToObj(i -> new Option("-B", valueOf(i))),
+        Stream.of(new Option(invalidPrefix, "value")))
+        .flatMap(s -> s));
 
     expectJvmOptionNotEmptyAndStartByDashIAE();
 
-    new JvmOptions(arguments);
+    new JvmOptions(mandatoryJvmOptions);
   }
 
   @Test
-  public void constructor_throws_IAE_if_argument_does_not_start_with_dash() {
-    expectJvmOptionNotEmptyAndStartByDashIAE();
+  public void constructor_throws_NPE_if_any_option_value_is_null() {
+    Map<String, String> mandatoryJvmOptions = shuffleThenToMap(
+      Stream.of(
+        IntStream.range(0, random.nextInt(10)).mapToObj(i -> new Option("-B", valueOf(i))),
+        Stream.of(new Option("-prefix", null)))
+        .flatMap(s -> s));
+
+    expectedException.expect(NullPointerException.class);
+    expectedException.expectMessage("JVM option value can't be null");
 
-    new JvmOptions(RandomStringUtils.randomAlphanumeric(3));
+    new JvmOptions(mandatoryJvmOptions);
   }
 
   @Test
-  public void constructor_throws_IAE_if_any_argument_does_not_start_with_dash() {
-    String[] arguments = Stream.of(
-      Stream.of("-S1"),
-      IntStream.range(0, random.nextInt(2)).mapToObj(i -> "-B" + i),
-      Stream.of(RandomStringUtils.randomAlphanumeric(3)),
-      IntStream.range(0, random.nextInt(2)).mapToObj(i -> "-A" + i))
-      .flatMap(s -> s)
-      .toArray(String[]::new);
-
-    expectJvmOptionNotEmptyAndStartByDashIAE();
-
-    new JvmOptions(arguments);
+  @UseDataProvider("variousEmptyStrings")
+  public void constructor_accepts_any_empty_option_value(String emptyString) {
+    Map<String, String> mandatoryJvmOptions = shuffleThenToMap(
+      Stream.of(
+        IntStream.range(0, random.nextInt(10)).mapToObj(i -> new Option("-B", valueOf(i))),
+        Stream.of(new Option("-prefix", emptyString)))
+        .flatMap(s -> s));
+
+    new JvmOptions(mandatoryJvmOptions);
   }
 
   @Test
@@ -142,7 +161,69 @@ public class JvmOptionsTest {
   public void add_throws_IAE_if_argument_does_not_start_with_dash() {
     expectJvmOptionNotEmptyAndStartByDashIAE();
 
-    underTest.add(RandomStringUtils.randomAlphanumeric(3));
+    underTest.add(randomAlphanumeric(3));
+  }
+
+  @Test
+  @UseDataProvider("variousEmptyStrings")
+  public void add_adds_with_trimming(String emptyString) {
+    underTest.add(emptyString + "-foo" + emptyString);
+
+    assertThat(underTest.getAll()).containsOnly("-foo");
+  }
+
+  @Test
+  public void add_throws_MessageException_if_option_starts_with_prefix_of_mandatory_option_but_has_different_value() {
+    String[] optionOverrides = {
+      randomPrefix,
+      randomPrefix + randomAlphanumeric(1),
+      randomPrefix + randomAlphanumeric(2),
+      randomPrefix + randomAlphanumeric(3),
+      randomPrefix + randomAlphanumeric(4),
+      randomPrefix + randomValue.substring(1),
+      randomPrefix + randomValue.substring(2),
+      randomPrefix + randomValue.substring(3)
+    };
+
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    for (String optionOverride : optionOverrides) {
+      try {
+        underTest.add(optionOverride);
+        fail("an MessageException should have been thrown");
+      } catch (MessageException e) {
+        assertThat(e.getMessage()).isEqualTo("a JVM option can't overwrite mandatory JVM options. " + optionOverride + " overwrites " + randomPrefix + randomValue);
+      }
+    }
+  }
+
+  @Test
+  public void add_checks_against_mandatory_options_is_case_sensitive() {
+    String[] optionOverrides = {
+      randomPrefix,
+      randomPrefix + randomAlphanumeric(1),
+      randomPrefix + randomAlphanumeric(2),
+      randomPrefix + randomAlphanumeric(3),
+      randomPrefix + randomAlphanumeric(4),
+      randomPrefix + randomValue.substring(1),
+      randomPrefix + randomValue.substring(2),
+      randomPrefix + randomValue.substring(3)
+    };
+
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    for (String optionOverride : optionOverrides) {
+      underTest.add(optionOverride.toUpperCase(Locale.ENGLISH));
+    }
+  }
+
+  @Test
+  public void add_accepts_property_equal_to_mandatory_option_and_does_not_add_it_twice() {
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    underTest.add(randomPrefix + randomValue);
+
+    assertThat(underTest.getAll()).containsOnly(randomPrefix + randomValue);
   }
 
   @Test
@@ -153,8 +234,7 @@ public class JvmOptionsTest {
   }
 
   @Test
-  @UseDataProvider("variousEmptyStrings")
-  public void addFromMandatoryProperty_fails_with_IAE_if_property_contains_an_empty_value(String emptyString) {
+  public void addFromMandatoryProperty_fails_with_IAE_if_property_contains_an_empty_value() {
     expectMissingPropertyIAE(this.randomPropertyName);
 
     underTest.addFromMandatoryProperty(props, randomPropertyName);
@@ -172,10 +252,10 @@ public class JvmOptionsTest {
 
   @Test
   @UseDataProvider("variousEmptyStrings")
-  public void addFromMandatoryProperty_fails_with_IAE_if_property_does_not_start_with_dash_after_trimmed(String emptyString) {
+  public void addFromMandatoryProperty_fails_with_MessageException_if_property_does_not_start_with_dash_after_trimmed(String emptyString) {
     properties.put(randomPropertyName, emptyString + "foo -bar");
 
-    expectJvmOptionNotEmptyAndStartByDashIAE();
+    expectJvmOptionNotEmptyAndStartByDashMessageException(randomPropertyName, "foo");
 
     underTest.addFromMandatoryProperty(props, randomPropertyName);
   }
@@ -200,6 +280,83 @@ public class JvmOptionsTest {
   }
 
   @Test
+  public void addFromMandatoryProperty_throws_IAE_if_option_starts_with_prefix_of_mandatory_option_but_has_different_value() {
+    String[] optionOverrides = {
+      randomPrefix,
+      randomPrefix + randomValue.substring(1),
+      randomPrefix + randomValue.substring(1),
+      randomPrefix + randomValue.substring(2),
+      randomPrefix + randomValue.substring(3),
+      randomPrefix + randomValue.substring(3) + randomAlphanumeric(1),
+      randomPrefix + randomValue.substring(3) + randomAlphanumeric(2),
+      randomPrefix + randomValue.substring(3) + randomAlphanumeric(3),
+      randomPrefix + randomValue + randomAlphanumeric(1)
+    };
+
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    for (String optionOverride : optionOverrides) {
+      try {
+        properties.put(randomPropertyName, optionOverride);
+        underTest.addFromMandatoryProperty(props, randomPropertyName);
+        fail("an MessageException should have been thrown");
+      } catch (MessageException e) {
+        assertThat(e.getMessage())
+          .isEqualTo("a JVM option can't overwrite mandatory JVM options. " +
+            "The following JVM options defined by property '" + randomPropertyName + "' are invalid: " + optionOverride + " overwrites " + randomPrefix + randomValue);
+      }
+    }
+  }
+
+  @Test
+  public void addFromMandatoryProperty_checks_against_mandatory_options_is_case_sensitive() {
+    String[] optionOverrides = {
+      randomPrefix,
+      randomPrefix + randomValue.substring(1),
+      randomPrefix + randomValue.substring(1),
+      randomPrefix + randomValue.substring(2),
+      randomPrefix + randomValue.substring(3),
+      randomPrefix + randomValue.substring(3) + randomAlphanumeric(1),
+      randomPrefix + randomValue.substring(3) + randomAlphanumeric(2),
+      randomPrefix + randomValue.substring(3) + randomAlphanumeric(3),
+      randomPrefix + randomValue + randomAlphanumeric(1)
+    };
+
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    for (String optionOverride : optionOverrides) {
+      properties.setProperty(randomPropertyName, optionOverride.toUpperCase(Locale.ENGLISH));
+      underTest.addFromMandatoryProperty(props, randomPropertyName);
+    }
+  }
+
+  @Test
+  public void addFromMandatoryProperty_reports_all_overriding_options_in_single_exception() {
+    String overriding1 = randomPrefix;
+    String overriding2 = randomPrefix + randomValue + randomAlphanumeric(1);
+    properties.setProperty(randomPropertyName, "-foo " + overriding1 + " -bar " + overriding2);
+
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("a JVM option can't overwrite mandatory JVM options. " +
+      "The following JVM options defined by property '" + randomPropertyName + "' are invalid: " +
+      overriding1 + " overwrites " + randomPrefix + randomValue + ", " + overriding2 + " overwrites " + randomPrefix + randomValue);
+
+    underTest.addFromMandatoryProperty(props, randomPropertyName);
+  }
+
+  @Test
+  public void addFromMandatoryProperty_accepts_property_equal_to_mandatory_option_and_does_not_add_it_twice() {
+    JvmOptions underTest = new JvmOptions(ImmutableMap.of(randomPrefix, randomValue));
+
+    properties.put(randomPropertyName, randomPrefix + randomValue);
+    underTest.addFromMandatoryProperty(props, randomPropertyName);
+
+    assertThat(underTest.getAll()).containsOnly(randomPrefix + randomValue);
+  }
+
+  @Test
   public void toString_prints_all_jvm_options() {
     underTest.add("-foo").add("-bar");
 
@@ -216,6 +373,12 @@ public class JvmOptionsTest {
     expectedException.expectMessage("a JVM option can't be empty and must start with '-'");
   }
 
+  private void expectJvmOptionNotEmptyAndStartByDashMessageException(String randomPropertyName, String option) {
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("a JVM option can't be empty and must start with '-'. " +
+      "The following JVM options defined by property '" + randomPropertyName + "' are invalid: " + option);
+  }
+
   public void expectMissingPropertyIAE(String randomPropertyName) {
     expectedException.expect(IllegalArgumentException.class);
     expectedException.expectMessage("Missing property: " + randomPropertyName);
@@ -229,4 +392,37 @@ public class JvmOptionsTest {
       {"     "}
     };
   }
+
+  private static Map<String, String> shuffleThenToMap(Stream<Option> stream) {
+    List<Option> options = stream.collect(Collectors.toList());
+    Collections.shuffle(options);
+    Map<String, String> res = new HashMap<>(options.size());
+    for (Option option : options) {
+      res.put(option.getPrefix(), option.getValue());
+    }
+    return res;
+  }
+
+  private static final class Option {
+    private final String prefix;
+    private final String value;
+
+    private Option(String prefix, String value) {
+      this.prefix = prefix;
+      this.value = value;
+    }
+
+    public String getPrefix() {
+      return prefix;
+    }
+
+    public String getValue() {
+      return value;
+    }
+
+    @Override
+    public String toString() {
+      return "[" + prefix + "-" + value + ']';
+    }
+  }
 }