`--dry-run` generates broken command line when using multiple `--add-opens` in an `[exec]` module

[jnehlmeier]

Jetty version(s) 10.0.11, bug introduced in 10.0.9

Java version/vendor Any that supports --add-opens. Tried with AdoptJdk 15, 18

OS type/version macOs 10.15.7

Description I have a custom module that uses [exec] to set custom JVM parameters. A slightly shortened version of that module looks like

[description]
Custom JVM Parameter

[tags]
jvm

[exec]
-server
-XX:+AlwaysPreTouch
-XX:+PrintFlagsFinal
-XshowSettings:properties
-XX:InitialRAMPercentage=80
-XX:MinRAMPercentage=80
-XX:MaxRAMPercentage=80
-XX:+ExitOnOutOfMemoryError
-Djava.net.preferIPv4Stack=true
-Dnetworkaddress.cache.ttl=0
-Dnetworkaddress.cache.negative.ttl=0
--add-opens
java.base/java.lang=ALL-UNNAMED
--add-opens
java.base/java.util=ALL-UNNAMED
--add-opens
java.base/sun.nio.ch=ALL-UNNAMED
--add-opens
java.desktop/sun.awt=ALL-UNNAMED
--add-opens
java.desktop/sun.java2d=ALL-UNNAMED
--add-opens
jdk.management/com.sun.management.internal=ALL-UNNAMED
--illegal-access=warn
${JAVA_OPTIONS_INHERIT}
${JAVA_OPTIONS_ADDITIONS}

I then use a shell script which uses --dry-run to generate the command (and then expands env variables, but that does not matter here).

The (shortened) generated command now looks like

/Library/Java/JavaVirtualMachines/adoptopenjdk-15.jdk/Contents/Home/bin/java
....
--add-opens
java.base/java.lang=ALL-UNNAMED
java.base/java.util=ALL-UNNAMED
java.base/sun.nio.ch=ALL-UNNAMED
java.desktop/sun.awt=ALL-UNNAMED
java.desktop/sun.java2d=ALL-UNNAMED
jdk.management/com.sun.management.internal=ALL-UNNAMED
--illegal-access=warn
${JAVA_OPTIONS_INHERIT}
${JAVA_OPTIONS_ADDITIONS}
--class-path ... 
org.eclipse.jetty.xml.XmlConfiguration
<properties>
<jetty xml files>

As you can see there is only one --add-opens remaining. Because it only accepts one parameter value java then treats java.base/java.util=ALL-UNNAMED as main class and fails.

[joakime]

The removal of duplicate command line entries seems to be biting you.

What happens if you cheese it in one line instead?

--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/sun.nio.ch=ALL-UNNAMED
--add-opens java.desktop/sun.awt=ALL-UNNAMED
--add-opens java.desktop/sun.java2d=ALL-UNNAMED
--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED

[jnehlmeier]

Nice, that seems to work. The generated command then contains

--add-opens\ java.base/java.lang=ALL-UNNAMED --add-opens\ java.base/java.util=ALL-UNNAMED ...

All parameters are preserved and java accepts it.

I always thought that java would complain that --add-opens\ java.base/java.lang=ALL-UNNAMED isn't a valid parameter because the escaped space causes it to treat the whole thing as one parameter.

[joakime]

It's a cheesy workaround (hack).

I think that when you pass the --dry-run output to java, (via a shell?) the \ (escaped space) is basically always a space.

We need to supporting the jpms style of options better.

[jnehlmeier]

I think that when you pass the --dry-run output to java, (via a shell?) the \ (escaped space) is basically always a space.

Yes, I guess you are right. I am basically doing

CMD=`java -jar $JETTY_HOME/start.jar --dry-run`
# eval triggers variable expansion within generated command line
CMD=$(eval "echo $CMD")
exec $CMD

[joakime]

Here's an alternative declaration, using the [jpms] module.

[description]
Custom JVM Parameter

[tags]
jvm

[exec]
-server
-XX:+AlwaysPreTouch
-XX:+PrintFlagsFinal
-XshowSettings:properties
-XX:InitialRAMPercentage=80
-XX:MinRAMPercentage=80
-XX:MaxRAMPercentage=80
-XX:+ExitOnOutOfMemoryError
-Djava.net.preferIPv4Stack=true
-Dnetworkaddress.cache.ttl=0
-Dnetworkaddress.cache.negative.ttl=0
--illegal-access=warn
${JAVA_OPTIONS_INHERIT}
${JAVA_OPTIONS_ADDITIONS}

