Issue #9309 - Better jetty.sh integration for start.jar with eye on supporting odd properties

[joakime]

When bridging output from start.jar --dry-run to the RUN_CMD in jetty.sh weird properties with various special characters and spaces would mess up the command line that eventually gets executed.

A change to start.jar --dry-run has been made to wrap individual args that have spaces with ' (single-tick quote), and a change in jetty.sh to separate the various args produced by the --dry-run properly for the execution of java and eventually the call to XmlConfiguration.

A minor update to CustomRequestLog exception messages has been pushed here as well.

Signed-off-by: Joakim Erdfelt joakim.erdfelt@gmail.com

[gregw]

LGTM

[jmcc0nn3ll]

I think the usage of xargs in the jetty.sh script should be fine, but if there is a more strictly bash solution available, it may make sense to investigate that down the road if some reports an issue.

[gregw]

a couple of quick comments. I need to review in more depth once these are resolved.

[amjaliks]

With these changes my 10.0.x fails to start as a services. Even before I try to set 'an odd property' (a custom request log format).

A stacktrace:

2023-02-18 13:30:13.328:WARN :oejx.XmlConfiguration:main: Unable to execute XmlConfiguration
java.security.PrivilegedActionException: java.security.UnrecoverableKeyException: Get Key failed: Cannot read the array length because "password" is null
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:573)
	at org.eclipse.jetty.xml.XmlConfiguration.main(XmlConfiguration.java:1818)
Caused by: 
java.security.UnrecoverableKeyException: Get Key failed: Cannot read the array length because "password" is null
	at java.base/sun.security.pkcs12.PKCS12KeyStore.engineGetKey(PKCS12KeyStore.java:450)
	at java.base/sun.security.util.KeyStoreDelegator.engineGetKey(KeyStoreDelegator.java:91)

The same configuration starts normally with 10.0.13.

[joakime]

@amjaliks can you include more of the stacktrace on that exception?
It will help me setup a similar jetty.base to you

[amjaliks]

@joakime I got this fixed by using xargs for calling start-stop-deamon in jetty.sh.

I replaced this (starting line 467):

      start-stop-daemon --start $CH_USER \
       --pidfile "$JETTY_PID" \
       --chdir "$JETTY_BASE" \
       --background \
       --make-pidfile \
       --startas "$JAVA" \
       -- ${RUN_ARGS[@]} start-log-file="$JETTY_START_LOG"

With this:

      echo ${RUN_CMD[*]} start-log-file="$JETTY_START_LOG" | xargs start-stop-daemon --start $CH_USER \
       --pidfile "$JETTY_PID" \
       --chdir "$JETTY_BASE" \
       --background \
       --make-pidfile \
       --startas "$JAVA" \
       --

End even more. I applied similar changes to jetty.sh from 10.0.13 and 11.0.13:

      echo $JETTY_SYS_PROPS ${RUN_ARGS[@]} start-log-file="$JETTY_START_LOG" | xargs start-stop-daemon --start $CH_USER \
       --pidfile "$JETTY_PID" \
       --chdir "$JETTY_BASE" \
       --background \
       --make-pidfile \
       --startas "$JAVA" \
       --

And this fixed #9309.

[amjaliks]

@joakime You need to update lines 586 and 599, otherwise actions supervise, run, and demo will be broken.

[amjaliks]

This PR has missed 10.0.14. How about 10.0.15?

[joakime]

@amjaliks have you tested this branch on your environment?
@gregw are you ok with this being in Jetty 10.0.15 release? (if so, I need an approval to merge this)

[github-advanced-security]

You have successfully added a new CodeQL configuration .github/workflows/codeql-analysis.yml:analyze/languages:java. As part of the setup process, we have scanned this repository and found 159 existing alerts. Please check the repository Security tab to see all alerts.

[joakime]

I've found a few problems with this branch on Windows that needs to be addressed.

Stale review

[joakime]

I have finished my testing on Windows.

Other than the usual situation with overly long command lines, this PR appears to be working as intended on the Windows platform.

[joakime]

@sbordet need a fresh approval, had to fix a build issue with javadoc