diff --git a/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java b/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java index 9b84b617164..b621d32bfac 100644 --- a/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java +++ b/jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java @@ -113,7 +113,7 @@ public class HTTP2Client extends ContainerLifeCycle private int maxDecoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY; private int maxEncoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY; private int maxHeaderBlockFragment = 0; - private int maxResponseHeadersSize = -1; + private int maxResponseHeadersSize = 8 * 1024; private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F); private long streamIdleTimeout; private boolean useInputDirectByteBuffers = true; diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java index b64ba15a6fd..facd47e0511 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java @@ -46,6 +46,11 @@ public class HeaderBlockParser this.notifier = notifier; } + public int getMaxHeaderListSize() + { + return hpackDecoder.getMaxHeaderListSize(); + } + /** * Parses @{code blockLength} HPACK bytes from the given {@code buffer}. * diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java index 7873a6842a6..e4bda6471ee 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java @@ -174,6 +174,10 @@ public class HeadersBodyParser extends BodyParser { if (hasFlag(Flags.END_HEADERS)) { + int maxLength = headerBlockParser.getMaxHeaderListSize(); + if (maxLength > 0 && length > maxLength) + return connectionFailure(buffer, ErrorCode.REFUSED_STREAM_ERROR.code, "invalid_headers_frame"); + MetaData metaData = headerBlockParser.parse(buffer, length); if (metaData == HeaderBlockParser.SESSION_FAILURE) return false; diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java index fc50ce3d438..b1f2d8a5bac 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java @@ -119,6 +119,10 @@ public class PushPromiseBodyParser extends BodyParser } case HEADERS: { + int maxLength = headerBlockParser.getMaxHeaderListSize(); + if (maxLength > 0 && length > maxLength) + return connectionFailure(buffer, ErrorCode.REFUSED_STREAM_ERROR.code, "invalid_headers_frame"); + MetaData.Request metaData = (MetaData.Request)headerBlockParser.parse(buffer, length); if (metaData == HeaderBlockParser.SESSION_FAILURE) return false; diff --git a/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 0846881fa71..0995b6126d4 100644 --- a/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -83,6 +83,11 @@ public class HpackDecoder _maxTableCapacity = maxTableCapacity; } + public int getMaxHeaderListSize() + { + return _builder.getMaxSize(); + } + public void setMaxHeaderListSize(int maxHeaderListSize) { _builder.setMaxSize(maxHeaderListSize); diff --git a/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/internal/MetaDataBuilder.java b/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/internal/MetaDataBuilder.java index a10e2e4b95a..c38d6ac5380 100644 --- a/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/internal/MetaDataBuilder.java +++ b/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/internal/MetaDataBuilder.java @@ -77,7 +77,7 @@ public class MetaDataBuilder { HttpHeader header = field.getHeader(); String name = field.getName(); - if (name == null || name.length() == 0) + if (name == null || name.isEmpty()) throw new SessionException("Header size 0"); String value = field.getValue(); int fieldSize = name.length() + (value == null ? 0 : value.length()); @@ -148,7 +148,7 @@ public class MetaDataBuilder case C_PATH: if (checkPseudoHeader(header, _path)) { - if (value != null && value.length() > 0) + if (value != null && !value.isEmpty()) _path = value; else streamException("No Path"); @@ -256,7 +256,7 @@ public class MetaDataBuilder return new MetaData.Request( NanoTime.now(), // TODO #9900 make beginNanoTime accurate _method, - _scheme == null ? HttpScheme.HTTP.asString() : _scheme.asString(), + _scheme.asString(), _authority, _path, HttpVersion.HTTP_2, diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java index 7abd05b15f4..fd19f866d70 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java @@ -53,11 +53,14 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.Jetty; import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.Graceful; import org.junit.jupiter.api.Test; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -984,6 +987,149 @@ public class HTTP2Test extends AbstractTest assertFalse(((HTTP2Session)serverSession).getEndPoint().isOpen()); } + @Test + public void testClientSendsLargeHeader() throws Exception + { + CountDownLatch settingsLatch = new CountDownLatch(2); + + CompletableFuture serverFailureFuture = new CompletableFuture<>(); + CompletableFuture serverCloseReasonFuture = new CompletableFuture<>(); + start(new ServerSessionListener.Adapter() + { + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + + @Override + public void onFailure(Session session, Throwable failure) + { + serverFailureFuture.complete(failure); + } + + @Override + public void onClose(Session session, GoAwayFrame frame) + { + serverCloseReasonFuture.complete(frame.tryConvertPayload()); + } + }); + + CompletableFuture clientFailureFuture = new CompletableFuture<>(); + CompletableFuture clientCloseReasonFuture = new CompletableFuture<>(); + Session.Listener.Adapter listener = new Session.Listener.Adapter() + { + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + + @Override + public void onFailure(Session session, Throwable failure) + { + clientFailureFuture.complete(failure); + } + + @Override + public void onClose(Session session, GoAwayFrame frame) + { + clientCloseReasonFuture.complete(frame.tryConvertPayload()); + } + }; + + HTTP2Session session = (HTTP2Session)newClient(listener); + assertTrue(settingsLatch.await(5, TimeUnit.SECONDS)); + session.getGenerator().getHpackEncoder().setMaxHeaderListSize(1024 * 1024); + + String value = StringUtil.stringFrom("x", 8 * 1024); + HttpFields requestFields = HttpFields.build() + .put("custom", value); + MetaData.Request metaData = newRequest("GET", requestFields); + HeadersFrame request = new HeadersFrame(metaData, null, true); + session.newStream(request, new FuturePromise<>(), new Stream.Listener.Adapter()); + + // Test failure and close reason on client. + String closeReason = clientCloseReasonFuture.get(5, TimeUnit.SECONDS); + assertThat(closeReason, equalTo("invalid_hpack_block")); + assertNull(clientFailureFuture.getNow(null)); + + // Test failure and close reason on server. + closeReason = serverCloseReasonFuture.get(5, TimeUnit.SECONDS); + assertThat(closeReason, equalTo("invalid_hpack_block")); + Throwable failure = serverFailureFuture.get(5, TimeUnit.SECONDS); + assertThat(failure, instanceOf(IOException.class)); + assertThat(failure.getMessage(), containsString("invalid_hpack_block")); + } + + @Test + public void testServerSendsLargeHeader() throws Exception + { + CompletableFuture serverFailureFuture = new CompletableFuture<>(); + CompletableFuture serverCloseReasonFuture = new CompletableFuture<>(); + start(new ServerSessionListener.Adapter() + { + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + HTTP2Session session = (HTTP2Session)stream.getSession(); + session.getGenerator().getHpackEncoder().setMaxHeaderListSize(1024 * 1024); + + String value = StringUtil.stringFrom("x", 8 * 1024); + HttpFields fields = HttpFields.build().put("custom", value); + MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, fields); + stream.headers(new HeadersFrame(stream.getId(), response, null, true)); + return null; + } + + @Override + public void onFailure(Session session, Throwable failure) + { + serverFailureFuture.complete(failure); + } + + @Override + public void onClose(Session session, GoAwayFrame frame) + { + serverCloseReasonFuture.complete(frame.tryConvertPayload()); + } + }); + + CompletableFuture clientFailureFuture = new CompletableFuture<>(); + CompletableFuture clientCloseReasonFuture = new CompletableFuture<>(); + Session.Listener.Adapter listener = new Session.Listener.Adapter() + { + @Override + public void onFailure(Session session, Throwable failure) + { + clientFailureFuture.complete(failure); + } + + @Override + public void onClose(Session session, GoAwayFrame frame) + { + clientCloseReasonFuture.complete(frame.tryConvertPayload()); + } + }; + + Session session = newClient(listener); + MetaData.Request metaData = newRequest("GET", HttpFields.EMPTY); + HeadersFrame request = new HeadersFrame(metaData, null, true); + session.newStream(request, new FuturePromise<>(), new Stream.Listener.Adapter()); + + // Test failure and close reason on server. + String closeReason = serverCloseReasonFuture.get(5, TimeUnit.SECONDS); + assertThat(closeReason, equalTo("invalid_hpack_block")); + assertNull(serverFailureFuture.getNow(null)); + + // Test failure and close reason on client. + closeReason = clientCloseReasonFuture.get(5, TimeUnit.SECONDS); + assertThat(closeReason, equalTo("invalid_hpack_block")); + Throwable failure = clientFailureFuture.get(5, TimeUnit.SECONDS); + assertThat(failure, instanceOf(IOException.class)); + assertThat(failure.getMessage(), containsString("invalid_hpack_block")); + } + private static void sleep(long time) { try diff --git a/jetty-core/jetty-server/src/main/config/modules/state.mod b/jetty-core/jetty-server/src/main/config/modules/state.mod index 5a4e09de420..aceeb233c1b 100644 --- a/jetty-core/jetty-server/src/main/config/modules/state.mod +++ b/jetty-core/jetty-server/src/main/config/modules/state.mod @@ -9,6 +9,9 @@ start [depends] server +[before] +deploy + [xml] etc/jetty-state.xml diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/StateLifeCycleListener.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/StateLifeCycleListener.java index bf7fcdb2f3b..5dab34d83e6 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/StateLifeCycleListener.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/StateLifeCycleListener.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.server; +import java.io.IOException; import java.io.Writer; import java.nio.file.Files; import java.nio.file.Path; @@ -24,7 +25,7 @@ import org.slf4j.LoggerFactory; import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.file.StandardOpenOption.APPEND; -import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.CREATE_NEW; import static java.nio.file.StandardOpenOption.WRITE; /** @@ -35,55 +36,77 @@ public class StateLifeCycleListener implements LifeCycle.Listener { private static final Logger LOG = LoggerFactory.getLogger(StateLifeCycleListener.class); - private final Path _filename; + private final Path stateFile; - public StateLifeCycleListener(String filename) + public StateLifeCycleListener(String filename) throws IOException { - _filename = Paths.get(filename).toAbsolutePath(); + stateFile = Paths.get(filename).toAbsolutePath(); if (LOG.isDebugEnabled()) - LOG.debug("State File: {}", _filename); + LOG.debug("State File: {}", stateFile); + + // We use raw Files APIs here to allow important IOExceptions + // to fail the startup of Jetty, as these kinds of errors + // point to filesystem permission issues that must be resolved + // by the user before the state file can be used. + + // Start with fresh file (for permission reasons) + Files.deleteIfExists(stateFile); + + // Create file + Files.writeString(stateFile, "INIT " + this + "\n", UTF_8, WRITE, CREATE_NEW); } - private void writeState(String action, LifeCycle lifecycle) + private void appendStateChange(String action, Object obj) { - try (Writer out = Files.newBufferedWriter(_filename, UTF_8, WRITE, CREATE, APPEND)) + try (Writer out = Files.newBufferedWriter(stateFile, UTF_8, WRITE, APPEND)) { - out.append(action).append(" ").append(lifecycle.toString()).append("\n"); + String entry = String.format("%s %s\n", action, obj); + if (LOG.isDebugEnabled()) + LOG.debug("appendEntry to {}: {}", stateFile, entry); + out.append(entry); } - catch (Exception e) + catch (IOException e) { - LOG.warn("Unable to write state", e); + // this can happen if the uid of the Jetty process changes after it has been started + // such as can happen with some setuid configurations + LOG.warn("Unable to append to state file: " + stateFile, e); } } @Override public void lifeCycleStarting(LifeCycle event) { - writeState("STARTING", event); + appendStateChange("STARTING", event); } @Override public void lifeCycleStarted(LifeCycle event) { - writeState("STARTED", event); + appendStateChange("STARTED", event); } @Override public void lifeCycleFailure(LifeCycle event, Throwable cause) { - writeState("FAILED", event); + appendStateChange("FAILED", event); } @Override public void lifeCycleStopping(LifeCycle event) { - writeState("STOPPING", event); + appendStateChange("STOPPING", event); } @Override public void lifeCycleStopped(LifeCycle event) { - writeState("STOPPED", event); + appendStateChange("STOPPED", event); + } + + @Override + public String toString() + { + return String.format("%s@%h", this.getClass().getSimpleName(), this); } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ScheduledExecutorScheduler.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ScheduledExecutorScheduler.java index 8c3a999bb60..ce2df177d15 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ScheduledExecutorScheduler.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ScheduledExecutorScheduler.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.util.thread; import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -43,7 +44,7 @@ public class ScheduledExecutorScheduler extends AbstractLifeCycle implements Sch private final ThreadGroup threadGroup; private final int threads; private final AtomicInteger count = new AtomicInteger(); - private volatile ScheduledThreadPoolExecutor scheduler; + private volatile ScheduledExecutorService scheduler; private volatile Thread thread; public ScheduledExecutorScheduler() @@ -76,7 +77,7 @@ public class ScheduledExecutorScheduler extends AbstractLifeCycle implements Sch * @param daemon True if scheduler threads should be daemon * @param classLoader The classloader to run the threads with or null to use the current thread context classloader * @param threadGroup The threadgroup to use or null for no thread group - * @param threads The number of threads to pass to the the core {@link ScheduledThreadPoolExecutor} or -1 for a + * @param threads The number of threads to pass to the core {@link ScheduledExecutorService} or -1 for a * heuristic determined number of threads. */ public ScheduledExecutorScheduler(@Name("name") String name, @Name("daemon") boolean daemon, @Name("classLoader") ClassLoader classLoader, @Name("threadGroup") ThreadGroup threadGroup, @Name("threads") int threads) @@ -88,33 +89,54 @@ public class ScheduledExecutorScheduler extends AbstractLifeCycle implements Sch this.threads = threads; } + /** + * @param scheduledExecutorService the core {@link ScheduledExecutorService} to be used + */ + public ScheduledExecutorScheduler(ScheduledExecutorService scheduledExecutorService) + { + this.name = null; + this.daemon = false; + this.classloader = null; + this.threadGroup = null; + this.threads = 0; + this.scheduler = scheduledExecutorService; + } + @Override protected void doStart() throws Exception { - int size = threads > 0 ? threads : 1; - scheduler = new ScheduledThreadPoolExecutor(size, r -> + if (this.scheduler == null) { - Thread thread = ScheduledExecutorScheduler.this.thread = new Thread(threadGroup, r, name + "-" + count.incrementAndGet()); - thread.setDaemon(daemon); - thread.setContextClassLoader(classloader); - return thread; - }); - scheduler.setRemoveOnCancelPolicy(true); + int size = threads > 0 ? threads : 1; + ScheduledThreadPoolExecutor scheduler = new ScheduledThreadPoolExecutor(size, r -> + { + Thread thread = ScheduledExecutorScheduler.this.thread = new Thread(threadGroup, r, name + "-" + count.incrementAndGet()); + thread.setDaemon(daemon); + thread.setContextClassLoader(classloader); + return thread; + }); + scheduler.setRemoveOnCancelPolicy(true); + this.scheduler = scheduler; + } super.doStart(); } @Override protected void doStop() throws Exception { - scheduler.shutdownNow(); + // If name is set to null, this means we got the scheduler from the constructor. + if (name != null) + { + scheduler.shutdownNow(); + scheduler = null; + } super.doStop(); - scheduler = null; } @Override public Task schedule(Runnable task, long delay, TimeUnit unit) { - ScheduledThreadPoolExecutor s = scheduler; + ScheduledExecutorService s = scheduler; if (s == null) return () -> false; ScheduledFuture result = s.schedule(task, delay, unit); diff --git a/jetty-home/src/main/resources/bin/jetty.sh b/jetty-home/src/main/resources/bin/jetty.sh index 08278d16128..b29dbd2600c 100755 --- a/jetty-home/src/main/resources/bin/jetty.sh +++ b/jetty-home/src/main/resources/bin/jetty.sh @@ -118,35 +118,92 @@ findDirectory() done } +# test if process specified in PID file is still running running() { - if [ -f "$1" ] - then - local PID=$(cat "$1" 2>/dev/null) || return 1 - kill -0 "$PID" 2>/dev/null - return + local PIDFILE=$1 + if [ -r "$PIDFILE" ] ; then + local PID=$(tail -1 "$PIDFILE") + if kill -0 "$PID" 2>/dev/null ; then + return 0 + fi fi - rm -f "$1" return 1 } +# Test state file (after timeout) for started state started() { - # wait for 60s to see "STARTED" in PID file, needs jetty-started.xml as argument - for ((T = 0; T < $(($3 / 4)); T++)) + local STATEFILE=$1 + local PIDFILE=$2 + local STARTTIMEOUT=$3 + # wait till timeout to see "STARTED" in state file, needs --module=state as argument + for ((T = 0; T < $STARTTIMEOUT; T++)) do - sleep 4 - [ -z "$(tail -1 $1 | grep STARTED 2>/dev/null)" ] || return 0 - [ -z "$(tail -1 $1 | grep STOPPED 2>/dev/null)" ] || return 1 - [ -z "$(tail -1 $1 | grep FAILED 2>/dev/null)" ] || return 1 - local PID=$(cat "$2" 2>/dev/null) || return 1 - kill -0 "$PID" 2>/dev/null || return 1 - echo -n ". " + echo -n "." + sleep 1 + if [ -r $STATEFILE ] ; then + STATENOW=$(tail -1 $STATEFILE) + (( DEBUG )) && echo "State (now): $STATENOW" + case "$STATENOW" in + STARTED*) + echo " started" + return 0;; + STOPPED*) + echo " stopped" + return 1;; + FAILED*) + echo " failed" + return 1;; + esac + else + (( DEBUG )) && echo "Unable to read State File: $STATEFILE" + fi done - + (( DEBUG )) && echo "Timeout $STARTTIMEOUT expired waiting for start state from $STATEFILE" + echo " timeout" return 1; } +pidKill() +{ + local PIDFILE=$1 + local TIMEOUT=$2 + + if [ -r $PIDFILE ] ; then + local PID=$(tail -1 "$PIDFILE") + if [ -z "$PID" ] ; then + echo "ERROR: no pid found in $PIDFILE" + return 1 + fi + + # Try default kill first + if kill -0 "$PID" 2>/dev/null ; then + (( DEBUG )) && echo "PID=$PID is running, sending kill" + kill "$PID" 2>/dev/null + else + rm -f $PIDFILE 2> /dev/null + return 0 + fi + + # Perform harsh kill next + while kill -0 "$PID" 2>/dev/null + do + if (( TIMEOUT-- == 0 )) ; then + (( DEBUG )) && echo "PID=$PID is running, sending kill signal=KILL (TIMEOUT=$TIMEOUT)" + kill -KILL "$PID" 2>/dev/null + fi + echo -n "." + sleep 1 + done + echo "Killed $PID" + return 0 + else + (( DEBUG )) && echo "Unable to read PID File: $PIDFILE" + return 1 + fi +} + readConfig() { @@ -436,10 +493,6 @@ esac JETTY_DRY_RUN=$("$JAVA" -jar "$JETTY_START" --dry-run=opts,path,main,args,envs ${JETTY_ARGS[*]} ${JAVA_OPTIONS[*]}) RUN_ARGS=($JETTY_SYS_PROPS ${JETTY_DRY_RUN[@]}) -##################################################### -# Comment these out after you're happy with what -# the script is doing. -##################################################### if (( DEBUG )) then dumpEnv @@ -497,22 +550,23 @@ case "$ACTION" in su - "$JETTY_USER" $SU_SHELL -c " cd \"$JETTY_BASE\" echo ${RUN_ARGS[*]} start-log-file=\"$JETTY_START_LOG\" | xargs ${JAVA} > /dev/null & - disown $(pgrep -P $!)" + disown \$!" else # Startup if not switching users echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null & - disown $(pgrep -P $!) + disown $! fi fi - if expr "${JETTY_ARGS[*]}" : '.*jetty-started.xml.*' >/dev/null + if expr "${JETTY_ARGS[*]}" : '.*jetty\.state=.*' >/dev/null then if started "$JETTY_STATE" "$JETTY_PID" "$JETTY_START_TIMEOUT" then echo "OK `date`" else echo "FAILED `date`" + pidKill $JETTY_PID 30 exit 1 fi else @@ -537,26 +591,12 @@ case "$ACTION" in done else # Stop from a non-service path - if [ ! -f "$JETTY_PID" ] ; then + if [ ! -r "$JETTY_PID" ] ; then echo "ERROR: no pid found at $JETTY_PID" exit 1 fi - PID=$(cat "$JETTY_PID" 2>/dev/null) - if [ -z "$PID" ] ; then - echo "ERROR: no pid id found in $JETTY_PID" - exit 1 - fi - kill "$PID" 2>/dev/null - - TIMEOUT=30 - while running $JETTY_PID; do - if (( TIMEOUT-- == 0 )); then - kill -KILL "$PID" 2>/dev/null - fi - - sleep 1 - done + pidKill "$JETTY_PID" 30 fi rm -f "$JETTY_PID" diff --git a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index dcdbcc105ab..3a975aea99e 100644 --- a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -15,6 +15,7 @@ package org.eclipse.jetty.tests.distribution; import java.io.File; import java.io.FileWriter; +import java.io.IOException; import java.net.URI; import java.net.http.WebSocket; import java.nio.ByteBuffer; @@ -25,6 +26,7 @@ import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.util.ArrayList; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Queue; @@ -53,6 +55,7 @@ import org.eclipse.jetty.tests.hometester.JettyHomeTester; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.PathMatchers; import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -107,6 +110,78 @@ public class DistributionTests extends AbstractJettyHomeTest } } + @Test + public void testJettyConf() throws Exception + { + String jettyVersion = System.getProperty("jettyVersion"); + JettyHomeTester distribution = JettyHomeTester.Builder.newInstance() + .jettyVersion(jettyVersion) + .mavenLocalRepository(System.getProperty("mavenRepoPath")) + .build(); + + try (JettyHomeTester.Run run1 = distribution.start("--add-modules=http")) + { + assertTrue(run1.awaitFor(10, TimeUnit.SECONDS)); + assertEquals(0, run1.getExitValue()); + + Path pidfile = run1.getConfig().getJettyBase().resolve("jetty.pid"); + Path statefile = run1.getConfig().getJettyBase().resolve("jetty.state"); + + int port = distribution.freePort(); + + List args = new ArrayList<>(); + args.add("jetty.http.port=" + port); + args.add("jetty.state=" + statefile); + args.add("jetty.pid=" + pidfile); + + Path confFile = run1.getConfig().getJettyHome().resolve("etc/jetty.conf"); + for (String line : Files.readAllLines(confFile, StandardCharsets.UTF_8)) + { + if (line.startsWith("#") || StringUtil.isBlank(line)) + continue; // skip + args.add(line); + } + + try (JettyHomeTester.Run run2 = distribution.start(args)) + { + assertTrue(run2.awaitConsoleLogsFor("Started Server@", 10, TimeUnit.SECONDS)); + + assertTrue(Files.isRegularFile(pidfile), "PID file should exist"); + assertTrue(Files.isRegularFile(statefile), "State file should exist"); + String state = tail(statefile); + assertThat("State file", state, startsWith("STARTED ")); + + startHttpClient(); + ContentResponse response = client.GET("http://localhost:" + port); + assertEquals(HttpStatus.NOT_FOUND_404, response.getStatus()); + } + + await().atMost(Duration.ofSeconds(10)).until(() -> !Files.exists(pidfile)); + await().atMost(Duration.ofSeconds(10)).until(() -> tail(statefile).startsWith("STOPPED ")); + } + } + + /** + * Get the last line of the file. + * + * @param file the file to read from + * @return the string representing the last line of the file, or null if not found + */ + private static String tail(Path file) + { + try + { + List lines = Files.readAllLines(file, StandardCharsets.UTF_8); + if (lines.isEmpty()) + return ""; + return lines.get(lines.size() - 1); + } + catch (IOException e) + { + return ""; + } + } + @ParameterizedTest @ValueSource(strings = {"ee9", "ee10"}) public void testQuickStartGenerationAndRun(String env) throws Exception @@ -1164,7 +1239,7 @@ public class DistributionTests extends AbstractJettyHomeTest int port = distribution.freePort(); String[] args2 = { "jetty.http.port=" + port, - }; + }; try (JettyHomeTester.Run run2 = distribution.start(args2)) { assertTrue(run2.awaitConsoleLogsFor("Started oejs.Server@", START_TIMEOUT, TimeUnit.SECONDS)); @@ -1180,7 +1255,7 @@ public class DistributionTests extends AbstractJettyHomeTest Path requestLog = distribution.getJettyBase().resolve("logs/test.request.log"); List loggedLines = Files.readAllLines(requestLog, StandardCharsets.UTF_8); - for (String loggedLine: loggedLines) + for (String loggedLine : loggedLines) { assertThat(loggedLine, containsString(" [foo space here] ")); }