Bug 63815 - Expansion of JAVA_OPTS in catalina.sh containing '*' stops startup on linux
Summary: Expansion of JAVA_OPTS in catalina.sh containing '*' stops startup on linux
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.x-trunk
Hardware: Other Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 64068 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-10-08 12:16 UTC by mrogers@zaizi.com
Modified: 2020-10-02 12:39 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mrogers@zaizi.com 2019-10-08 12:16:23 UTC
On Linux, the catalina.sh does not correctly quote use of $JAVA_OPTS so if you specify an option containing a bash shell special character, in particular '*' then tomcat fails to start.

For example set 

JAVA_OPTS="-Dfoo.cronExpression=\"0 0 0/12 * * ?\"" 

And then try to start tomcat, it will fail with a non zero exit code.



Part of the fix is to stop the expansion by simply put the use of JAVA_OPTS in double quotes.   i.e. "$JAVA_OPTS"

An additional complication is that the unquoted expression strips newline characters so for backwards compatibility its also necessary to strip newline characters.
Comment 1 Mark Thomas 2019-10-08 16:51:59 UTC
I'm wondering how important it is to remove newlines backwards compatibility. might it be sufficient to document the change in behaviour?
Comment 2 Konstantin Kolinko 2019-10-08 21:47:35 UTC
What exact change are you asking for?

Please note that on the last step (when invoking a java executable) the JAVA_OPTS must produce several separate command line arguments: java expects that each -D is passed as a separate command-line argument. As such, JAVA_OPTS cannot be quoted there.


I have not tested, but I think that you should have used the quotes around the whole -D argument:

JAVA_OPTS="\"-Dfoo.cronExpression=0 0 0/12 * * ?\""
Comment 3 Eugène Adell 2019-10-13 22:41:41 UTC
First, please use CATALINA_OPTS when you want to give information to the application, instead of JAVA_OPTS, and edit setenv.sh instead of catalina.sh (it will make your maintenance and upgrades easier).

I can't reproduce your problem on Tomcat 7.0 or 8.5. The only side effect that I get with '*' is of course the variable expanding but I can neutralize this by adding a set -o noglob to the setenv.sh
Please send the output of 'env|sort', 'bash --version', 'echo $SHELLOPTS' then maybe we can compare
My tests below.


$ cat bin/setenv.sh
set -o noglob
CATALINA_OPTS="-Dfoo.cronExpression=\"0 0 0/12 * * ?\""

$ ps -eaf  | grep 15394
eadell   15394     1  2 00:22 pts/0    00:00:25 /usr/local/java/1.9/bin/java -Djava.util.logging.config.file=/home/eadell/tests/85/conf/logging.properties -Djava.util.logging.manager=org.apache.juli.ClassLoaderLogManager -Djdk.tls.ephemeralDHKeySize=2048 -Djava.protocol.handler.pkgs=org.apache.catalina.webresources -Dorg.apache.catalina.security.SecurityListener.UMASK=0027 -Dfoo.cronExpression=0 0 0/12 * * ? -Dignore.endorsed.dirs= -classpath /home/eadell/tests/85/bin/bootstrap.jar:/home/eadell/tests/85/bin/tomcat-juli.jar -Dcatalina.base=/home/eadell/tests/85 -Dcatalina.home=/home/eadell/tests/85 -Djava.io.tmpdir=/home/eadell/tests/85/temp org.apache.catalina.startup.Bootstrap start
eadell   15909 14747  0 00:37 pts/0    00:00:00 grep --color=auto 15394


reading system properties from a JSP :
foo.cronExpression: 0 0 0/12 * * ?
Comment 4 mrogers@zaizi.com 2019-10-14 09:43:53 UTC
Thank you for the comments.

I'm actually using Tomcat within docker so would like to avoid touching any tomcat code or configuration, whether catalina.sh or setenv.sh.   The pattern I'm seeing used more and more is to specify a long list of -D options in the docker-compose.yml file.

Answers to questions 
GNU bash, version 4.2.46(2)-release (x86_64-redhat-linux-gnu)
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

braceexpand:emacs:hashall:histexpand:history:interactive-comments:monitor

Agreed I should probably be using CATALINA_OPTS however that seems to have exactly the same issue with expansion of '*' as JAVA_OPTS

I think the quoting I have used is O.K. since with a quick hack to the tomcat source files then it works.

What exact change are you asking for?
It's taken me some time to diagnose and work-around this problem.  If I can share knowledge then that helps the community.

I've fixed this by replacing $JAVA_OPTS with "${JAVA_OPTS//$'\n'/ }" in catalina.sh.  As pointed out below the same effect can be obtained by set -o noglob .

