[jdk8u-dev] RFR: 8288483: JAVA_TOOL_OPTIONS are silently truncated if they exceed 1024 bytes [v2]

[Andrew John Hughes]

On Fri, 17 Jun 2022 13:08:58 GMT, Dmitry Samersoff <dsamersoff at openjdk.org> wrote:

>> The JAVA_TOOL_OPTIONS environment variable is used to pass additional JVM arguments.
>> 
>> The current implementation in jdk8 has an internal limit on the length of the option variable (1024 bytes) and the number of options (64).
>> 
>> A longer variable will be silently truncated and some options will be lost or the VM will exist with an unrecognized option error.
>> 
>> The fix is not a direct backport of the changes in the later JDKs, but I kept the code as close as possible to what we have in the latest jdk.
>> 
>> It's partial backport of relevant changes from JDK-8135195, JDK-8074895, JDK-8061999
>
> Dmitry Samersoff has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8288483: JAVA_TOOL_OPTIONS are silently truncated if they exceed 1024 bytes

I tend to agree with Severin here.  Generally, enhancements shouldn't be backported.

Also, I don't see why this is being introduced under a new bug ID, and this is effectively hiding two enhancements inside a bug. The relevant fix for the 1024 limit is [JDK-8135195](https://bugs.openjdk.org/browse/JDK-8135195), but that issue is referring to a limit on an options file, which is not supported in 8u, as it doesn't have [JDK-8061999](https://bugs.openjdk.org/browse/JDK-8061999).  This patch doesn't backport the options file support, so is effectively treading new ground in lifting the limit just for the environment variable, but with most of the code changes from the aforementioned bugs.

I think they may be a case for a simpler fix that improves `os::getenv` so it warns when the environment variable passed is greater than the buffer length (probably only when `Verbose` is on). However, I don't see the case mentioned in the title of this PR; the variable is not truncated, it is simply ignored in its entirety if it is too long ("The variable will be ignored if it exceeds the length of the buffer.")

>From `hotspot/src/os/linux/vm/os_linux.cpp`:

~~~
bool os::getenv(const char* name, char* buf, int len) {
  const char* val = ::getenv(name);
  if (val != NULL && strlen(val) < (size_t)len) {
    strcpy(buf, val);
    return true;
  }
  if (len > 0) buf[0] = 0;  // return a null string                                                                                                         
  return false;
}
~~~

So `os::getenv` will return `false`, just as it would if the environment variable was not defined.

-------------

PR: https://git.openjdk.org/jdk8u-dev/pull/74