[jpms]
add-opens:java.base/java.lang=ALL-UNNAMED
add-opens:java.base/java.util=ALL-UNNAMED
add-opens:java.base/sun.nio.ch=ALL-UNNAMED
add-opens:java.desktop/sun.awt=ALL-UNNAMED
add-opens:java.desktop/sun.java2d=ALL-UNNAMED
add-opens:jdk.management/com.sun.management.internal=ALL-UNNAMED

Documented here - https://github.com/eclipse/jetty.project/blob/jetty-10.0.x/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/modules.adoc

Can you give this a try?

[jnehlmeier]

Here's an alternative declaration, using the [jpms] module.

When I do that and change the command to --jpms --dry-run the generated command line contains all --add-opens without an escaped space (plus some more from other jetty modules). But I am not sure if I will use that, because I never used module-path before with this app.

[joakime]

@sbordet what do you think about having --dry-run output the [jpms] section details even if --jpms isn't used?

[joakime]

Ya know, there's nothing stopping us from having a jpms part on --dry-run=<part>

https://github.com/eclipse/jetty.project/blob/jetty-10.0.x/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt#L67-L73

https://github.com/eclipse/jetty.project/blob/4a6c2744a0d4ff404daee0a521e88ae386356e06/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt#L67-L73

It would introduce a difference between non-dry-run with --jpms and without though.

[sbordet]

@sbordet what do you think about having --dry-run output the [jpms] section details even if --jpms isn't used?

I don't think it's right, and also don't think it's right to have a jpms part.

JPMS is orthogonal to what --dry-run=<part> does, so it should stay as it, behavior wise.

We should fix the handling of --add-opens etc. if they are explicitly added in an [exec] section, but without --jpms. Should not be just a matter of blindly copying those lines to the constructed command line?

[jnehlmeier]

@joakime Not sure if you want to have an extra issue for this opened by I am currently trying to migrate to the official jetty image and while everything seems to work I can't figure out a way to use JPMS style parameters with the official jetty image.

Since the official jetty images passes on JAVA_OPTIONS I thought to just fill that with app specific --add-opens but that fails.

Any idea?

The simplest reproduction is

docker run -it --rm -e JAVA_OPTIONS="--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED" jetty 
Error: --add-opens requires modules to be specified

[jnehlmeier]

Hm I can't even workaround it by creating an my-app.mod file instead with an [exec] block as above and adding that module to jetty.

Basically I can not do

• JAVA_OPTIONS="--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED"
• or as an alternative custom jetty module with [exec] block with
  • --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED on the same line
  • --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED on two lines

These three approaches fail with official jetty image. Maybe because of the changes done in https://github.com/eclipse/jetty.docker/pull/140

[joakime]

BTW, that is a good question to file at the correct project -> https://github.com/eclipse/jetty.docker/

Also, to run the server in jpms mode, the --jpms argument should be passed to the start.jar command. (eg: start.jar --jpms) This is also supported in a ${jetty.base}/start.d/<name>.ini file (just add the --jpms on a line of its own)

[joakime]

If you want to use the custom module approach, use the [jpms] section.

See https://eclipse.dev/jetty/documentation/jetty-10/operations-guide/index.html#og-modules-directive-jpms

Example:

https://github.com/eclipse/jetty.project/blob/30ed83f3d082fc914330de2d5bbde4d0f4b91371/jetty-websocket/websocket-jetty-client/src/main/config/modules/websocket-jetty-client.mod#L20-L23

[jnehlmeier]

Ok did a little debug and here is what happens

FROM jetty:10.0.15-jdk17-eclipse-temurin
ENV JAVA_OPTIONS --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED \
    --add-opens java.base/sun.nio.ch=ALL-UNNAMED \
    --add-opens java.base/jdk.internal.misc=ALL-UNNAMED \
    --add-opens java.base/java.nio=ALL-UNNAMED \
    -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true

generates jetty.start (replaced single space with newline for better readability):

exec
/opt/java/openjdk/bin/java
-Djetty.home=/usr/local/jetty
-Djetty.base=/var/lib/jetty
--add-opens
-Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
--class-path

Only --add-opens without arguments remain. The specified modules are completely gone. This fails with --add-opens requires modules to be specified.

---

A custom module with

[exec]
--add-opens
jdk.management/com.sun.management.internal=ALL-UNNAMED
--add-opens
java.base/sun.nio.ch=ALL-UNNAMED
--add-opens
java.base/jdk.internal.misc=ALL-UNNAMED
--add-opens
java.base/java.nio=ALL-UNNAMED
-Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true

generates jetty.start (replaced single space with newline for better readability):