Also I couldn't see any discussion about globing in the Tomcat documentation or comments so perhaps a sentence could be added?
Comment 5 Eugène Adell 2019-10-14 16:53:37 UTC
Your quoting is OK (confirmed in https://docs.oracle.com/javase/7/docs/technotes/tools/windows/java.html although the unix equivalent webpage doesn't mention this).

I'm not convinced Tomcat is guilty of anything here. Probably it fails starting because the application is unable to run with an incomplete parameter, and there are several workarounds for no-globbing. It's the normal behaviour of shells to expand variables. Another workaround is reading a property file, maybe that's what people are doing when facing this issue, I don't know.

--Also I couldn't see any discussion about globing in the Tomcat documentation or comments so perhaps a sentence could be added?
Thanks, it's true :) The different variables are explained in the RUNNING.txt file, we could add a note there but I'm scared very few people read that.
Comment 6 mrogers@zaizi.com 2019-10-15 08:32:20 UTC
There are various style guides on the web suggesting that it is good practice to always quote the use of variables in bash scripts 

e.g. Google   https://google.github.io/styleguide/shell.xml
     Linux    https://www.tldp.org/LDP/abs/html/quotingvar.html
Comment 7 Mark Thomas 2019-10-16 13:00:53 UTC
(In reply to Konstantin Kolinko from comment #2)

> Please note that on the last step (when invoking a java executable) the
> JAVA_OPTS must produce several separate command line arguments: java expects
> that each -D is passed as a separate command-line argument. As such,
> JAVA_OPTS cannot be quoted there.

I've confirmed that the above is not a concern. The arguments are passed correct when that use of JAVA_OPTS is quoted. The quoting simply prevents the expansion of * and strips new lines.

I have a patch locally that applies the quoting fix to JAVA_OPTS and CATALINA_OPTS in all affected shell scripts.

I'm still leaning towards applying this fix without the removal of newlines and noting the change of behaviour in the change log.
Comment 8 Mark Thomas 2019-10-20 20:24:57 UTC
I didn;t see any objections so I went ahead and applied the patch.

Fixed in:
- master for 9.0.28 onwards
- 8.5.x for 8.5.48 onwards
- 7.0.x for 7.0.98 onwards
Comment 9 Mark Thomas 2019-11-25 14:39:54 UTC
This fix triggered a regression in tool-wrapper.sh and daemon.sh. catalina.sh was not impacted due to the use of eval in that script.

I'm working on a fix.
Comment 10 Mark Thomas 2019-11-25 15:51:14 UTC
Konstantin was right in comment #2. I missed it because I only tested code paths where eval was used. All the code paths that don't use eval are now broken.
Comment 11 Mark Thomas 2019-11-25 20:53:28 UTC
I have reverted this for :
- master for 9.0.30 onwards
- 8.5.x for 8.5.50 onwards
- 7.0.99 onwards

due to the regressions.

There are broadly three different scenarios:
a) passing to java in catalina.sh/tool-wrapper.sh
b) passing to jsvc in daemon.sh
c) passing to jdb in catalina.sh

I haven't been able to find a one size fits all solution. There are various options for a) - which would address the original bug report - but I'd like to explore this some more to see what options exist for b) and c). I could live with no solving c) but ideally b) needs a solution.

For anyone else looking at this, it is worth looking through the history of deamon.sh for all the various edge cases that have been addressed over the years.
Comment 12 Mark Thomas 2019-11-26 12:39:10 UTC
Scenario c) is currently unfixable due to a jdb bug. I've created a test case and opened https://bugs.openjdk.java.net/browse/JDK-8234808
Comment 13 Mark Thomas 2019-11-26 16:20:30 UTC
Scenarios a) and b) can use the same solution. I have implemented this committed the changes in:
- master for 9.0.30 onwards
- 8.5.x for 8.5.50 onwards
- 7.0.99 onwards

I'm hopeful that scenario c) will be able to use the same fix once the JRE bug is fixed.

I'm moving this to NEEDINFO as that is th ebest state we have to indicate that we are waiting on a JRE fix to complete this fix.
Comment 14 Konstantin Kolinko 2019-12-11 09:32:31 UTC
(In reply to Mark Thomas from comment #13)
> Scenarios a) and b) can use the same solution. I have implemented this
> committed the changes in:
> - master for 9.0.30 onwards
> - 8.5.x for 8.5.50 onwards
> - 7.0.99 onwards

For a record,
there was a typo in daemon.sh in that commit.

Apparently it has no consequences (as far as Mark and I tested), but I am noting it here in case we are missing something.

It was reported and discussed in the following mail thread:
https://tomcat.markmail.org/message/g7fdokfpx4axdwa5
"Tag Tomcat 7", 2019-12-10.

The issue is fixed with the following commit:
https://github.com/apache/tomcat/commit/1d97bbbd4f060c7c9ab39f12bc67fde3085f36ef

The fix will be in Tomcat 9.0.31, 8.5.51, 7.0.99.
Comment 15 Mark Thomas 2020-01-13 22:07:43 UTC
*** Bug 64068 has been marked as a duplicate of this bug. ***
Comment 16 Mark Thomas 2020-09-22 09:52:53 UTC
This has been fixed in the main development branch for openjdk 16. The fix should be in the next EA release.

There is currently no information on whether or not the fix will be back-ported.

Once the catalina.sh fix is applied, debug won't work unless either:
a) A JDK that includes the JDK fix is used; or
b) the changes in catalina.sh are reverted

So the question is, when do we apply the catalina.sh fix?
- As soon as a 16 EA release is available with the fix?
- As soon as a 16 GA release is available with the fix?
- As soon as an LTS release is available with the fix?
- As soon the minimum version of Java required by the major Tomcat version is guaranteed to include the JDK fix?

Those time ranges run from a few days to years in the future. None of those options are appealing.

I'm currently thinking of the following:
- apply the catalina.sh fix once there is an 16 EA release with the JDK fix
- introduce a new command "debug-legacy" that invokes jdb assuming the JDK fix is not present
- remove debug legacy once the minimum Java version required by the Tomcat major version is guaranteed to include the JDK fix.


I don't like maintaining what is essentially two versions of invoking jdb but I don't see a better plan.
Comment 17 Mark Thomas 2020-10-02 12:39:54 UTC
In final testing there was no way I could get Tomcat to start in debug mode when a system, property needed quoting so I dropped the idea of a debug-legacy option

Fixed in:
- master for 10.0.0-M9 onwards
- 9.0.x for 9.0.39 onwards
- 8.5.x for 8.5.59 onwards
- 7.0.x for 7.0.107 onwards