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: NEEDINFO
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:
Depends on:
Blocks:
 
Reported: 2019-10-08 12:16 UTC by mrogers@zaizi.com
Modified: 2019-12-11 09:32 UTC (History)
0 users



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.