JDK-8243936 : NonWriteable system properties are actually writeable
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9,10,11,12,13,14,15
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-04-28
  • Updated: 2024-10-03
  • Resolved: 2020-05-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 15
11.0.20-oracleFixed 15 b25Fixed
Related Reports
Blocks :  
Relates :  
Relates :  
Description
The SystemProperty code used for VM argument parsing was reworked in Java 9 as part of the module system updates. In Java 8 the set_value() method was defined as follows:

bool set_value(char *value) {
    if (writeable()) {
      if (_value != NULL) {
        FreeHeap(_value);
      }
      _value = AllocateHeap(strlen(value)+1, mtInternal);
      if (_value != NULL) {
        strcpy(_value, value);
      }
      return true;
    }
    return false;
  }

so we can only update a writeable flag.

In Java 9 this was refactored so that SystemProperty extends PathString, which contains a simple set_value method that always sets the value:

bool PathString::set_value(const char *value) {
  if (_value != NULL) {
    FreeHeap(_value);
  }
  _value = AllocateHeap(strlen(value)+1, mtArguments);
  assert(_value != NULL, "Unable to allocate space for new path value");
  if (_value != NULL) {
    strcpy(_value, value);
  } else {
    // not able to allocate
    return false;
  }
  return true;
}

and SystemProperty added a new method:

 // A system property should only have its value set
  // via an external interface if it is a writeable property.
  // The internal, non-writeable property jdk.boot.class.path.append
  // is the only exception to this rule.  It can be set externally
  // via -Xbootclasspath/a or JVMTI OnLoad phase call to AddToBootstrapClassLoaderSearch.
  // In those cases for jdk.boot.class.path.append, the base class
  // set_value and append_value methods are called directly.
  bool set_writeable_value(const char *value) {
    if (writeable()) {
      return set_value(value);
    }
    return false;
  }

The intent was obviously that in general set_writeable_value should be used to attempt to set the value, but the general property handling code was not updated:

// This add maintains unique property key in the list.
void Arguments::PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v,
                                        PropertyAppendable append, PropertyWriteable writeable,
                                        PropertyInternal internal) {
  if (plist == NULL)
    return;

  // If property key exist then update with new value.
  SystemProperty* prop;
  for (prop = *plist; prop != NULL; prop = prop->next()) {
    if (strcmp(k, prop->key()) == 0) {
      if (append == AppendProperty) {
        prop->append_value(v);
      } else {
        prop->set_value(v);
      }
      return;
    }
  }

  PropertyList_add(plist, k, v, writeable == WriteableProperty, internal == InternalProperty);
}

Consequently whether the flag is writeable or not is ignored and it will always be appended or set. As a result the user can override non-writable system properties on the command-line e.g:

public class JavaProps {
  public static void main(String[] args) {
    System.out.println(System.getProperty(args[0]));
  }
}

> java9 JavaProps java.vm.name
Java HotSpot(TM) 64-Bit Server VM
> java9 -Djava.vm.name=MyVM JavaProps java.vm.name
MyVM

Property was re-defined by the user! Whereas in Java 8:

> java JavaProps java.vm.name
OpenJDK 64-Bit Server VM
> java -Djava.vm.name=MyVM JavaProps java.vm.name
OpenJDK 64-Bit Server VM

Comments
A pull request was submitted for review. URL: https://git.openjdk.org/jdk11u-dev/pull/1871 Date: 2023-05-08 15:20:06 +0000
08-05-2023

Fix request [11u] I would like to backport this change for parity with Oracle JDK 11.0.20. Fix applies cleanly. Tests (SAP and GHA) showed no issues.
08-05-2023

Related git change (for easier back port to 11.0.20): 686ca5ae490895c5fb42b0234496ee767783d2f8
05-05-2023

URL: https://hg.openjdk.java.net/jdk/jdk/rev/4dc557899b4e User: dholmes Date: 2020-05-28 07:02:20 +0000
28-05-2020

I think I see why JDK-8136930 made the change. The module systems defines a bunch of VM command-line flags that are converted to system properties for conveying to the JDK side. All those properties are created as non-writeable. But some of the flags operate on the "last one wins" principle, which means the property gets overwritten - which won't work if it is not writeable and you use the set_writeable_value() API. So it appears they chose to to use set_value() to workaround this. I'm going to allow the affected property(s) to be defined as writeable in the first place.
21-05-2020

The original modules push actually called set_writeable_value rather than set_value (though still appears broken for the append case), but that was reverted by a follow up change - JDK-8136930 "Simplify use of module-system options by custom launchers". I don't see any discussion of that specific area of the change in the review thread.
28-04-2020