infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-02-22 16:26:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

8377302: HttpServer::stop uses full timeout duration if handler throws

Browse Source 
Reviewed-by: vyazici, michaelm
This commit is contained in:
Daniel Fuchs 2026-02-16 17:01:07 +00:00
parent f5e1e313da
commit a08c730d5f
7 changed files with 457 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/jdk.httpserver/share/classes/sun/net/httpserver/ChunkedOutputStream.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -154,9 +154,7 @@ class ChunkedOutputStream extends FilterOutputStream
} finally {
closed = true;
}
Event e = new Event.WriteFinished(t);
t.getHttpContext().getServerImpl().addEvent(e);
t.postExchangeFinished(true);
}
public void flush() throws IOException {

14
src/jdk.httpserver/share/classes/sun/net/httpserver/Event.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -47,13 +47,15 @@ abstract sealed class Event {
}
/**
* Event indicating that writing is finished for a given exchange.
* Event indicating that the exchange is finished,
* without having necessarily read the complete
* request or sent the complete response.
* Typically, this event is posted when invoking
* the filter chain throws an exception.
*/
static final class WriteFinished extends Event {
WriteFinished(ExchangeImpl t) {
static final class ExchangeFinished extends Event {
ExchangeFinished(ExchangeImpl t) {
super(Objects.requireNonNull(t));
assert !t.writefinished;
t.writefinished = true;
}
}
}

72
src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -32,10 +32,10 @@ import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.lang.System.Logger;
import java.lang.System.Logger.Level;
import java.text.*;
import java.time.Instant;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;
import com.sun.net.httpserver.*;
import static com.sun.net.httpserver.HttpExchange.RSPBODY_EMPTY;
@ -46,7 +46,7 @@ class ExchangeImpl {
Headers reqHdrs, rspHdrs;
Request req;
String method;
boolean writefinished;
private boolean writefinished;
URI uri;
HttpConnection connection;
long reqContentLen;
@ -87,6 +87,19 @@ class ExchangeImpl {
HttpPrincipal principal;
ServerImpl server;
// Used to control that ServerImpl::endExchange is called
// exactly once for this exchange. ServerImpl::endExchange decrements
// the refcount that was incremented by calling ServerImpl::startExchange
// in this ExchangeImpl constructor.
private final AtomicBoolean ended = new AtomicBoolean();
// Used to ensure that the Event.ExchangeFinished is posted only
// once for this exchange. The Event.ExchangeFinished is what will
// eventually cause the ServerImpl::finishedLatch to be triggered,
// once the number of active exchanges reaches 0 and ServerImpl::stop
// has been requested.
private final AtomicBoolean finished = new AtomicBoolean();
ExchangeImpl(
String m, URI u, Request req, long len, HttpConnection connection
) throws IOException {
@ -107,6 +120,55 @@ class ExchangeImpl {
server.startExchange();
}
/**
* When true, writefinished indicates that all bytes expected
* by the client have been written to the response body
* outputstream, and that the response body outputstream has
* been closed. When all bytes have also been pulled from
* the request body input stream, this makes it possible to
* reuse the connection for the next request.
*/
synchronized boolean writefinished() {
return writefinished;
}
/**
* Calls ServerImpl::endExchange if not already called for this
* exchange. ServerImpl::endExchange must be called exactly once
* per exchange, and this method ensures that it is not called
* more than once for this exchange.
* @return the new (or current) value of the exchange count.
*/
int endExchange() {
// only call server.endExchange(); once per exchange
if (ended.compareAndSet(false, true)) {
return server.endExchange();
}
return server.getExchangeCount();
}
/**
* Posts the ExchangeFinished event if not already posted.
* If `writefinished` is true, marks the exchange as {@link
* #writefinished()} so that the connection can be reused.
* @param writefinished whether all bytes expected by the
* client have been writen out to the
* response body output stream.
*/
void postExchangeFinished(boolean writefinished) {
// only post ExchangeFinished once per exchange
if (finished.compareAndSet(false, true)) {
if (writefinished) {
synchronized (this) {
assert this.writefinished == false;
this.writefinished = true;
}
}
Event e = new Event.ExchangeFinished(this);
getHttpContext().getServerImpl().addEvent(e);
}
}
public Headers getRequestHeaders() {
return reqHdrs;
}
@ -140,7 +202,7 @@ class ExchangeImpl {
/* close the underlying connection if,
* a) the streams not set up yet, no response can be sent, or
* b) if the wrapper output stream is not set up, or
* c) if the close of the input/outpu stream fails
* c) if the close of the input/output stream fails
*/
try {
if (uis_orig == null || uos == null) {
@ -157,6 +219,8 @@ class ExchangeImpl {
uos.close();
} catch (IOException e) {
connection.close();
} finally {
postExchangeFinished(false);
}
}

5
src/jdk.httpserver/share/classes/sun/net/httpserver/FixedLengthOutputStream.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -94,8 +94,7 @@ class FixedLengthOutputStream extends FilterOutputStream
is.close();
} catch (IOException e) {}
}
Event e = new Event.WriteFinished(t);
t.getHttpContext().getServerImpl().addEvent(e);
t.postExchangeFinished(true);
}
// flush is a pass-through

127
src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -419,7 +419,8 @@ class ServerImpl {
// Stopping marking the state as finished if stop is requested,
// termination is in progress and exchange count is 0
if (r instanceof Event.StopRequested) {
logger.log(Level.TRACE, "Handling Stop Requested Event");
logger.log(Level.TRACE, "Handling {0} event",
r.getClass().getSimpleName());
// checking if terminating is set to true
final boolean terminatingCopy = terminating;
@ -437,10 +438,11 @@ class ServerImpl {
HttpConnection c = t.getConnection();
try {
if (r instanceof Event.WriteFinished) {
if (r instanceof Event.ExchangeFinished) {
logger.log(Level.TRACE, "Write Finished");
int exchanges = endExchange();
logger.log(Level.TRACE, "Handling {0} event",
r.getClass().getSimpleName());
int exchanges = t.endExchange();
if (terminating && exchanges == 0 && reqConnections.isEmpty()) {
finishedLatch.countDown();
}
@ -842,68 +844,77 @@ class ServerImpl {
tx = new ExchangeImpl(
method, uri, req, clen, connection
);
String chdr = headers.getFirst("Connection");
Headers rheaders = tx.getResponseHeaders();
try {
if (chdr != null && chdr.equalsIgnoreCase("close")) {
tx.close = true;
}
if (version.equalsIgnoreCase("http/1.0")) {
tx.http10 = true;
if (chdr == null) {
String chdr = headers.getFirst("Connection");
Headers rheaders = tx.getResponseHeaders();
if (chdr != null && chdr.equalsIgnoreCase("close")) {
tx.close = true;
rheaders.set("Connection", "close");
} else if (chdr.equalsIgnoreCase("keep-alive")) {
rheaders.set("Connection", "keep-alive");
int idleSeconds = (int) (ServerConfig.getIdleIntervalMillis() / 1000);
String val = "timeout=" + idleSeconds;
rheaders.set("Keep-Alive", val);
}
}
if (version.equalsIgnoreCase("http/1.0")) {
tx.http10 = true;
if (chdr == null) {
tx.close = true;
rheaders.set("Connection", "close");
} else if (chdr.equalsIgnoreCase("keep-alive")) {
rheaders.set("Connection", "keep-alive");
int idleSeconds = (int) (ServerConfig.getIdleIntervalMillis() / 1000);
String val = "timeout=" + idleSeconds;
rheaders.set("Keep-Alive", val);
}
}
if (newconnection) {
connection.setParameters(
rawin, rawout, chan, engine, sslStreams,
sslContext, protocol, ctx, rawin
);
}
/* check if client sent an Expect 100 Continue.
* In that case, need to send an interim response.
* In future API may be modified to allow app to
* be involved in this process.
*/
String exp = headers.getFirst("Expect");
if (exp != null && exp.equalsIgnoreCase("100-continue")) {
logReply(100, requestLine, null);
sendReply(
Code.HTTP_CONTINUE, false, null
);
}
/* uf is the list of filters seen/set by the user.
* sf is the list of filters established internally
* and which are not visible to the user. uc and sc
* are the corresponding Filter.Chains.
* They are linked together by a LinkHandler
* so that they can both be invoked in one call.
*/
final List<Filter> sf = ctx.getSystemFilters();
final List<Filter> uf = ctx.getFilters();
if (newconnection) {
connection.setParameters(
rawin, rawout, chan, engine, sslStreams,
sslContext, protocol, ctx, rawin
);
}
/* check if client sent an Expect 100 Continue.
* In that case, need to send an interim response.
* In future API may be modified to allow app to
* be involved in this process.
*/
String exp = headers.getFirst("Expect");
if (exp != null && exp.equalsIgnoreCase("100-continue")) {
logReply(100, requestLine, null);
sendReply(
Code.HTTP_CONTINUE, false, null
);
}
/* uf is the list of filters seen/set by the user.
* sf is the list of filters established internally
* and which are not visible to the user. uc and sc
* are the corresponding Filter.Chains.
* They are linked together by a LinkHandler
* so that they can both be invoked in one call.
*/
final List<Filter> sf = ctx.getSystemFilters();
final List<Filter> uf = ctx.getFilters();
final Filter.Chain sc = new Filter.Chain(sf, ctx.getHandler());
final Filter.Chain uc = new Filter.Chain(uf, new LinkHandler(sc));
final Filter.Chain sc = new Filter.Chain(sf, ctx.getHandler());
final Filter.Chain uc = new Filter.Chain(uf, new LinkHandler(sc));
/* set up the two stream references */
tx.getRequestBody();
tx.getResponseBody();
if (https) {
uc.doFilter(new HttpsExchangeImpl(tx));
} else {
uc.doFilter(new HttpExchangeImpl(tx));
/* set up the two stream references */
tx.getRequestBody();
tx.getResponseBody();
if (https) {
uc.doFilter(new HttpsExchangeImpl(tx));
} else {
uc.doFilter(new HttpExchangeImpl(tx));
}
} catch (Throwable t) {
// release the exchange.
logger.log(Level.TRACE, "ServerImpl.Exchange", t);
if (!tx.writefinished()) {
closeConnection(connection);
}
tx.postExchangeFinished(false);
}
} catch (Exception e) {
logger.log(Level.TRACE, "ServerImpl.Exchange", e);
if (tx == null || !tx.writefinished) {
if (tx == null || !tx.writefinished()) {
closeConnection(connection);
}
} catch (Throwable t) {

5
src/jdk.httpserver/share/classes/sun/net/httpserver/UndefLengthOutputStream.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2007, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2007, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -75,8 +75,7 @@ class UndefLengthOutputStream extends FilterOutputStream
is.close();
} catch (IOException e) {}
}
Event e = new Event.WriteFinished(t);
t.getHttpContext().getServerImpl().addEvent(e);
t.postExchangeFinished(true);
}
// flush is a pass-through

306
test/jdk/com/sun/net/httpserver/FailAndStopTest.java Normal file
View File

@ -0,0 +1,306 @@
/*
* Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/**
* @test
* @bug 8377302
* @summary HttpServer.stop() blocks indefinitely if handler throws
* @modules jdk.httpserver java.logging
* @library /test/lib
* @run main/othervm ${test.main.class}
*/
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Optional;
import java.util.function.BiPredicate;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSession;
import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler;
import com.sun.net.httpserver.HttpServer;
import com.sun.net.httpserver.HttpsConfigurator;
import com.sun.net.httpserver.HttpsServer;
import jdk.test.lib.net.SimpleSSLContext;
import jdk.test.lib.net.URIBuilder;
import jdk.test.lib.Utils;
import static com.sun.net.httpserver.HttpExchange.RSPBODY_CHUNKED;
public class FailAndStopTest implements HttpHandler {
// Keep that logger in a static field to make sure it doesn't
// get GC'ed and recreated before the HttpServer is initialized.
private static final Logger LOGGER = Logger.getLogger("com.sun.net.httpserver");
private static final String BODY = "OK";
static enum TestCases {
FAILNOW("failNow", TestCases::shouldAlwaysFail),
ASSERTNOW("assertNow", TestCases::shouldAlwaysFail),
RESPANDFAIL("failAfterResponseStatus", TestCases::shouldFailExceptForHead),
RESPANDASSERT("assertAfterResponseStatus", TestCases::shouldFailExceptForHead),
CLOSEAFTERRESP("closeExchangeAfterResponseStatus", TestCases::shouldFailExceptForHead),
BODYANDFAIL("failAfterBody", TestCases::shouldFailExceptForHeadOrHttps),
BODYANDASSERT("assertAfterBody", TestCases::shouldFailExceptForHeadOrHttps),
CLOSEBEFOREOS("closeExchangeBeforeOS", TestCases::shouldNeverFail),
CLOSEANDRETURN("closeAndReturn", TestCases::shouldNeverFail),
CLOSEANDFAIL("closeAndFail", TestCases::shouldNeverFail),
CLOSEANDASSERT("closeAndAssert", TestCases::shouldNeverFail);
private final String query;
private BiPredicate<String,String> shouldFail;
TestCases(String query, BiPredicate<String,String> shouldFail) {
this.query = query;
this.shouldFail = shouldFail;
}
boolean shouldFail(String scheme, String method) {
// in case of HEAD method the client should not
// fail if we throw after sending response headers
return shouldFail.test(scheme, method);
}
private static boolean shouldAlwaysFail(String scheme, String method) {
return true;
}
private static boolean shouldNeverFail(String scheme, String method) {
return false;
}
private static boolean shouldFailExceptForHead(String scheme, String method) {
return !"HEAD".equals(method);
}
private static boolean shouldFailExceptForHeadOrHttps(String scheme, String method) {
// When using https, the buffered response bytes may be sent
// when the connection is closed, in which case the full body
// will be correctly received, and the client connection
// will not fail. With plain http, the bytes are not sent and
// the client fails with premature end of file.
return !"HEAD".equals(method) && !"https".equalsIgnoreCase(scheme);
}
}
@Override
public void handle(HttpExchange ex) throws IOException {
String query = ex.getRequestURI().getRawQuery();
TestCases step = TestCases.FAILNOW;
if (query == null || query.equals(step.query)) {
System.out.println("Server: " + step);
throw new NullPointerException("Got you!");
}
step = TestCases.ASSERTNOW;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new AssertionError("Got you!");
}
byte[] body = BODY.getBytes(StandardCharsets.UTF_8);
ex.sendResponseHeaders(200, body.length);
step = TestCases.RESPANDFAIL;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new NullPointerException("Got you!");
}
step = TestCases.RESPANDASSERT;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new AssertionError("Got you!");
}
step = TestCases.CLOSEAFTERRESP;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
ex.close();
return;
}
if (!"HEAD".equals(ex.getRequestMethod())) {
ex.getResponseBody().write(body);
}
step = TestCases.BODYANDFAIL;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new NullPointerException("Got you!");
}
step = TestCases.BODYANDASSERT;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new AssertionError("Got you!");
}
step = TestCases.CLOSEBEFOREOS;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
ex.close();
return;
}
System.out.println("Server: closing response body");
ex.getResponseBody().close();
step = TestCases.CLOSEANDRETURN;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
ex.close();
return;
}
step = TestCases.CLOSEANDFAIL;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new NullPointerException("Got you!");
}
step = TestCases.CLOSEANDASSERT;
if (query.equals(step.query)) {
System.out.println("Server: " + step);
throw new AssertionError("Got you!");
}
}
private static void enableHttpServerLogging() {
// set HttpServer's logger to ALL
LOGGER.setLevel(Level.ALL);
// get the root logger, get its first handler (by default
// it's a ConsoleHandler), and set its level to ALL (by
// default its level is INFO).
Logger.getLogger("").getHandlers()[0].setLevel(Level.ALL);
}
public static void main(String[] args) throws Exception {
enableHttpServerLogging();
// test with GET
for (var test : TestCases.values()) {
test(test, Optional.empty(), "http");
test(test, Optional.empty(), "https");
}
// test with HEAD
for (var test : TestCases.values()) {
test(test, Optional.of("HEAD"), "http");
test(test, Optional.of("HEAD"), "https");
}
}
private static SSLContext initSSLContext(boolean secure) {
SSLContext context = secure ? SimpleSSLContext.findSSLContext() : null;
if (secure) {
SSLContext.setDefault(context);
}
return context;
}
private static HttpServer createHttpServer(SSLContext context) throws IOException {
var address = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
if (context != null) {
var server = HttpsServer.create(address, 0);
server.setHttpsConfigurator(new HttpsConfigurator(context));
return server;
} else {
return HttpServer.create(address, 0);
}
}
private static HttpURLConnection createConnection(URL url, boolean secure)
throws IOException {
HttpURLConnection urlc = (HttpURLConnection) url.openConnection(Proxy.NO_PROXY);
if (secure) {
((HttpsURLConnection)urlc).setHostnameVerifier(new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true;
}
});
}
return urlc;
}
private static void test(TestCases test, Optional<String> method, String scheme)
throws Exception {
boolean secure = "https".equalsIgnoreCase(scheme);
SSLContext context = initSSLContext(secure);
HttpServer server = createHttpServer(context);
System.out.println("Test: " + method.orElse("GET") + " " + test.query);
System.out.println("Server listening at: " + server.getAddress());
try {
server.createContext("/FailAndStopTest/", new FailAndStopTest());
server.start();
URL url = URIBuilder.newBuilder()
.scheme(scheme)
.loopback()
.port(server.getAddress().getPort())
.path("/FailAndStopTest/")
.query(test.query)
.toURLUnchecked();
System.out.println("Connecting to: " + url);
HttpURLConnection urlc = createConnection(url, secure);
if (method.isPresent()) urlc.setRequestMethod(method.get());
try {
System.out.println("Client: Response code received: " + urlc.getResponseCode());
InputStream is = urlc.getInputStream();
String body = new String(is.readAllBytes(), StandardCharsets.UTF_8);
is.close();
System.out.printf("Client: read body: \"%s\"%n", body);
if (test.shouldFail(scheme, urlc.getRequestMethod())) {
throw new AssertionError(test.query + ": test did not fail");
}
if (!method.orElse("GET").equals("HEAD")) {
if (!BODY.equals(body)) {
throw new AssertionError(
String.format("\"%s\" != \"%s\"", body, BODY));
}
} else if (!body.isEmpty()) {
throw new AssertionError("Body is not empty: " + body);
}
} catch (IOException so) {
if (test.shouldFail(scheme, urlc.getRequestMethod())) {
// expected
System.out.println(test.query + ": Got expected exception: " + so);
} else if (!test.shouldFail("http", urlc.getRequestMethod())) {
// When using https, the buffered response bytes may be sent
// when the connection is closed, in which case the full body
// will be correctly received, and the client connection
// will not fail. With plain http, the bytes are not sent and
// the client fails with premature end of file.
// So only fail here if the test should not fail with plain
// http - we want to accept possible exception for https...
throw new AssertionError(
String.format("%s: test failed with %s", test.query, so), so);
} else {
System.out.printf("%s: WARNING: unexpected exception: %s%n", test.query, so);
// should only happen in those two cases:
assert secure && !"HEAD".equals(method) &&
(test == TestCases.BODYANDFAIL || test == TestCases.BODYANDASSERT);
}
}
} finally {
// if not fixed will cause the test to fail in jtreg timeout
server.stop((int)Utils.adjustTimeout(5000));
System.out.println("Server stopped as expected");
}
}
}