exec
/opt/java/openjdk/bin/java
-Djetty.home=/usr/local/jetty
-Djetty.base=/var/lib/jetty
--add-opens
jdk.management/com.sun.management.internal=ALL-UNNAMED
java.base/sun.nio.ch=ALL-UNNAMED
java.base/jdk.internal.misc=ALL-UNNAMED
java.base/java.nio=ALL-UNNAMED
-Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
--class-path

A little better but --add-opens has been deduplicated. This fails with java.base/sun.nio.ch=ALL-UNNAMED not being a java class.

---

A custom module with

[exec]
--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED
--add-opens java.base/sun.nio.ch=ALL-UNNAMED
--add-opens java.base/jdk.internal.misc=ALL-UNNAMED
--add-opens java.base/java.nio=ALL-UNNAMED
-Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true

generates jetty.start (replaced single space with newline for better readability):

exec
/opt/java/openjdk/bin/java
-Djetty.home=/usr/local/jetty
-Djetty.base=/var/lib/jetty
'--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED'
'--add-opens java.base/sun.nio.ch=ALL-UNNAMED'
'--add-opens java.base/jdk.internal.misc=ALL-UNNAMED'
'--add-opens java.base/java.nio=ALL-UNNAMED'
-Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
--class-path

Doesn't look too bad but because of the quotes if fails with Unrecognized option: --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED

[joakime]

You cannot use [exec] in combination with --<anything-not-recognized-by-start.jar-as-an-argument-for-itself>

You have to use [jpms] section instead. See comment above https://github.com/eclipse/jetty.project/issues/8348#issuecomment-1665714980

[joakime]

Know that every line in an ini or mod file is a single argument.

The command line --add-opens java.base/jdk.internal.misc=ALL-UNNAMED is two arguments to the Java process.

So that means you have to use 2 lines.

--add-opens
java.base/jdk.internal.misc=ALL-UNNAMED

BUT those single lines are de-duplicated in ini and mod files (always have been), which is what you are hitting.

So an mod (or ini) with the lines ..

[exec]
--foo
--foo
zed
--bar
--foo
yotta

Would wind up being 4 arguments.

1. --foo
2. zed
3. --bar
4. yotta

[jnehlmeier]

@joakime I am not sure how/if it will affect the web application running in jetty. So I tried to avoid using JPMS mode of jetty. Especially because I have a custom jetty module that provides additional appenders for Logback in a jar file without JPMS module-info.

[joakime]

I wonder if having a special named file to support the world of java vms would be worthwhile here?

Something like ${jetty.base}/start.d/jvm-options.args which is just a list of arguments for the JVM in whatever native syntax the JVM needs (so strong quotes for linux/unix/osx on openjdk on sh/bash/zsh, windows quoting for windows jvms, etc). No validation of the contents are performed, they are just used as-is.

[jnehlmeier]

Well shouldn't this be JAVA_OPTIONS? But as shown above it does not work at all for these kind of parameters.

[joakime]

The JAVA_OPTIONS argument can impact the results of --dry-run output. So it was decided to take --dry-run and run it as-is for the exec of java. This means JAVA_OPTIONS was not reused for the second java execution.

IIRC, On the docker image, when you build the image the --dry-run output is stored (this is the step that uses JAVA_OPTIONS properly). When you run that pre-built image it just uses that stored --dry-run (not using JAVA_OPTIONS again for this execution)

[jnehlmeier]

@joakime So I guess two things need to be done:

1. Handle the contents of JAVA_OPTIONS better because currently JPMS parameters are somehow processed but swallowed so that the final --dry-run output, that will be exec, is broken (see above, only a single --add-opens without any parameter remains).
2. Provide a whitelist with valid, repeatable JVM parameters that are not deduplicated, unless their following parameter value is the same as well. This would fix custom modules with [exec] blocks that have JPMS parameters + values on distinct lines.

If I manage to launch Jetty in JPMS mode, will this affect class loading of the web application itself? Must the web application and all its libraries be JPMS compatible then or will WebAppClassLoader work as normal?

[jnehlmeier]

Ok I tried [jpms] now but then the container fails to start because generate-jetty-start.sh does an egrep '[^ ]*java .* org\.eclipse\.jetty\.xml\.XmlConfiguration ' on the dry-run result which results in an empty jetty.start file. With JPMS enabled the dry-run command emits --module org.eclipse.jetty.xml/org.eclipse.jetty.xml.XmlConfiguration which does not match the provided regex. Thus the line is filtered.

I will create a dedicated issue now at the jetty.docker project.

[sbordet]

@lachlan-roberts @joakime can this issue be closed?