Merge pull request #9313 from eclipse/fix/jetty-10.0.x/jetty-sh-start-properties

Issue #9309 - Better jetty.sh integration for start.jar with eye on supporting odd properties
This commit is contained in:
Joakim Erdfelt 2023-03-24 08:34:37 -05:00 committed by GitHub
commit a9bae8eaf8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 77 additions and 78 deletions

View File

@ -24,7 +24,8 @@ jobs:
strategy:
fail-fast: false
matrix:
language: [ 'java']
languages:
- java
# CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ]
# Learn more about CodeQL language support at https://aka.ms/codeql-docs/language-support
@ -62,7 +63,7 @@ jobs:
- name: Initialize CodeQL
uses: github/codeql-action/init@v2
with:
languages: ${{ matrix.language }}
languages: ${{ matrix.languages }}
# If you wish to specify custom queries, you can do so here or in a config file.
# By default, queries listed here will override any specified in a config file.
# Prefix the list here with "+" to use these queries and those in the config file.
@ -71,7 +72,7 @@ jobs:
# queries: security-extended,security-and-quality
- name: Set up Maven
uses: stCarolas/setup-maven@v4
uses: stCarolas/setup-maven@v4.5
with:
maven-version: 3.8.6

View File

@ -170,7 +170,8 @@ dumpEnv()
echo "JETTY_START_LOG = $JETTY_START_LOG"
echo "JETTY_STATE = $JETTY_STATE"
echo "JETTY_START_TIMEOUT = $JETTY_START_TIMEOUT"
echo "RUN_CMD = ${RUN_CMD[*]}"
echo "JETTY_SYS_PROPS = $JETTY_SYS_PROPS"
echo "RUN_ARGS = ${RUN_ARGS[*]}"
}
@ -414,9 +415,6 @@ TMPDIR="`cygpath -w $TMPDIR`"
;;
esac
BASE_JETTY_SYS_PROPS=$(echo -ne "-Djetty.home=$JETTY_HOME" "-Djetty.base=$JETTY_BASE" "-Djava.io.tmpdir=$TMPDIR")
JETTY_SYS_PROPS=(${JETTY_SYS_PROPS[*]} $BASE_JETTY_SYS_PROPS)
#####################################################
# This is how the Jetty server will be started
#####################################################
@ -434,8 +432,9 @@ case "`uname`" in
CYGWIN*) JETTY_START="`cygpath -w $JETTY_START`";;
esac
RUN_ARGS=$("$JAVA" -jar "$JETTY_START" --dry-run=opts,path,main,args ${JETTY_ARGS[*]} ${JAVA_OPTIONS[*]})
RUN_CMD=("$JAVA" $JETTY_SYS_PROPS ${RUN_ARGS[@]})
# Collect the dry-run (of opts,path,main,args) from the jetty.base configuration
JETTY_DRY_RUN=$("$JAVA" -jar "$JETTY_START" --dry-run=opts,path,main,args ${JETTY_ARGS[*]} ${JAVA_OPTIONS[*]})
RUN_ARGS=($JETTY_SYS_PROPS ${JETTY_DRY_RUN[@]})
#####################################################
# Comment these out after you're happy with what
@ -466,13 +465,14 @@ case "$ACTION" in
CH_USER="--chuid $JETTY_USER"
fi
start-stop-daemon --start $CH_USER \
echo ${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" \
-- ${RUN_ARGS[@]} start-log-file="$JETTY_START_LOG"
--
else
@ -495,11 +495,11 @@ case "$ACTION" in
# FIXME: Broken solution: wordsplitting, pathname expansion, arbitrary command execution, etc.
su - "$JETTY_USER" $SU_SHELL -c "
cd \"$JETTY_BASE\"
exec ${RUN_CMD[*]} start-log-file=\"$JETTY_START_LOG\" > /dev/null &
echo ${RUN_ARGS[*]} start-log-file=\"$JETTY_START_LOG\" | xargs ${JAVA} > /dev/null &
disown \$!
echo \$! > \"$JETTY_PID\""
else
"${RUN_CMD[@]}" > /dev/null &
echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null &
disown $!
echo $! > "$JETTY_PID"
fi
@ -584,7 +584,7 @@ case "$ACTION" in
# Under control of daemontools supervise monitor which
# handles restarts and shutdowns via the svc program.
#
exec "${RUN_CMD[@]}"
echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null &
;;
@ -597,7 +597,7 @@ case "$ACTION" in
exit 1
fi
exec "${RUN_CMD[@]}"
echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null &
;;
check|status)

View File

@ -571,14 +571,14 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
}
else
{
throw new IllegalStateException("formatString parsing error");
throw new IllegalStateException("formatString parsing error: " + formatString);
}
remaining = m.group("REMAINING");
}
else
{
throw new IllegalArgumentException("Invalid format string");
throw new IllegalArgumentException("Invalid format string: " + formatString);
}
}

View File

@ -59,34 +59,12 @@ public class CommandLineBuilder
*
* @param arg the argument to quote
* @return the quoted and escaped argument
* @deprecated no replacement, quoting is done by {@link #toQuotedString()} now.
*/
@Deprecated
public static String quote(String arg)
{
boolean needsQuoting = (arg.indexOf(' ') >= 0) || (arg.indexOf('"') >= 0);
if (!needsQuoting)
{
return arg;
}
StringBuilder buf = new StringBuilder();
// buf.append('"');
boolean escaped = false;
boolean quoted = false;
for (char c : arg.toCharArray())
{
if (!quoted && !escaped && ((c == '"') || (c == ' ')))
{
buf.append("\\");
}
// don't quote text in single quotes
if (!escaped && (c == '\''))
{
quoted = !quoted;
}
escaped = (c == '\\');
buf.append(c);
}
// buf.append('"');
return buf.toString();
return "'" + arg + "'";
}
private List<String> args;
@ -113,7 +91,7 @@ public class CommandLineBuilder
{
if (arg != null)
{
args.add(quote(arg));
args.add(arg);
}
}
@ -136,11 +114,11 @@ public class CommandLineBuilder
{
if ((value != null) && (value.length() > 0))
{
args.add(quote(name + "=" + value));
args.add(name + "=" + value);
}
else
{
args.add(quote(name));
args.add(name);
}
}
@ -180,7 +158,31 @@ public class CommandLineBuilder
{
buf.append(delim);
}
buf.append(quote(arg));
buf.append(arg); // we assume escaping has occurred during addArg
}
return buf.toString();
}
/**
* A version of {@link #toString()} where every arg is evaluated for potential {@code '} (single-quote tick) wrapping.
*
* @return the toString but each arg that has spaces is surrounded by {@code '} (single-quote tick)
*/
public String toQuotedString()
{
StringBuilder buf = new StringBuilder();
for (String arg : args)
{
if (buf.length() > 0)
buf.append(' ');
boolean needsQuotes = (arg.contains(" "));
if (needsQuotes)
buf.append("'");
buf.append(arg);
if (needsQuotes)
buf.append("'");
}
return buf.toString();

View File

@ -440,7 +440,8 @@ public class Main
if (args.isDryRun())
{
CommandLineBuilder cmd = args.getMainArgs(args.getDryRunParts());
System.out.println(cmd.toString(StartLog.isDebugEnabled() ? " \\\n" : " "));
cmd.debug();
System.out.println(cmd.toQuotedString());
}
if (args.isStopCommand())

View File

@ -13,7 +13,6 @@
package org.eclipse.jetty.start;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
@ -21,54 +20,46 @@ import static org.hamcrest.Matchers.is;
public class CommandLineBuilderTest
{
private CommandLineBuilder cmd = new CommandLineBuilder("java");
@BeforeEach
public void setUp()
{
cmd.addEqualsArg("-Djava.io.tmpdir", "/home/java/temp dir/");
cmd.addArg("--version");
}
@Test
public void testSimpleCommandline()
{
assertThat(cmd.toString(), is("java -Djava.io.tmpdir=/home/java/temp\\ dir/ --version"));
CommandLineBuilder cmd = new CommandLineBuilder("java");
cmd.addEqualsArg("-Djava.io.tmpdir", "/home/java/temp dir/");
cmd.addArg("--version");
assertThat(cmd.toQuotedString(), is("java '-Djava.io.tmpdir=/home/java/temp dir/' --version"));
}
@Test
public void testQuotingSimple()
public void testSimpleHomeNoSpace()
{
assertQuoting("/opt/jetty", "/opt/jetty");
CommandLineBuilder cmd = new CommandLineBuilder("java");
cmd.addEqualsArg("-Djetty.home", "/opt/jetty");
assertThat(cmd.toQuotedString(), is("java -Djetty.home=/opt/jetty"));
}
@Test
public void testQuotingSpaceInPath()
public void testSimpleHomeWithSpace()
{
assertQuoting("/opt/jetty 7/home", "/opt/jetty\\ 7/home");
CommandLineBuilder cmd = new CommandLineBuilder("java");
cmd.addEqualsArg("-Djetty.home", "/opt/jetty 10/home");
assertThat(cmd.toQuotedString(), is("java '-Djetty.home=/opt/jetty 10/home'"));
}
@Test
public void testQuotingSpaceAndQuotesInPath()
public void testEscapedFormattingString()
{
assertQuoting("/opt/jetty 7 \"special\"/home", "/opt/jetty\\ 7\\ \\\"special\\\"/home");
CommandLineBuilder cmd = new CommandLineBuilder("java");
cmd.addEqualsArg("-Djetty.home", "/opt/jetty");
cmd.addEqualsArg("jetty.requestlog.formatter", "%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\"");
assertThat(cmd.toQuotedString(), is("java -Djetty.home=/opt/jetty 'jetty.requestlog.formatter=%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\"'"));
}
@Test
public void testToStringIsQuotedEvenIfArgsAreNotQuotedForProcessBuilder()
public void testEscapeUnicode()
{
System.out.println(cmd.toString());
}
@Test
public void testQuoteQuotationMarks()
{
assertQuoting("-XX:OnOutOfMemoryError='kill -9 %p'", "-XX:OnOutOfMemoryError='kill -9 %p'");
}
private void assertQuoting(String raw, String expected)
{
String actual = CommandLineBuilder.quote(raw);
assertThat("Quoted version of [" + raw + "]", actual, is(expected));
CommandLineBuilder cmd = new CommandLineBuilder("java");
cmd.addEqualsArg("-Djetty.home", "/opt/jetty");
cmd.addEqualsArg("monetary.symbol", "");
assertThat(cmd.toQuotedString(), is("java -Djetty.home=/opt/jetty monetary.symbol=€"));
}
}

View File

@ -31,7 +31,10 @@ import org.eclipse.jetty.server.session.TestServer;
import org.eclipse.jetty.util.NanoTime;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledForJreRange;
import org.junit.jupiter.api.condition.JRE;
import org.testcontainers.junit.jupiter.Testcontainers;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -43,6 +46,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
* See bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=444595
*/
@Testcontainers(disabledWithoutDocker = true)
@Tag("flaky")
public class AttributeNameTest
{
@BeforeAll