diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java index cac5341a49c1..019b1504c150 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http; import java.nio.ByteBuffer; +import java.util.EnumSet; import org.eclipse.jetty.util.Index; import org.eclipse.jetty.util.StringUtil; @@ -132,6 +133,18 @@ public enum HttpHeader C_STATUS(":status", true), C_PROTOCOL(":protocol"); + public static final EnumSet CONTENT_HEADERS; + public static final EnumSet CONTENT_HEADERS_304; + + static + { + CONTENT_HEADERS = EnumSet.of( + CONTENT_TYPE, CONTENT_LENGTH, CONTENT_ENCODING, CONTENT_LANGUAGE, CONTENT_RANGE, CONTENT_MD5, CONTENT_LOCATION, TRANSFER_ENCODING, CACHE_CONTROL, LAST_MODIFIED, EXPIRES, VARY, ETAG + ); + CONTENT_HEADERS_304 = EnumSet.copyOf(CONTENT_HEADERS); + CONTENT_HEADERS_304.remove(ETAG); + } + public static final Index CACHE = new Index.Builder() .caseSensitive(false) .withAll(HttpHeader.values(), HttpHeader::toString) diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/StreamResetTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/StreamResetTest.java index 15e19881cd86..15e17a05b816 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/StreamResetTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/StreamResetTest.java @@ -556,7 +556,7 @@ public void succeeded() }); // Wait for WINDOW_UPDATEs to be processed by the client. - await().atMost(1000, TimeUnit.SECONDS).until(() -> ((HTTP2Session)client).updateSendWindow(0), Matchers.greaterThan(0)); + await().atMost(5, TimeUnit.SECONDS).until(() -> ((HTTP2Session)client).updateSendWindow(0), Matchers.greaterThan(0)); latch.set(new CountDownLatch(2 * streams.size())); // Notify all blocked threads to wakeup. diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index f1b211e2e1e5..3f3318e91141 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -250,7 +250,7 @@ default void push(MetaData.Request resource) /** *

Adds a listener for asynchronous errors.

*

The listener is a predicate function that should return {@code true} to indicate - * that the function has completed (either successfully or with a failure) the callback + * that the function will complete (either successfully or with a failure) the callback * received from {@link org.eclipse.jetty.server.Handler#handle(Request, Response, Callback)}, or * {@code false} otherwise.

*

Listeners are processed in sequence, and the first that returns {@code true} diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index 6426e084af94..84f723641e71 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -41,6 +41,7 @@ import org.eclipse.jetty.http.QuotedQualityCSV; import org.eclipse.jetty.io.ByteBufferOutputStream; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.Retainable; import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; @@ -86,6 +87,8 @@ public boolean errorPageForMethod(String method) @Override public boolean handle(Request request, Response response, Callback callback) { + if (LOG.isDebugEnabled()) + LOG.debug("handle({}, {}, {})", request, response, callback); if (_cacheControl != null) response.getHeaders().put(_cacheControl); @@ -256,22 +259,7 @@ else if (charsets.contains(StandardCharsets.ISO_8859_1)) } response.getHeaders().put(type.getContentTypeField(charset)); - response.write(true, buffer.getByteBuffer(), new Callback.Nested(callback) - { - @Override - public void succeeded() - { - buffer.release(); - super.succeeded(); - } - - @Override - public void failed(Throwable x) - { - buffer.release(); - super.failed(x); - } - }); + response.write(true, buffer.getByteBuffer(), new WriteErrorCallback(callback, buffer)); return true; } @@ -576,5 +564,34 @@ public Set getAttributeNameSet() names.add(ERROR_EXCEPTION); return names; } + + @Override + public String toString() + { + return "%s@%x:%s".formatted(getClass().getSimpleName(), hashCode(), getWrapped()); + } + } + + /** + * The callback used by + * {@link ErrorHandler#generateAcceptableResponse(Request, Response, Callback, String, List, int, String, Throwable)} + * when calling {@link Response#write(boolean, ByteBuffer, Callback)} to wrap the passed in {@link Callback} + * so that the {@link RetainableByteBuffer} used can be released. + */ + private static class WriteErrorCallback extends Callback.Nested + { + private final Retainable _retainable; + + public WriteErrorCallback(Callback callback, Retainable retainable) + { + super(callback); + _retainable = retainable; + } + + @Override + public void completed() + { + _retainable.release(); + } } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index 90c2bc7a03e2..74b9e9f34680 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -77,27 +77,27 @@ */ public class HttpChannelState implements HttpChannel, Components { - private static final Logger LOG = LoggerFactory.getLogger(HttpChannelState.class); - private static final Throwable DO_NOT_SEND = new Throwable("No Send"); - private static final MetaData.Request ERROR_REQUEST = new MetaData.Request("GET", HttpURI.from("/"), HttpVersion.HTTP_1_0, HttpFields.EMPTY); - private static final HttpField SERVER_VERSION = new PreEncodedHttpField(HttpHeader.SERVER, HttpConfiguration.SERVER_VERSION); - private static final HttpField POWERED_BY = new PreEncodedHttpField(HttpHeader.X_POWERED_BY, HttpConfiguration.SERVER_VERSION); - /** * The state of the written response */ - enum WriteState + private enum StreamSendState { - /** Not yet written */ - NOT_LAST, + /** Last content not yet sent */ + SENDING, - /** Last content written, but write not yet completed */ - LAST_WRITTEN, + /** Last content sent, but send not yet completed */ + LAST_SENDING, - /** Last content written and completed */ - LAST_WRITE_COMPLETED, + /** Last content sent and completed */ + LAST_COMPLETE } + private static final Logger LOG = LoggerFactory.getLogger(HttpChannelState.class); + private static final Throwable DO_NOT_SEND = new Throwable("No Send"); + private static final MetaData.Request ERROR_REQUEST = new MetaData.Request("GET", HttpURI.from("/"), HttpVersion.HTTP_1_0, HttpFields.EMPTY); + private static final HttpField SERVER_VERSION = new PreEncodedHttpField(HttpHeader.SERVER, HttpConfiguration.SERVER_VERSION); + private static final HttpField POWERED_BY = new PreEncodedHttpField(HttpHeader.X_POWERED_BY, HttpConfiguration.SERVER_VERSION); + private final AutoLock _lock = new AutoLock(); private final HandlerInvoker _handlerInvoker = new HandlerInvoker(); private final ConnectionMetaData _connectionMetaData; @@ -106,15 +106,15 @@ enum WriteState private final ResponseHttpFields _responseHeaders = new ResponseHttpFields(); private Thread _handling; private boolean _handled; - private WriteState _writeState = WriteState.NOT_LAST; + private StreamSendState _streamSendState = StreamSendState.SENDING; private boolean _callbackCompleted = false; - private Throwable _failure; private ChannelRequest _request; + private ChannelResponse _response; private HttpStream _stream; private long _committedContentLength = -1; private Runnable _onContentAvailable; - private Callback _writeCallback; private Content.Chunk.Error _error; + private Throwable _failure; private Predicate _onError; private Attributes _cache; @@ -135,23 +135,23 @@ public void recycle() // Break the link between request and channel, so that // applications cannot use request/response/callback anymore. - _request._httpChannel = null; + _request._httpChannelState = null; // Break the links with the upper and lower layers. _request = null; + _response = null; _stream = null; + _streamSendState = StreamSendState.SENDING; // Recycle. _requestAttributes.clearAttributes(); _responseHeaders.reset(); _handling = null; _handled = false; - _writeState = WriteState.NOT_LAST; _callbackCompleted = false; _failure = null; _committedContentLength = -1; _onContentAvailable = null; - _writeCallback = null; _error = null; _onError = null; } @@ -259,8 +259,9 @@ public Runnable onRequest(MetaData.Request request) if (_request != null) throw new IllegalStateException("duplicate request"); _request = new ChannelRequest(this, request); + _response = new ChannelResponse(_request); - HttpFields.Mutable responseHeaders = _request._response.getHeaders(); + HttpFields.Mutable responseHeaders = _response.getHeaders(); if (getHttpConfiguration().getSendServerVersion()) responseHeaders.add(SERVER_VERSION); if (getHttpConfiguration().getSendXPoweredBy()) @@ -285,7 +286,7 @@ public Response getResponse() { try (AutoLock ignored = _lock.lock()) { - return _request == null ? null : _request._response; + return _response; } } @@ -344,9 +345,10 @@ public Runnable onFailure(Throwable x) // If the channel doesn't have a request, then the error must have occurred during the parsing of // the request line / headers, so make a temp request for logging and producing an error response. _request = new ChannelRequest(this, ERROR_REQUEST); + _response = new ChannelResponse(_request); } - // Remember the error and arrange for any subsequent reads, demands or writes to fail with this error. + // Set the error to arrange for any subsequent reads, demands or writes to fail. if (_error == null) { _error = Content.Chunk.from(x); @@ -357,48 +359,55 @@ else if (_error.getCause() != x) return null; } - // Invoke onContentAvailable() if we are currently demanding. - Runnable invokeOnContentAvailable = _onContentAvailable; - _onContentAvailable = null; + // If not handled, then we just fail the request callback + if (!_handled && _handling == null) + { + task = () -> _request._callback.failed(x); + } + else + { + // if we are currently demanding, take the onContentAvailable runnable to invoke below. + Runnable invokeOnContentAvailable = _onContentAvailable; + _onContentAvailable = null; - // If a write() is in progress, fail the write callback. - Callback writeCallback = _writeCallback; - _writeCallback = null; - Runnable invokeWriteFailure = writeCallback == null ? null : () -> writeCallback.failed(x); + // If a write call is in progress, take the writeCallback to fail below + Runnable invokeWriteFailure = _response.lockedFailWrite(x); - ChannelRequest request = _request; - Runnable invokeCallback = () -> - { - // Only fail the callback if the request was not accepted. - boolean handling; - try (AutoLock ignore = _lock.lock()) - { - handling = _handling != null || _handled; - } - if (handling) + // Create runnable to invoke any onError listeners + ChannelRequest request = _request; + Runnable invokeListeners = () -> { - if (LOG.isDebugEnabled()) - LOG.debug("already handled, skipping failing callback in {}", HttpChannelState.this); - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("failing callback in {}", this, x); - request._callback.failed(x); - } - }; + Predicate onError; + try (AutoLock ignore = _lock.lock()) + { + onError = _onError; + } - // Invoke error listeners. - Predicate onError = _onError; - _onError = null; - Runnable invokeOnErrorAndCallback = onError == null ? invokeCallback : () -> - { - if (!onError.test(x)) - invokeCallback.run(); - }; + try + { + if (LOG.isDebugEnabled()) + LOG.debug("invokeListeners {} {}", HttpChannelState.this, onError, x); + if (onError.test(x)) + return; + } + catch (Throwable throwable) + { + if (ExceptionUtil.areNotAssociated(x, throwable)) + x.addSuppressed(throwable); + } - // Serialize all the error actions. - task = _serializedInvoker.offer(invokeOnContentAvailable, invokeWriteFailure, invokeOnErrorAndCallback); + // If the application has not been otherwise informed of the failure + if (invokeOnContentAvailable == null && invokeWriteFailure == null) + { + if (LOG.isDebugEnabled()) + LOG.debug("failing callback in {}", this, x); + request._callback.failed(x); + } + }; + + // Serialize all the error actions. + task = _serializedInvoker.offer(invokeOnContentAvailable, invokeWriteFailure, invokeListeners); + } } // Consume content as soon as possible to open any flow control window. @@ -443,41 +452,62 @@ private void resetResponse() } } - private Throwable lockedCheckWrite(boolean last, long length) + private Throwable lockedStreamSend(boolean last, long length) { - assert _request._lock.isHeldByCurrentThread(); + assert _lock.isHeldByCurrentThread(); - return switch (_writeState) + return switch (_streamSendState) { - case NOT_LAST -> + case SENDING -> { - _writeState = last ? WriteState.LAST_WRITTEN : WriteState.NOT_LAST; - _request._response._contentBytesWritten += length; + _streamSendState = last ? StreamSendState.LAST_SENDING : StreamSendState.SENDING; yield null; } // There are many instances of code that wants to ensure the output is closed, so - // it does a redundant write(true, callback). The DO_NOT_SEND option supports this by - // turning such writes into a NOOP. - case LAST_WRITTEN, LAST_WRITE_COMPLETED -> (length > 0) + // it does a redundant write(true, callback). Other code may do a write(false, callback) to ensure + // they are flushed. The DO_NOT_SEND option supports these by turning such writes into a NOOP. + case LAST_SENDING, LAST_COMPLETE -> (length > 0) ? new IllegalStateException("last already written") : DO_NOT_SEND; }; } + private void lockedStreamSendCompleted(boolean success) + { + assert _lock.isHeldByCurrentThread(); + if (_streamSendState == StreamSendState.LAST_SENDING) + _streamSendState = success ? StreamSendState.LAST_COMPLETE : StreamSendState.SENDING; + } + + private boolean lockedIsLastStreamSendCompleted() + { + assert _lock.isHeldByCurrentThread(); + return _streamSendState == StreamSendState.LAST_COMPLETE; + } + + private boolean lockedLastStreamSend() + { + assert _lock.isHeldByCurrentThread(); + if (_streamSendState != StreamSendState.SENDING) + return false; + + _streamSendState = StreamSendState.LAST_SENDING; + return true; + } + @Override public String toString() { try (AutoLock ignored = _lock.lock()) { - return String.format("%s@%x{handling=%s, handled=%b, writeState=%s, completed=%b, writeCallback=%s, request=%s}", + return String.format("%s@%x{handling=%s, handled=%b, send=%s, completed=%b, request=%s}", this.getClass().getSimpleName(), hashCode(), _handling, _handled, - _writeState, + _streamSendState, _callbackCompleted, - _writeCallback, _request); } } @@ -492,18 +522,19 @@ public void run() // However, if a thread calling the application throws, then that exception will be reported // to the callback. ChannelRequest request; + ChannelResponse response; try (AutoLock ignored = _lock.lock()) { assert _handling == null && !_handled; _handling = Thread.currentThread(); request = _request; + response = _response; } if (LOG.isDebugEnabled()) LOG.debug("invoking handler in {}", HttpChannelState.this); Server server = _connectionMetaData.getConnector().getServer(); - Throwable failure = null; try { if (!HttpMethod.PRI.is(request.getMethod()) && @@ -526,7 +557,7 @@ public void run() HttpConfiguration configuration = getHttpConfiguration(); Request customized = request; - HttpFields.Mutable responseHeaders = request._response.getHeaders(); + HttpFields.Mutable responseHeaders = response.getHeaders(); for (HttpConfiguration.Customizer customizer : configuration.getCustomizers()) { Request next = customizer.customize(customized, responseHeaders); @@ -536,44 +567,43 @@ public void run() if (customized != request && server.getRequestLog() != null) request.setLoggedRequest(customized); - if (!server.handle(customized, request._response, request._callback)) - Response.writeError(customized, request._response, request._callback, HttpStatus.NOT_FOUND_404); + if (!server.handle(customized, response, request._callback)) + Response.writeError(customized, response, request._callback, HttpStatus.NOT_FOUND_404); } catch (Throwable t) { - failure = t; + request._callback.failed(t); } HttpStream stream; - boolean completeStream = false; + Throwable failure; + boolean completeStream; + boolean callbackCompleted; + boolean lastStreamSendComplete; try (AutoLock ignored = _lock.lock()) { stream = _stream; + _handling = null; + _handled = true; + failure = _failure; + callbackCompleted = _callbackCompleted; + lastStreamSendComplete = lockedIsLastStreamSendCompleted(); + completeStream = callbackCompleted && lastStreamSendComplete; - if (failure == null) - { - _handling = null; - _handled = true; - failure = ExceptionUtil.combine(_failure, failure); - completeStream = _callbackCompleted && (failure != null || _writeState == WriteState.LAST_WRITE_COMPLETED); - } + if (LOG.isDebugEnabled()) + LOG.debug("handler invoked: completeStream={} failure={} callbackCompleted={} {}", completeStream, failure, callbackCompleted, HttpChannelState.this); } - if (failure != null) - { - request._callback.failed(failure); + if (LOG.isDebugEnabled()) + LOG.debug("stream={}, failure={}, callbackCompleted={}, completeStream={}", stream, failure, callbackCompleted, completeStream); - try (AutoLock ignored = _lock.lock()) - { - _handling = null; - _handled = true; - failure = ExceptionUtil.combine(_failure, failure); - completeStream = _callbackCompleted && (failure != null || _writeState == WriteState.LAST_WRITE_COMPLETED); - } - } if (completeStream) + { + if (LOG.isDebugEnabled()) + LOG.debug("completeStream({}, {})", stream, Objects.toString(failure)); completeStream(stream, failure); + } } /** @@ -586,8 +616,8 @@ public void succeeded() boolean completeStream; try (AutoLock ignored = _lock.lock()) { - _writeState = WriteState.LAST_WRITE_COMPLETED; assert _callbackCompleted; + _streamSendState = StreamSendState.LAST_COMPLETE; completeStream = _handling == null; stream = _stream; } @@ -597,7 +627,7 @@ public void succeeded() } /** - * Called only as {@link Callback} by last write from {@link ChannelCallback#succeeded} + * Called only as {@link Callback} by last send from {@link ChannelCallback#succeeded} */ @Override public void failed(Throwable failure) @@ -606,17 +636,11 @@ public void failed(Throwable failure) boolean completeStream; try (AutoLock ignored = _lock.lock()) { - _writeState = WriteState.LAST_WRITE_COMPLETED; assert _callbackCompleted; + _streamSendState = StreamSendState.LAST_COMPLETE; completeStream = _handling == null; stream = _stream; - if (_failure == null) - _failure = failure; - else if (ExceptionUtil.areNotAssociated(_failure, failure)) - { - _failure.addSuppressed(failure); - failure = _failure; - } + failure = _failure = ExceptionUtil.combine(_failure, failure); } if (completeStream) completeStream(stream, failure); @@ -632,7 +656,7 @@ private void completeStream(HttpStream stream, Throwable failure) if (LOG.isDebugEnabled()) LOG.debug("logging {}", HttpChannelState.this); - requestLog.log(_request.getLoggedRequest(), _request._response); + requestLog.log(_request.getLoggedRequest(), _response); } // Clean up any multipart tmp files and release any associated resources. @@ -664,21 +688,19 @@ public static class ChannelRequest implements Attributes, Request private final String _id; private final ConnectionMetaData _connectionMetaData; private final MetaData.Request _metaData; - private final ChannelResponse _response; private final AutoLock _lock; private final LongAdder _contentBytesRead = new LongAdder(); - private HttpChannelState _httpChannel; + private HttpChannelState _httpChannelState; private Request _loggedRequest; private HttpFields _trailers; - ChannelRequest(HttpChannelState httpChannel, MetaData.Request metaData) + ChannelRequest(HttpChannelState httpChannelState, MetaData.Request metaData) { - _httpChannel = Objects.requireNonNull(httpChannel); - _id = httpChannel.getHttpStream().getId(); // Copy ID now, as stream will ultimately be nulled - _connectionMetaData = httpChannel.getConnectionMetaData(); + _httpChannelState = Objects.requireNonNull(httpChannelState); + _id = httpChannelState.getHttpStream().getId(); // Copy ID now, as stream will ultimately be nulled + _connectionMetaData = httpChannelState.getConnectionMetaData(); _metaData = Objects.requireNonNull(metaData); - _response = new ChannelResponse(this); - _lock = httpChannel._lock; + _lock = httpChannelState._lock; } public void setLoggedRequest(Request request) @@ -693,7 +715,7 @@ public Request getLoggedRequest() HttpStream getHttpStream() { - return getHttpChannel()._stream; + return getHttpChannelState()._stream; } public long getContentBytesRead() @@ -704,7 +726,7 @@ public long getContentBytesRead() @Override public Object getAttribute(String name) { - HttpChannelState httpChannel = getHttpChannel(); + HttpChannelState httpChannel = getHttpChannelState(); if (name.startsWith("org.eclipse.jetty")) { if (Server.class.getName().equals(name)) @@ -723,7 +745,7 @@ public Object getAttribute(String name) @Override public Object removeAttribute(String name) { - return getHttpChannel()._requestAttributes.removeAttribute(name); + return getHttpChannelState()._requestAttributes.removeAttribute(name); } @Override @@ -731,19 +753,19 @@ public Object setAttribute(String name, Object attribute) { if (Server.class.getName().equals(name) || HttpChannelState.class.getName().equals(name) || HttpConnection.class.getName().equals(name)) return null; - return getHttpChannel()._requestAttributes.setAttribute(name, attribute); + return getHttpChannelState()._requestAttributes.setAttribute(name, attribute); } @Override public Set getAttributeNameSet() { - return getHttpChannel()._requestAttributes.getAttributeNameSet(); + return getHttpChannelState()._requestAttributes.getAttributeNameSet(); } @Override public void clearAttributes() { - getHttpChannel()._requestAttributes.clearAttributes(); + getHttpChannelState()._requestAttributes.clearAttributes(); } @Override @@ -755,7 +777,7 @@ public String getId() @Override public Components getComponents() { - return getHttpChannel(); + return getHttpChannelState(); } @Override @@ -764,20 +786,20 @@ public ConnectionMetaData getConnectionMetaData() return _connectionMetaData; } - HttpChannelState getHttpChannel() + HttpChannelState getHttpChannelState() { try (AutoLock ignore = _lock.lock()) { - return lockedGetHttpChannel(); + return lockedGetHttpChannelState(); } } - private HttpChannelState lockedGetHttpChannel() + private HttpChannelState lockedGetHttpChannelState() { assert _lock.isHeldByCurrentThread(); - if (_httpChannel == null) + if (_httpChannelState == null) throw new IllegalStateException("channel already completed"); - return _httpChannel; + return _httpChannelState; } @Override @@ -819,7 +841,7 @@ public long getTimeStamp() @Override public long getNanoTime() { - HttpStream stream = _httpChannel.getHttpStream(); + HttpStream stream = _httpChannelState.getHttpStream(); if (stream != null) return stream.getNanoTime(); throw new IllegalStateException(); @@ -843,7 +865,7 @@ public Content.Chunk read() HttpStream stream; try (AutoLock ignored = _lock.lock()) { - HttpChannelState httpChannel = lockedGetHttpChannel(); + HttpChannelState httpChannel = lockedGetHttpChannelState(); Content.Chunk error = httpChannel._error; if (error != null) @@ -872,7 +894,7 @@ public boolean consumeAvailable() HttpStream stream; try (AutoLock ignored = _lock.lock()) { - HttpChannelState httpChannel = lockedGetHttpChannel(); + HttpChannelState httpChannel = lockedGetHttpChannelState(); stream = httpChannel._stream; } @@ -886,7 +908,7 @@ public void demand(Runnable demandCallback) HttpStream stream; try (AutoLock ignored = _lock.lock()) { - HttpChannelState httpChannel = lockedGetHttpChannel(); + HttpChannelState httpChannel = lockedGetHttpChannelState(); if (LOG.isDebugEnabled()) LOG.debug("demand {}", httpChannel); @@ -904,7 +926,7 @@ public void demand(Runnable demandCallback) if (error) // TODO: can we avoid re-grabbing the lock to get the HttpChannel? - getHttpChannel()._serializedInvoker.run(demandCallback); + getHttpChannelState()._serializedInvoker.run(demandCallback); else stream.demand(); } @@ -912,7 +934,7 @@ public void demand(Runnable demandCallback) @Override public void fail(Throwable failure) { - // TODO + _httpChannelState.onFailure(failure); } @Override @@ -926,7 +948,7 @@ public boolean addErrorListener(Predicate onError) { try (AutoLock ignored = _lock.lock()) { - HttpChannelState httpChannel = lockedGetHttpChannel(); + HttpChannelState httpChannel = lockedGetHttpChannelState(); if (httpChannel._error != null) return false; @@ -958,7 +980,7 @@ public TunnelSupport getTunnelSupport() @Override public void addHttpStreamWrapper(Function wrapper) { - getHttpChannel().addHttpStreamWrapper(wrapper); + getHttpChannelState().addHttpStreamWrapper(wrapper); } @Override @@ -974,18 +996,58 @@ public String toString() } } + /** + * The Channel's implementation of the {@link Response} API. + * Also is a {@link Callback} used by the {@link #write(boolean, ByteBuffer, Callback)} + * method when calling + * {@link HttpStream#send(MetaData.Request, MetaData.Response, boolean, ByteBuffer, Callback)} + */ public static class ChannelResponse implements Response, Callback { private final ChannelRequest _request; private int _status; private long _contentBytesWritten; private Supplier _trailers; + private Callback _writeCallback; + protected boolean _errorMode; private ChannelResponse(ChannelRequest request) { _request = request; } + private void lockedPrepareErrorResponse() + { + // reset the response state, so we can generate an error response, + // remembering any server or date headers (probably a nicer way of doing this). + HttpChannelState httpChannelState = _request.lockedGetHttpChannelState(); + HttpField serverField = httpChannelState._responseHeaders.getField(HttpHeader.SERVER); + HttpField dateField = httpChannelState._responseHeaders.getField(HttpHeader.DATE); + httpChannelState._responseHeaders.reset(); + httpChannelState._committedContentLength = -1; + reset(); + if (serverField != null) + httpChannelState._responseHeaders.put(serverField); + if (dateField != null) + httpChannelState._responseHeaders.put(dateField); + setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); + _errorMode = true; + } + + private boolean lockedIsWriting() + { + assert _request._lock.isHeldByCurrentThread(); + return _writeCallback != null; + } + + private Runnable lockedFailWrite(Throwable x) + { + assert _request._lock.isHeldByCurrentThread(); + Callback writeCallback = _writeCallback; + _writeCallback = null; + return writeCallback == null ? null : () -> writeCallback.failed(x); + } + public long getContentBytesWritten() { return _contentBytesWritten; @@ -1013,7 +1075,7 @@ public void setStatus(int code) @Override public HttpFields.Mutable getHeaders() { - return _request.getHttpChannel()._responseHeaders; + return _request.getHttpChannelState()._responseHeaders; } @Override @@ -1040,46 +1102,58 @@ public void write(boolean last, ByteBuffer content, Callback callback) long length = BufferUtil.length(content); long totalWritten; - HttpChannelState httpChannel; + HttpChannelState httpChannelState; HttpStream stream = null; - Throwable failure; + Throwable failure = null; MetaData.Response responseMetaData = null; try (AutoLock ignored = _request._lock.lock()) { - httpChannel = _request.lockedGetHttpChannel(); + httpChannelState = _request.lockedGetHttpChannelState(); + long committedContentLength = httpChannelState._committedContentLength; + totalWritten = _contentBytesWritten + length; + long contentLength = committedContentLength >= 0 ? committedContentLength : getHeaders().getLongField(HttpHeader.CONTENT_LENGTH); - if (httpChannel._writeCallback != null) + if (_writeCallback != null) failure = new IllegalStateException("write pending"); - else if (httpChannel._error != null) - failure = httpChannel._error.getCause(); - else - failure = httpChannel.lockedCheckWrite(last, length); - - if (failure == null) + else if (!_errorMode && httpChannelState._error != null) + failure = httpChannelState._error.getCause(); + else if (contentLength >= 0) { - httpChannel._writeCallback = callback; - - stream = httpChannel._stream; - totalWritten = _contentBytesWritten; - - if (httpChannel._responseHeaders.commit()) - responseMetaData = lockedPrepareResponse(httpChannel, last); - // If the content length were not compatible with what was written, then we need to abort. - long committedContentLength = httpChannel._committedContentLength; - if (committedContentLength >= 0) + String lengthError = (totalWritten > contentLength) ? "written %d > %d content-length" + : (last && totalWritten < contentLength) ? "written %d < %d content-length" : null; + if (lengthError != null) { - String lengthError = (totalWritten > committedContentLength) ? "written %d > %d content-length" - : (last && totalWritten < committedContentLength) ? "written %d < %d content-length" : null; - if (lengthError != null) - { - String message = lengthError.formatted(totalWritten, committedContentLength); - if (LOG.isDebugEnabled()) - LOG.debug("fail {} {}", callback, message); - failure = new IOException(message); - } + String message = lengthError.formatted(totalWritten, contentLength); + if (LOG.isDebugEnabled()) + LOG.debug("fail {} {}", callback, message); + failure = new IOException(message); } } + + // If no failure by this point, we can try to send + if (failure == null) + failure = httpChannelState.lockedStreamSend(last, length); + + // Have we failed in some way? + if (failure == DO_NOT_SEND) + { + httpChannelState._serializedInvoker.run(callback::succeeded); + } + else if (failure != null) + { + Throwable throwable = failure; + httpChannelState._serializedInvoker.run(() -> callback.failed(throwable)); + } + else + { + // We have not failed, so we will do a stream send + _writeCallback = callback; + _contentBytesWritten = totalWritten; + stream = httpChannelState._stream; + if (httpChannelState._responseHeaders.commit()) + responseMetaData = lockedPrepareResponse(httpChannelState, last); + } } if (failure == null) @@ -1088,51 +1162,58 @@ else if (httpChannel._error != null) LOG.debug("writing last={} {} {}", last, BufferUtil.toDetailString(content), this); stream.send(_request._metaData, responseMetaData, last, content, this); } - else if (failure == DO_NOT_SEND) - { - httpChannel._serializedInvoker.run(callback::succeeded); - } - else - { - Throwable t = failure; - httpChannel._serializedInvoker.run(() -> callback.failed(t)); - } } + /** + * Called when the call to + * {@link HttpStream#send(MetaData.Request, MetaData.Response, boolean, ByteBuffer, Callback)} + * made by {@link ChannelResponse#write(boolean, ByteBuffer, Callback)} succeeds. + * The implementation maintains the {@link #_streamSendState} before taking + * and serializing the call to the {@link #_writeCallback}, which was set by the call to {@code write}. + */ @Override public void succeeded() { + if (LOG.isDebugEnabled()) + LOG.debug("write succeeded {}", this); // Called when an individual write succeeds. Callback callback; HttpChannelState httpChannel; try (AutoLock ignored = _request._lock.lock()) { - httpChannel = _request.lockedGetHttpChannel(); - callback = httpChannel._writeCallback; - httpChannel._writeCallback = null; - if (httpChannel._writeState == WriteState.LAST_WRITTEN) - httpChannel._writeState = WriteState.LAST_WRITE_COMPLETED; + httpChannel = _request.lockedGetHttpChannelState(); + callback = _writeCallback; + _writeCallback = null; + httpChannel.lockedStreamSendCompleted(true); } - if (LOG.isDebugEnabled()) - LOG.debug("write succeeded {} {}", callback, this); if (callback != null) httpChannel._serializedInvoker.run(callback::succeeded); } + /** + * Called when the call to + * {@link HttpStream#send(MetaData.Request, MetaData.Response, boolean, ByteBuffer, Callback)} + * made by {@link ChannelResponse#write(boolean, ByteBuffer, Callback)} fails. + *

+ * The implementation maintains the {@link #_streamSendState} before taking + * and serializing the call to the {@link #_writeCallback}, which was set by the call to {@code write}. + * @param x The reason for the failure. + */ @Override public void failed(Throwable x) { - // Called when an individual write fails. + if (LOG.isDebugEnabled()) + LOG.debug("write failed {}", this, x); + // Called when an individual write succeeds. Callback callback; HttpChannelState httpChannel; try (AutoLock ignored = _request._lock.lock()) { - httpChannel = _request.lockedGetHttpChannel(); - callback = httpChannel._writeCallback; - httpChannel._writeCallback = null; + httpChannel = _request.lockedGetHttpChannelState(); + callback = _writeCallback; + _writeCallback = null; + httpChannel.lockedStreamSendCompleted(false); } - if (LOG.isDebugEnabled()) - LOG.debug("write failed {}", callback, x); if (callback != null) httpChannel._serializedInvoker.run(() -> callback.failed(x)); } @@ -1140,26 +1221,24 @@ public void failed(Throwable x) @Override public InvocationType getInvocationType() { - return Invocable.getInvocationType(_request.getHttpChannel()._writeCallback); + return Invocable.getInvocationType(_writeCallback); } @Override public boolean isCommitted() { - return _request.getHttpChannel()._responseHeaders.isCommitted(); + return _request.getHttpChannelState()._responseHeaders.isCommitted(); } @Override public boolean isCompletedSuccessfully() { - // TODO: this should return whether the last write (or the stream) is completed - // not _completed because the last write may still be pending. try (AutoLock ignored = _request._lock.lock()) { - if (_request._httpChannel == null) + if (_request._httpChannelState == null) return false; - return _request._httpChannel._callbackCompleted && _request._httpChannel._failure == null; + return _request._httpChannelState._callbackCompleted && _request._httpChannelState._failure == null; } } @@ -1168,7 +1247,8 @@ public void reset() { _status = 0; _trailers = null; - _request.getHttpChannel().resetResponse(); + _contentBytesWritten = 0; + _request.getHttpChannelState().resetResponse(); } @Override @@ -1177,7 +1257,7 @@ public CompletableFuture writeInterim(int status, HttpFields headers) Completable completable = new Completable(); if (HttpStatus.isInterim(status)) { - HttpChannelState channel = _request.getHttpChannel(); + HttpChannelState channel = _request.getHttpChannelState(); HttpVersion version = channel.getConnectionMetaData().getHttpVersion(); MetaData.Response response = new MetaData.Response(status, null, version, headers); channel._stream.send(_request._metaData, response, false, null, completable); @@ -1189,7 +1269,7 @@ public CompletableFuture writeInterim(int status, HttpFields headers) return completable; } - private MetaData.Response lockedPrepareResponse(HttpChannelState httpChannel, boolean last) + MetaData.Response lockedPrepareResponse(HttpChannelState httpChannel, boolean last) { // Assume 200 unless told otherwise. if (_status == 0) @@ -1213,6 +1293,12 @@ private MetaData.Response lockedPrepareResponse(HttpChannelState httpChannel, bo getTrailersSupplier() ); } + + @Override + public String toString() + { + return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), getStatus(), getRequest()); + } } private static class ChannelCallback implements Callback @@ -1225,49 +1311,51 @@ private ChannelCallback(ChannelRequest request) _request = request; } + /** + * Called when the {@link Handler} (or it's delegates) succeeds the request handling. + */ @Override public void succeeded() { // Called when the request/response cycle is completing successfully. HttpStream stream; - boolean needLastWrite; + boolean needLastStreamSend; HttpChannelState httpChannelState; Throwable failure = null; + ChannelRequest request; + ChannelResponse response; MetaData.Response responseMetaData = null; boolean completeStream; try (AutoLock ignored = _request._lock.lock()) { - if (!lockedOnComplete()) - return; - httpChannelState = _request._httpChannel; - completeStream = - httpChannelState._handling == null && - httpChannelState._writeState == WriteState.LAST_WRITE_COMPLETED; + request = _request; + httpChannelState = _request._httpChannelState; + response = httpChannelState._response; + stream = httpChannelState._stream; // We are being tough on handler implementations and expect them // to not have pending operations when calling succeeded or failed. if (httpChannelState._onContentAvailable != null) throw new IllegalStateException("demand pending"); - if (httpChannelState._writeCallback != null) + if (response.lockedIsWriting()) throw new IllegalStateException("write pending"); - // Here, httpChannelState._error might have been set by some - // asynchronous event such as an idle timeout, and that's ok. - needLastWrite = switch (httpChannelState._writeState) - { - case NOT_LAST -> true; - case LAST_WRITTEN, LAST_WRITE_COMPLETED -> false; - }; - stream = httpChannelState._stream; + if (lockedCompleteCallback()) + return; + + assert httpChannelState._failure == null; + + needLastStreamSend = httpChannelState.lockedLastStreamSend(); + completeStream = !needLastStreamSend && httpChannelState._handling == null && httpChannelState.lockedIsLastStreamSendCompleted(); if (httpChannelState._responseHeaders.commit()) - responseMetaData = _request._response.lockedPrepareResponse(httpChannelState, true); + responseMetaData = response.lockedPrepareResponse(httpChannelState, true); - long totalWritten = _request._response._contentBytesWritten; + long totalWritten = response._contentBytesWritten; long committedContentLength = httpChannelState._committedContentLength; if (committedContentLength >= 0 && committedContentLength != totalWritten) - failure = httpChannelState._failure = new IOException("content-length %d != %d written".formatted(committedContentLength, totalWritten)); + failure = new IOException("content-length %d != %d written".formatted(committedContentLength, totalWritten)); // is the request fully consumed? Throwable unconsumed = stream.consumeAvailable(); @@ -1275,80 +1363,86 @@ public void succeeded() LOG.debug("consumeAvailable: {} {} ", unconsumed == null, httpChannelState); if (unconsumed != null && httpChannelState.getConnectionMetaData().isPersistent()) + failure = ExceptionUtil.combine(failure, unconsumed); + + if (failure != null) { - if (failure == null) - failure = httpChannelState._failure = unconsumed; - else if (ExceptionUtil.areNotAssociated(failure, unconsumed)) - failure.addSuppressed(unconsumed); + httpChannelState._failure = failure; + if (!stream.isCommitted()) + response.lockedPrepareErrorResponse(); + else + completeStream = true; } } - if (failure == null && needLastWrite) + if (LOG.isDebugEnabled()) + LOG.debug("succeeded: failure={} needLastStreamSend={} {}", failure, needLastStreamSend, this); + + if (failure != null) + Response.writeError(request, response, new ErrorCallback(request, stream, failure), failure); + else if (needLastStreamSend) stream.send(_request._metaData, responseMetaData, true, null, httpChannelState._handlerInvoker); else if (completeStream) httpChannelState._handlerInvoker.completeStream(stream, failure); + else if (LOG.isDebugEnabled()) + LOG.debug("No action on succeeded {}", this); } + /** + * Called when the {@link Handler} (or it's delegates) fail the request handling. + * @param failure The reason for the failure. + */ @Override public void failed(Throwable failure) { // Called when the request/response cycle is completing with a failure. HttpStream stream; - boolean writeErrorResponse; ChannelRequest request; + ChannelResponse response; HttpChannelState httpChannelState; - boolean completeStream; + boolean writeError; try (AutoLock ignored = _request._lock.lock()) { - if (!lockedOnComplete()) - return; - httpChannelState = _request._httpChannel; - httpChannelState._failure = failure; - completeStream = httpChannelState._handling == null; - - // Verify whether we can write an error response. - writeErrorResponse = !httpChannelState._stream.isCommitted(); + httpChannelState = _request._httpChannelState; stream = httpChannelState._stream; request = _request; + response = httpChannelState._response; + + if (lockedCompleteCallback()) + return; + assert httpChannelState._failure == null; + + httpChannelState._failure = failure; // Consume any input. Throwable unconsumed = stream.consumeAvailable(); - if (unconsumed != null && ExceptionUtil.areNotAssociated(unconsumed, failure)) + if (ExceptionUtil.areNotAssociated(unconsumed, failure)) failure.addSuppressed(unconsumed); - if (writeErrorResponse) - { - // Cannot log or recycle just yet, since we need to generate the error response. - _request._response._status = HttpStatus.INTERNAL_SERVER_ERROR_500; - httpChannelState._responseHeaders.reset(); - httpChannelState._writeState = WriteState.NOT_LAST; - } - } - - if (LOG.isDebugEnabled()) - LOG.debug("failed {}", httpChannelState, failure); + if (LOG.isDebugEnabled()) + LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", httpChannelState._stream.isCommitted(), httpChannelState._response.isCommitted(), this); - if (writeErrorResponse) - { - ErrorResponse response = new ErrorResponse(request, stream, failure); - Response.writeError(request, response, response, failure); - } - else if (completeStream) - { - httpChannelState._handlerInvoker.completeStream(stream, failure); + writeError = !stream.isCommitted(); + if (writeError) + response.lockedPrepareErrorResponse(); } + + if (writeError) + Response.writeError(request, response, new ErrorCallback(request, stream, failure), failure); + else + _request.getHttpChannelState()._handlerInvoker.failed(failure); } - private boolean lockedOnComplete() + private boolean lockedCompleteCallback() { assert _request._lock.isHeldByCurrentThread(); - HttpChannelState httpChannelState = _request._httpChannel; + HttpChannelState httpChannelState = _request._httpChannelState; if (httpChannelState == null) { if (LOG.isDebugEnabled()) LOG.debug("already recycled after completion {} by", _request, _completedBy); - return false; + return true; } if (httpChannelState._callbackCompleted) @@ -1358,7 +1452,7 @@ private boolean lockedOnComplete() LOG.debug("already completed {} by", _request, _completedBy); LOG.debug("Second complete", new Throwable("second complete")); } - return false; + return true; } if (LOG.isDebugEnabled()) @@ -1366,7 +1460,7 @@ private boolean lockedOnComplete() httpChannelState._callbackCompleted = true; - return true; + return false; } @Override @@ -1377,92 +1471,86 @@ public InvocationType getInvocationType() } } - private static class ErrorResponse extends Response.Wrapper implements Callback + /** + * Used as the {@link Response} and {@link Callback} when writing the error response + * from {@link HttpChannelState.ChannelCallback#failed(Throwable)}. + */ + private static class ErrorCallback implements Callback { private final ChannelRequest _request; private final HttpStream _stream; private final Throwable _failure; - public ErrorResponse(ChannelRequest request, HttpStream stream, Throwable failure) + public ErrorCallback(ChannelRequest request, HttpStream stream, Throwable failure) { - super(request, request._response); _request = request; _stream = stream; _failure = failure; } - @Override - public void write(boolean last, ByteBuffer content, Callback callback) - { - long length = BufferUtil.length(content); - - HttpChannelState httpChannel; - Throwable failure; - MetaData.Response responseMetaData = null; - try (AutoLock ignored = _request._lock.lock()) - { - httpChannel = _request.lockedGetHttpChannel(); - httpChannel._writeCallback = callback; - failure = httpChannel.lockedCheckWrite(last, length); - if (httpChannel._responseHeaders.commit()) - responseMetaData = _request._response.lockedPrepareResponse(httpChannel, last); - } - - if (failure == null) - _stream.send(_request._metaData, responseMetaData, last, content, last ? Callback.from(this::lastWriteCompleted, callback) : callback); - else if (failure == DO_NOT_SEND) - httpChannel._serializedInvoker.run(callback::succeeded); - else - httpChannel._serializedInvoker.run(() -> callback.failed(failure)); - } - - private void lastWriteCompleted() - { - try (AutoLock ignored = _request._lock.lock()) - { - _request.lockedGetHttpChannel()._writeState = WriteState.LAST_WRITE_COMPLETED; - } - } - + /** + * Called when the error write in {@link HttpChannelState.ChannelCallback#failed(Throwable)} succeeds. + */ @Override public void succeeded() { + if (LOG.isDebugEnabled()) + LOG.debug("ErrorWrite succeeded: {}", this); boolean needLastWrite; MetaData.Response responseMetaData = null; - HttpChannelState httpChannel; + HttpChannelState httpChannelState; + Throwable failure; try (AutoLock ignored = _request._lock.lock()) { - httpChannel = _request.getHttpChannel(); + httpChannelState = _request.getHttpChannelState(); + failure = _failure; // Did the ErrorHandler do the last write? - needLastWrite = httpChannel._writeState.ordinal() <= WriteState.LAST_WRITTEN.ordinal(); - if (needLastWrite && httpChannel._responseHeaders.commit()) - responseMetaData = _request._response.lockedPrepareResponse(httpChannel, true); + needLastWrite = httpChannelState.lockedLastStreamSend(); + if (needLastWrite && httpChannelState._responseHeaders.commit()) + responseMetaData = httpChannelState._response.lockedPrepareResponse(httpChannelState, true); } if (needLastWrite) { _stream.send(_request._metaData, responseMetaData, true, null, - Callback.from(() -> httpChannel._handlerInvoker.failed(_failure), + Callback.from(() -> httpChannelState._handlerInvoker.failed(failure), x -> { - if (ExceptionUtil.areNotAssociated(_failure, x)) - _failure.addSuppressed(x); - httpChannel._handlerInvoker.failed(_failure); + if (ExceptionUtil.areNotAssociated(failure, x)) + failure.addSuppressed(x); + httpChannelState._handlerInvoker.failed(failure); })); } else { - httpChannel._handlerInvoker.failed(_failure); + httpChannelState._handlerInvoker.failed(failure); } } + /** + * Called when the error write in {@link HttpChannelState.ChannelCallback#failed(Throwable)} fails. + * @param x The reason for the failure. + */ @Override public void failed(Throwable x) { - if (ExceptionUtil.areNotAssociated(_failure, x)) - _failure.addSuppressed(x); - _request.getHttpChannel()._handlerInvoker.failed(_failure); + if (LOG.isDebugEnabled()) + LOG.debug("ErrorWrite failed: {}", this, x); + Throwable failure; + try (AutoLock ignored = _request._lock.lock()) + { + failure = _failure; + } + if (ExceptionUtil.areNotAssociated(failure, x)) + failure.addSuppressed(x); + _request.getHttpChannelState()._handlerInvoker.failed(failure); + } + + @Override + public String toString() + { + return "%s@%x".formatted(getClass().getSimpleName(), hashCode()); } } @@ -1505,7 +1593,7 @@ else if (error == null) // We are already in error, so we will not handle this one, // but we will add as suppressed if we have not seen it already. Throwable cause = error.getCause(); - if (cause != null && ExceptionUtil.areNotAssociated(cause, failure)) + if (ExceptionUtil.areNotAssociated(cause, failure)) error.getCause().addSuppressed(failure); } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 8faa21179508..56fb22d86fa4 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -959,20 +959,13 @@ private void releaseChunk() @Override protected void onCompleteSuccess() { - // TODO is this too late to get the request? And is that the right attribute and the right thing to do? - boolean upgrading = _httpChannel.getRequest() != null && _httpChannel.getRequest().getAttribute(HttpStream.UPGRADE_CONNECTION_ATTRIBUTE) != null; release().succeeded(); - // If successfully upgraded it is responsibility of the next protocol to close the connection. - if (_shutdownOut && !upgrading) - getEndPoint().shutdownOutput(); } @Override public void onCompleteFailure(final Throwable x) { failedCallback(release(), x); - if (_shutdownOut) - getEndPoint().shutdownOutput(); } @Override @@ -1573,6 +1566,9 @@ public void succeeded() if (LOG.isDebugEnabled()) LOG.debug("non-current completion {}", this); + if (_sendCallback._shutdownOut) + getEndPoint().shutdownOutput(); + // If we are looking for the next request if (_parser.isStart()) { @@ -1619,6 +1615,8 @@ public void failed(Throwable x) LOG.debug("ignored", x); return; } + if (LOG.isDebugEnabled()) + LOG.debug("aborting", x); abort(x); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/CustomRequestLogTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/CustomRequestLogTest.java index b47b6315b17e..dd770058badb 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/CustomRequestLogTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/CustomRequestLogTest.java @@ -13,7 +13,9 @@ package org.eclipse.jetty.server; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; import java.io.OutputStream; import java.net.InetAddress; import java.net.NetworkInterface; @@ -48,6 +50,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -623,15 +626,7 @@ public void testLogConnectionStatus() throws Exception @Override public boolean handle(Request request, Response response, Callback callback) { - if (Request.getPathInContext(request).equals("/abort")) - { - Callback cbk = Callback.from(() -> callback.failed(new QuietException.Exception("test fail")), callback::failed); - Content.Sink.write(response, false, "data", cbk); - } - else - { - callback.succeeded(); - } + callback.succeeded(); return true; } }); @@ -675,13 +670,52 @@ public boolean handle(Request request, Response response, Callback callback) assertEquals(HttpStatus.BAD_REQUEST_400, response.getStatus()); log = _logs.poll(5, TimeUnit.SECONDS); assertThat(log, is("/no/host ConnectionStatus: 400 X")); + } - getResponses(""" - GET /abort HTTP/1.1 - Host: localhost + @Test + public void testLogAbortConnectionStatus() throws Exception + { + start("%U ConnectionStatus: %s %X", new SimpleHandler() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Callback cbk = Callback.from(() -> callback.failed(new QuietException.Exception("test fail")), callback::failed); + Content.Sink.write(response, false, "data", cbk); + return true; + } + }); - """, 1); - log = _logs.poll(5, TimeUnit.SECONDS); + try (Socket socket = new Socket("localhost", _serverConnector.getLocalPort())) + { + socket.setSoTimeout(10000); + socket.setTcpNoDelay(true); + + OutputStream output = socket.getOutputStream(); + output.write(""" + GET /abort HTTP/1.1 + Host: localhost + + """.getBytes(StandardCharsets.ISO_8859_1)); + output.flush(); + + // Not using HttpTester here because we want to check that last chunk is not received. + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream(), StandardCharsets.ISO_8859_1)); + + String line = in.readLine(); + assertThat(line, is("HTTP/1.1 200 OK")); + while (line != null && line.length() > 0) + line = in.readLine(); + + line = in.readLine(); + assertThat("chunk", line, is("4")); + line = in.readLine(); + assertThat("data", line, is("data")); + line = in.readLine(); + // This is the crucial part of the test. abort is indicated by no last chunk. + assertThat(line, nullValue()); + } + String log = _logs.poll(5, TimeUnit.SECONDS); assertThat(log, is("/abort ConnectionStatus: 200 X")); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java index 653a06f96c36..f4a3c5d2d3f0 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; +import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; @@ -44,6 +45,7 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FutureCallback; import org.eclipse.jetty.util.FuturePromise; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.thread.SerializedInvoker; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -56,11 +58,13 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -80,8 +84,7 @@ public void beforeEach() throws Exception @AfterEach public void afterEach() throws Exception { - if (_server != null) - _server.stop(); + LifeCycle.stop(_server); } @Test @@ -574,32 +577,37 @@ public void testInsufficientContentWritten1() throws Exception @Override public boolean handle(Request request, Response response, Callback callback) { + request.addHttpStreamWrapper(HttpStreamCaptureFailure::asWrapper); response.getHeaders().put(HttpHeader.CONTENT_LENGTH, 10); - response.write(true, BufferUtil.toBuffer("12345"), callback); + try (StacklessLogging ignore = new StacklessLogging(Response.class)) + { + response.write(true, BufferUtil.toBuffer("12345"), callback); + } return true; } }; _server.setHandler(handler); - _server.start(); - - ConnectionMetaData connectionMetaData = new MockConnectionMetaData(new MockConnector(_server)); - HttpChannel channel = new HttpChannelState(connectionMetaData); - MockHttpStream stream = new MockHttpStream(channel); - HttpFields fields = HttpFields.build().add(HttpHeader.HOST, "localhost").asImmutable(); - MetaData.Request request = new MetaData.Request("GET", HttpURI.from("http://localhost/"), HttpVersion.HTTP_1_1, fields, 0); - Runnable onRequest = channel.onRequest(request); - try (StacklessLogging ignored = new StacklessLogging(Response.class)) - { - onRequest.run(); - } + LocalConnector localConnector = new LocalConnector(_server); + _server.addConnector(localConnector); + _server.start(); - assertThat(stream.isComplete(), is(true)); - assertThat(stream.getFailure(), notNullValue()); - assertThat(stream.getFailure().getMessage(), containsString("5 < 10")); - assertThat(stream.getResponse(), notNullValue()); - assertThat(stream.getResponse().getStatus(), is(500)); - assertThat(stream.getResponseContentAsString(), containsString("5 < 10")); + String rawRequest = """ + GET / HTTP/1.1 + Host: local + Connection: close + + """; + + HttpTester.Response response = HttpTester.parseResponse(localConnector.getResponse(rawRequest)); + assertEquals(500, response.getStatus()); + assertThat(response.getContent(), containsString("5 < 10")); + + HttpStreamCaptureFailure capture = HttpStreamCaptureFailure.captureRef.get(); + assertTrue(capture.failLatch.await(5, TimeUnit.SECONDS)); + Throwable failure = capture.failRef.get(); + assertThat(failure, notNullValue()); + assertThat(failure.getMessage(), containsString("5 < 10")); } @Test @@ -680,27 +688,40 @@ public void testExcessContentWritten2() throws Exception @Override public boolean handle(Request request, Response response, Callback callback) { + request.addHttpStreamWrapper(HttpStreamCaptureFailure::asWrapper); response.getHeaders().put(HttpHeader.CONTENT_LENGTH, 5); - response.write(false, BufferUtil.toBuffer("1234"), Callback.from(() -> response.write(true, BufferUtil.toBuffer("567890"), callback))); + try (StacklessLogging ignore = new StacklessLogging(Response.class)) + { + response.write(false, BufferUtil.toBuffer("1234"), + Callback.from(() -> + response.write(true, BufferUtil.toBuffer("567890"), + callback))); + } return true; } }; _server.setHandler(handler); - _server.start(); - - ConnectionMetaData connectionMetaData = new MockConnectionMetaData(new MockConnector(_server)); - HttpChannel channel = new HttpChannelState(connectionMetaData); - MockHttpStream stream = new MockHttpStream(channel); - HttpFields fields = HttpFields.build().add(HttpHeader.HOST, "localhost").asImmutable(); - MetaData.Request request = new MetaData.Request("GET", HttpURI.from("http://localhost/"), HttpVersion.HTTP_1_1, fields, 0); - Runnable task = channel.onRequest(request); - task.run(); + LocalConnector localConnector = new LocalConnector(_server); + _server.addConnector(localConnector); + _server.start(); - assertThat(stream.isComplete(), is(true)); - assertThat(stream.getFailure(), notNullValue()); - assertThat(stream.getFailure().getMessage(), containsString("10 > 5")); - assertThat(stream.getResponse(), notNullValue()); + String rawRequest = """ + GET / HTTP/1.1 + Host: local + Connection: close + + """; + + String rawResponse = localConnector.getResponse(rawRequest); + assertThat(rawResponse, startsWith("HTTP/1.1 200 OK")); + + HttpStreamCaptureFailure capture = HttpStreamCaptureFailure.captureRef.get(); + assertTrue(capture.failLatch.await(5, TimeUnit.SECONDS)); + Throwable failure = capture.failRef.get(); + assertThat(failure, notNullValue()); + assertThat(failure, instanceOf(IOException.class)); // the stream detected the error + assertThat(failure.getMessage(), containsString("10 > 5")); } @Test @@ -1456,4 +1477,31 @@ public void send(MetaData.Request request, MetaData.Response response, boolean l assertThat(stream.getResponse(), nullValue()); } } + + private static class HttpStreamCaptureFailure extends HttpStream.Wrapper + { + public static AtomicReference captureRef = new AtomicReference<>(); + public AtomicReference failRef = new AtomicReference<>(); + public CountDownLatch failLatch = new CountDownLatch(1); + + private HttpStreamCaptureFailure(HttpStream httpStream) + { + super(httpStream); + } + + public static HttpStream asWrapper(HttpStream httpStream) + { + HttpStreamCaptureFailure capture = new HttpStreamCaptureFailure(httpStream); + captureRef.set(capture); + return capture; + } + + @Override + public void failed(Throwable x) + { + super.failed(x); + failRef.set(x); + failLatch.countDown(); + } + } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java index 8bce03bd3317..cd3dd3f9147f 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java @@ -23,6 +23,7 @@ import java.io.OutputStream; import java.net.Socket; import java.net.URL; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Queue; @@ -35,6 +36,7 @@ import org.awaitility.Awaitility; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.ArrayByteBufferPool; @@ -146,10 +148,11 @@ public void testSimpleGET() throws Exception os.flush(); // Read the response. - String response = readResponse(client); + String rawResponse = readResponse(client); - assertThat(response, containsString("HTTP/1.1 200 OK")); - assertThat(response, containsString("Hello")); + assertThat(rawResponse, containsString("HTTP/1.1 200 OK")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getContent(), containsString("Hello")); } } @@ -177,10 +180,78 @@ public void testSimplePOST() throws Exception os.flush(); // Read the response. - String response = readResponse(client); + String rawResponse = readResponse(client); - assertThat(response, containsString("HTTP/1.1 200 OK")); - assertThat(response, containsString("0123456789")); + assertThat(rawResponse, containsString("HTTP/1.1 200 OK")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getContent(), containsString("0123456789")); + } + } + + @Test + public void testSimpleFailure() throws Exception + { + startServer(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + callback.failed(new Exception("Test failure")); + return true; + } + }); + + try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort())) + { + OutputStream os = client.getOutputStream(); + + String request = """ + GET / HTTP/1.1 + Host: localhost + Connection: close + + """; + os.write(request.getBytes(StandardCharsets.ISO_8859_1)); + os.flush(); + + // Read the response. + String rawResponse = readResponse(client); + assertThat(rawResponse, containsString("HTTP/1.1 500 Server Error")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getContent(), containsString("Exception: Test failure")); + } + } + + @Test + public void testSimpleException() throws Exception + { + startServer(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + throw new Exception("Test failure"); + } + }); + + try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort())) + { + OutputStream os = client.getOutputStream(); + + String request = """ + GET / HTTP/1.1 + Host: localhost + Connection: close + + """; + os.write(request.getBytes(StandardCharsets.ISO_8859_1)); + os.flush(); + + // Read the response. + String rawResponse = readResponse(client); + assertThat(rawResponse, containsString("HTTP/1.1 500 Server Error")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getContent(), containsString("Exception: Test failure")); } } @@ -1352,7 +1423,7 @@ public boolean handle(Request request, Response response, Callback callback) thr blocker.block(); } - throw new Exception(new Exception("exception after commit")); + throw new Exception("exception after commit"); } } @@ -1530,10 +1601,12 @@ public void testWriteBodyAfterNoBodyResponse() throws Exception BufferedReader in = new BufferedReader(new InputStreamReader(client.getInputStream())); String line = in.readLine(); + System.err.println(line); assertThat(line, containsString(" 304 ")); while (true) { line = in.readLine(); + System.err.println(line); if (line == null) throw new EOFException(); if (line.length() == 0) @@ -1544,6 +1617,7 @@ public void testWriteBodyAfterNoBodyResponse() throws Exception assertThat(line, not(containsString("Transfer-Encoding"))); } + System.err.println("---"); line = in.readLine(); assertThat(line, containsString(" 304 ")); while (true) @@ -1568,7 +1642,7 @@ private static class WriteBodyAfterNoBodyResponseHandler extends Handler.Abstrac @Override public boolean handle(Request request, Response response, Callback callback) { - response.setStatus(304); + response.setStatus(HttpStatus.NOT_MODIFIED_304); response.write(false, BufferUtil.toBuffer("yuck"), callback); return true; } @@ -1631,7 +1705,6 @@ public void testChunkedShutdown() throws Exception { startServer(new ReadExactHandler(4096)); byte[] content = new byte[4096]; - Arrays.fill(content, (byte)'X'); try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort())) { @@ -1639,37 +1712,39 @@ public void testChunkedShutdown() throws Exception // Send two persistent pipelined requests and then shutdown output os.write((""" - GET / HTTP/1.1\r + GET /one HTTP/1.1\r Host: localhost\r Transfer-Encoding: chunked\r \r 1000\r """).getBytes(StandardCharsets.ISO_8859_1)); + Arrays.fill(content, (byte)'1'); os.write(content); os.write("\r\n0\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1)); os.write((""" - GET / HTTP/1.1\r + GET /two HTTP/1.1\r Host: localhost\r Transfer-Encoding: chunked\r \r 1000\r """).getBytes(StandardCharsets.ISO_8859_1)); + Arrays.fill(content, (byte)'2'); os.write(content); os.write("\r\n0\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1)); os.flush(); + client.shutdownOutput(); - // Read the two pipelined responses - HttpTester.Response response = HttpTester.parseResponse(client.getInputStream()); + // Read the two pipelined responses until EOF + ByteBuffer responses = ByteBuffer.wrap(IO.readBytes(client.getInputStream())); + + HttpTester.Response response = HttpTester.parseResponse(responses); assertThat(response.getStatus(), is(200)); assertThat(response.getContent(), containsString("Read " + content.length)); - response = HttpTester.parseResponse(client.getInputStream()); + response = HttpTester.parseResponse(responses); assertThat(response.getStatus(), is(200)); assertThat(response.getContent(), containsString("Read " + content.length)); - - // Read the close - assertThat(client.getInputStream().read(), is(-1)); } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java index 3e22aad824ce..1f90b8e1a45d 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java @@ -13,28 +13,45 @@ package org.eclipse.jetty.server; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; -@Disabled // TODO public class LargeHeaderTest { + private static final Logger LOG = LoggerFactory.getLogger(LargeHeaderTest.class); + private static final String EXPECTED_ERROR_TEXT = "

HTTP ERROR 500 Response header too large

"; private Server server; @BeforeEach @@ -47,13 +64,10 @@ public void setup() throws Exception ServerConnector connector = new ServerConnector(server, http); connector.setPort(0); - connector.setIdleTimeout(5000); + connector.setIdleTimeout(15000); server.addConnector(connector); - /* TODO - server.setErrorHandler(new ErrorHandler()); - - server.setHandler(new AbstractHandler() + server.setHandler(new Handler.Abstract() { final String largeHeaderValue; @@ -64,19 +78,38 @@ public void setup() throws Exception } @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + public boolean handle(Request request, Response response, Callback callback) throws Exception { + String idCount = request.getHeaders().get("X-Count"); + LOG.debug("X-Count: {} [handle]", idCount); + response.getHeaders().put(HttpHeader.CONTENT_TYPE.toString(), MimeTypes.Type.TEXT_HTML.toString()); response.getHeaders().put("LongStr", largeHeaderValue); - PrintWriter writer = response.getWriter(); - writer.write("

FOO

"); - writer.flush(); - response.flushBuffer(); - baseRequest.setHandled(true); + + String responseBody = "

FOO

"; + + Callback topCallback = new Callback() + { + @Override + public void succeeded() + { + LOG.debug("X-Count: {} [callback.succeeded]", idCount); + callback.succeeded(); + } + + @Override + public void failed(Throwable x) + { + LOG.debug("X-Count: {} [callback.failed] {}", idCount, this); + callback.failed(x); + } + }; + response.write(true, BufferUtil.toBuffer(responseBody, UTF_8), topCallback); + LOG.debug("X-Count: {} [handle-completed]", idCount); + return true; } }); - */ server.start(); } @@ -86,44 +119,218 @@ public void teardown() LifeCycle.stop(server); } + private static String readResponse(Socket socket, int xCount, InputStream input) throws IOException + { + ByteArrayOutputStream readBytes = new ByteArrayOutputStream(); + int bufferSize = 65535; + byte[] buffer = new byte[bufferSize]; + int lenRead; + int lenTotal = 0; + + LOG.debug("X-Count: {} - Reading Response from {}->{}", xCount, socket.getLocalSocketAddress(), socket.getRemoteSocketAddress()); + + while (true) + { + lenRead = input.read(buffer, 0, bufferSize); + if (lenRead < 0) + break; + readBytes.write(buffer, 0, lenRead); + lenTotal += lenRead; + } + + LOG.debug("X-Count: {} - Read {} bytes of Response from {}->{}", xCount, lenTotal, socket.getLocalAddress(), socket.getRemoteSocketAddress()); + return readBytes.toString(UTF_8); + } + @Test public void testLargeHeader() throws Throwable { - ExecutorService executorService = Executors.newFixedThreadPool(8); + URI serverURI = server.getURI(); + String rawRequest = "GET / HTTP/1.1\r\n" + + "Host: " + serverURI.getAuthority() + "\r\n" + + "\r\n"; + + try (Socket client = new Socket(serverURI.getHost(), serverURI.getPort()); + OutputStream output = client.getOutputStream(); + InputStream input = client.getInputStream()) + { + output.write(rawRequest.getBytes(UTF_8)); + output.flush(); + + String rawResponse = readResponse(client, 1, input); + System.err.println(rawResponse); + assertThat(rawResponse, containsString(" 500 ")); + } + } + + @Test + public void testLargeHeaderNewConnectionsConcurrent() throws Throwable + { + ExecutorService executorService = Executors.newCachedThreadPool(); - int localPort = server.getURI().getPort(); + URI serverURI = server.getURI(); String rawRequest = "GET / HTTP/1.1\r\n" + - "Host: localhost:" + localPort + "\r\n" + + "Host: " + serverURI.getAuthority() + "\r\n" + + "Connection: close\r\n" + + "X-Count: %d\r\n" + // just so I can track them in wireshark "\r\n"; + final int iterations = 500; + final int timeout = 16; Throwable issues = new Throwable(); + AtomicInteger counter = new AtomicInteger(); + List> tasks = new ArrayList<>(); + AtomicInteger count500 = new AtomicInteger(0); + AtomicInteger countOther = new AtomicInteger(0); + AtomicInteger countFailure = new AtomicInteger(0); + AtomicInteger countEmpty = new AtomicInteger(0); - for (int i = 0; i < 500; ++i) + for (int i = 0; i < iterations; i++) { - executorService.submit(() -> + tasks.add(() -> { - try (Socket client = new Socket("localhost", localPort); + int count = counter.incrementAndGet(); + + try (Socket client = new Socket(serverURI.getHost(), serverURI.getPort()); OutputStream output = client.getOutputStream(); InputStream input = client.getInputStream()) { - output.write(rawRequest.getBytes(UTF_8)); + LOG.debug("X-Count: {} - Send Request - {}->{}", count, client.getLocalAddress(), client.getRemoteSocketAddress()); + + output.write(rawRequest.formatted(count).getBytes(UTF_8)); output.flush(); + // String rawResponse = readResponse(client, count, input); String rawResponse = IO.toString(input, UTF_8); + if (rawResponse.isEmpty()) + { + LOG.warn("X-Count: {} - Empty Raw Response", count); + countEmpty.incrementAndGet(); + return null; + } + HttpTester.Response response = HttpTester.parseResponse(rawResponse); - assertThat(response.getStatus(), is(500)); + if (response == null) + { + LOG.warn("X-Count: {} - Null HttpTester.Response", count); + countEmpty.incrementAndGet(); + } + else if (response.getStatus() == 500) + { + // expected result + count500.incrementAndGet(); + long contentLength = response.getLongField(HttpHeader.CONTENT_LENGTH); + String responseBody = response.getContent(); + assertThat((long)responseBody.length(), is(contentLength)); + } + else + { + LOG.warn("X-Count: {} - Unexpected Status Code: {}", count, response.getStatus()); + countOther.incrementAndGet(); + } } catch (Throwable t) { issues.addSuppressed(t); + countFailure.incrementAndGet(); } + return null; }); } - executorService.awaitTermination(5, TimeUnit.SECONDS); + List> futures = executorService.invokeAll(tasks, timeout, TimeUnit.SECONDS); + if (issues.getSuppressed().length > 0) { throw issues; } + + executorService.shutdown(); + executorService.awaitTermination(timeout * 2, TimeUnit.SECONDS); + assertEquals(iterations, count500.get(), () -> + { + return """ + All tasks did not fail as expected. + Iterations: %d + Count (500 response status) [expected]: %d + Count (empty response): %d + Count (throwables): %d + Count (other status codes): %d + """.formatted(iterations, count500.get(), countEmpty.get(), countFailure.get(), countOther.get()); + }); + } + + @Test + public void testLargeHeaderNewConnectionsSequential() throws Throwable + { + URI serverURI = server.getURI(); + String rawRequest = "GET / HTTP/1.1\r\n" + + "Host: " + serverURI.getAuthority() + "\r\n" + + "X-Count: %d\r\n" + // just so I can track them in wireshark + "\r\n"; + + final int iterations = 500; + AtomicInteger count500 = new AtomicInteger(0); + AtomicInteger countEmpty = new AtomicInteger(0); + AtomicInteger countOther = new AtomicInteger(0); + AtomicInteger countFailure = new AtomicInteger(0); + + for (int count = 0; count < iterations; count++) + { + try (Socket client = new Socket(serverURI.getHost(), serverURI.getPort()); + OutputStream output = client.getOutputStream(); + InputStream input = client.getInputStream()) + { + output.write(rawRequest.formatted(count).getBytes(UTF_8)); + output.flush(); + + String rawResponse = readResponse(client, count, input); + if (rawResponse.isEmpty()) + { + LOG.warn("X-Count: {} - Empty Raw Response", count); + countEmpty.incrementAndGet(); + break; + } + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + int status = response.getStatus(); + if (response == null) + { + LOG.warn("X-Count: {} - Empty Parsed Response", count); + countEmpty.incrementAndGet(); + } + else if (status == 500) + { + long contentLength = response.getLongField(HttpHeader.CONTENT_LENGTH); + String responseBody = response.getContent(); + assertThat((long)responseBody.length(), is(contentLength)); + assertThat(responseBody, containsString(EXPECTED_ERROR_TEXT)); + count500.incrementAndGet(); + } + else + { + LOG.warn("X-Count: {} - Unexpected Status Code: {}>", count, status); + countOther.incrementAndGet(); + break; + } + } + catch (Throwable t) + { + LOG.warn("X-Count: {} - ERROR", count, t); + countFailure.incrementAndGet(); + break; + } + } + + assertEquals(iterations, count500.get(), () -> + { + return """ + All tasks did not fail as expected. + Iterations: %d + Count (500 response status) [expected]: %d + Count (empty responses): %d + Count (throwables): %d + Count (other status codes): %d + """.formatted(iterations, count500.get(), countEmpty.get(), countFailure.get(), countOther.get()); + }); } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 973e10d07b8d..08d9f4534022 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -228,8 +228,6 @@ public boolean handle(org.eclipse.jetty.server.Request request, Response respons response.setStatus(200); response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain"); ByteArrayOutputStream buff = new ByteArrayOutputStream(); - - request.getHeaders().getFields(HttpHeader.COOKIE).forEach(System.err::println); List coreCookies = org.eclipse.jetty.server.Request.getCookies(request); if (coreCookies != null) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java new file mode 100644 index 000000000000..e6e1d003aa77 --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java @@ -0,0 +1,245 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.Socket; +import java.net.URI; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.logging.StacklessLogging; +import org.eclipse.jetty.server.internal.HttpConnection; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.component.LifeCycle; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; + +/** + * The purpose of this testcase is to ensure that we handle the various + * Response complete flows when dealing with errors and failures. + */ +public class ResponseCompleteTest +{ + private static final Logger LOG = LoggerFactory.getLogger(ResponseCompleteTest.class); + private static final byte[] GET_REQUEST_BYTES = """ + GET / HTTP/1.1 + Host: local + Connection: close + + """.getBytes(UTF_8); + private Server server; + + private Server startServer(HttpConnectionFactory httpConnectionFactory, Handler handler) throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server, 1, 1, httpConnectionFactory); + connector.setPort(0); + server.addConnector(connector); + server.setHandler(handler); + server.start(); + return server; + } + + @AfterEach + public void stopServer() + { + LifeCycle.stop(server); + } + + /** + * The Handler returns true, and doesn't throw. + * Callback not yet completing when response fails in different thread. + */ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testHandleCallbackNotCompletingYet(boolean throwFromHandler) throws Exception + { + AtomicReference callbackAtomicReference = new AtomicReference<>(); + startServer(new HttpConnectionFactory(), new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(200); + callbackAtomicReference.set(callback); + if (throwFromHandler) + throw new Exception("Test"); + return true; + } + }); + + try (Socket client = connectToServer(); + OutputStream output = client.getOutputStream(); + InputStream input = client.getInputStream(); + StacklessLogging ignore = new StacklessLogging(Response.class)) + { + output.write(GET_REQUEST_BYTES); + output.flush(); + + Thread.sleep(1000); + callbackAtomicReference.get().failed(new Exception("Test")); + + HttpTester.Response response = HttpTester.parseResponse(input); + assertThat(response.getStatus(), is(500)); + assertThat(response.getContent(), containsString("

HTTP ERROR 500 java.lang.Exception: Test

")); + } + } + + /** + * The Handler returns true, and doesn't throw. + * Callback is completing when response fails in different thread. + */ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testHandleCallbackCompleting(boolean throwFromHandler) throws Exception + { + CountDownLatch handleLatch = new CountDownLatch(1); + CountDownLatch failureLatch = new CountDownLatch(1); + startServer( + new HttpConnectionFactory() + { + @Override + public Connection newConnection(Connector connector, EndPoint endPoint) + { + HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) + { + @Override + protected HttpStreamOverHTTP1 newHttpStream(String method, String uri, HttpVersion version) + { + return new HttpStreamOverHTTP1(method, uri, version) + { + @Override + public Throwable consumeAvailable() + { + /* + * Wait till callback is complete + * + * We rely on the existence of consumeAvailable in ChannelCallback.failure() + */ + + try + { + handleLatch.countDown(); + failureLatch.await(); + LOG.debug("consumeAvailable allowed to continue"); + return super.consumeAvailable(); + } + catch (InterruptedException e) + { + throw new RuntimeException(e); + } + } + }; + } + }; + connection.setUseInputDirectByteBuffers(isUseInputDirectByteBuffers()); + connection.setUseOutputDirectByteBuffers(isUseOutputDirectByteBuffers()); + return configure(connection, connector, endPoint); + } + }, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(200); + getServer().getThreadPool().execute(() -> + { + LOG.debug("handle.threadPool.execute() -> callback.failed() being called"); + callback.failed(new Exception("Test-Threaded")); + }); + handleLatch.await(); + if (throwFromHandler) + throw new Exception("Test-Handler"); + return true; + } + }); + + try (Socket client = connectToServer(); + OutputStream output = client.getOutputStream(); + InputStream input = client.getInputStream(); + StacklessLogging ignore = new StacklessLogging(Response.class)) + { + output.write(GET_REQUEST_BYTES); + output.flush(); + + Thread.sleep(1000); // ensure we are fully out of the HandlerInvoker.run() + failureLatch.countDown(); + + LOG.debug("Reading response"); + String rawResponse = IO.toString(input, UTF_8); + assertThat("Raw Response Length", rawResponse.length(), greaterThan(0)); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(500)); + assertThat(response.getContent(), containsString("

HTTP ERROR 500 java.lang.Exception: Test-Threaded

")); + } + } + + /** + * The Handler returns true. + * Callback is completed when response fails in different thread. + */ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testHandleCallbackCompleted(boolean throwFromHandler) throws Exception + { + startServer(new HttpConnectionFactory(), new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(200); + callback.failed(new Exception("Test")); + if (throwFromHandler) + throw new Exception("Test"); + return true; + } + }); + + try (Socket client = connectToServer(); + OutputStream output = client.getOutputStream(); + InputStream input = client.getInputStream(); + StacklessLogging ignore = new StacklessLogging(Response.class)) + { + output.write(GET_REQUEST_BYTES); + output.flush(); + + HttpTester.Response response = HttpTester.parseResponse(input); + assertThat(response.getStatus(), is(500)); + assertThat(response.getContent(), containsString("

HTTP ERROR 500 java.lang.Exception: Test

")); + } + } + + private Socket connectToServer() throws IOException + { + URI serverURI = server.getURI(); + return new Socket(serverURI.getHost(), serverURI.getPort()); + } +} diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java new file mode 100644 index 000000000000..5702f13765c2 --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java @@ -0,0 +1,215 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Stream; + +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.io.QuietException; +import org.eclipse.jetty.server.internal.HttpChannelState; +import org.eclipse.jetty.server.internal.HttpConnection; +import org.eclipse.jetty.util.Blocker; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.thread.Invocable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; + +public class ServerTest +{ + private Server _server; + private LocalConnector _connector; + private final AtomicReference _afterHandle = new AtomicReference<>(); + + @BeforeEach + public void prepare() throws Exception + { + _server = new Server(); + _connector = new LocalConnector(_server, new HttpConnectionFactory() + { + @Override + public Connection newConnection(Connector connector, EndPoint endPoint) + { + HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) + { + @Override + protected HttpChannel newHttpChannel(Server server, HttpConfiguration configuration) + { + return new HttpChannelState(this) + { + @Override + public Runnable onRequest(MetaData.Request request) + { + Runnable onRequest = super.onRequest(request); + if (onRequest == null) + return null; + + return () -> + { + try + { + onRequest.run(); + } + finally + { + Runnable after = _afterHandle.getAndSet(null); + if (after != null) + getThreadPool().execute(after); + } + }; + } + }; + } + }; + connection.setUseInputDirectByteBuffers(isUseInputDirectByteBuffers()); + connection.setUseOutputDirectByteBuffers(isUseOutputDirectByteBuffers()); + return configure(connection, connector, endPoint); + } + }); + _connector.setIdleTimeout(60000); + _server.addConnector(_connector); + } + + @AfterEach + public void dispose() throws Exception + { + LifeCycle.stop(_server); + _connector = null; + } + + @Test + public void testSimpleGET() throws Exception + { + _server.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain"); + Content.Sink.write(response, true, "Hello", callback); + return true; + } + }); + _server.start(); + + String request = """ + GET /path HTTP/1.0\r + Host: hostname\r + \r + """; + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request)); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContent(), is("Hello")); + } + + public static Stream completionScenarios() + { + List arguments = new ArrayList<>(); + for (Boolean succeeded : List.of(true, false)) + { + for (Boolean handling : List.of(true, false)) + { + for (Boolean written : List.of(true, false)) + { + for (Boolean last : written ? List.of(true, false) : List.of(false)) + { + arguments.add(Arguments.of(succeeded, handling, written, last)); + } + } + } + } + return arguments.stream(); + } + + @ParameterizedTest + @MethodSource("completionScenarios") + public void testCompletion(boolean succeeded, boolean handling, boolean written, boolean last) throws Exception + { + _server.setHandler(new Handler.Abstract(Invocable.InvocationType.BLOCKING) + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain"); + if (written) + { + try (Blocker.Callback blocker = Blocker.callback()) + { + Content.Sink.write(response, last, "Hello", blocker); + blocker.block(); + } + } + + Runnable complete = succeeded ? callback::succeeded : () -> callback.failed(new QuietException.Exception("Test")); + if (handling) + complete.run(); + else + _afterHandle.set(complete); + + return true; + } + }); + _server.start(); + + String request = """ + GET /path HTTP/1.1\r + Host: hostname\r + \r + """; + String rawResponse = _connector.getResponse(request); + // System.err.printf("succeeded=%b handling=%b written=%b last=%b%n", succeeded, handling, written, last); + // System.err.println(rawResponse); + + if (succeeded || written) + assertThat(rawResponse, containsString("HTTP/1.1 200 OK")); + else + assertThat(rawResponse, containsString("HTTP/1.1 500 Server Error")); + + if (written) + assertThat(rawResponse, containsString("Hello")); + else + assertThat(rawResponse, not(containsString("Hello"))); + + if (written && !last) + { + assertThat(rawResponse, containsString("chunked")); + if (succeeded) + assertThat(rawResponse, containsString("\r\n0\r\n")); + else + assertThat(rawResponse, not(containsString("\r\n0\r\n"))); + } + else + { + assertThat(rawResponse, containsString("Content-Length:")); + } + } +} diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java index 57218f1770dd..7e41062c308d 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java @@ -408,17 +408,17 @@ public boolean handle(Request request, Response response, Callback callback) assertThat(response, containsString("HTTP/1.1 500 Server Error")); } - assertEquals(1, _statsHandler.getRequests()); - assertEquals(0, _statsHandler.getRequestsActive()); - assertEquals(1, _statsHandler.getRequestsActiveMax()); + assertEquals(1, _statsHandler.getRequests(), "stats.requests"); + assertEquals(0, _statsHandler.getRequestsActive(), "stats.requestActive"); + assertEquals(1, _statsHandler.getRequestsActiveMax(), "stats.requestsActiveMax"); // We get no recorded status, but we get a recorded thrown response. - assertEquals(0, _statsHandler.getResponses1xx()); - assertEquals(0, _statsHandler.getResponses2xx()); - assertEquals(0, _statsHandler.getResponses3xx()); - assertEquals(0, _statsHandler.getResponses4xx()); - assertEquals(1, _statsHandler.getResponses5xx()); - assertEquals(1, _statsHandler.getHandlingFailures()); + assertEquals(0, _statsHandler.getResponses1xx(), "stats.responses1xx"); + assertEquals(0, _statsHandler.getResponses2xx(), "stats.responses2xx"); + assertEquals(0, _statsHandler.getResponses3xx(), "stats.responses3xx"); + assertEquals(0, _statsHandler.getResponses4xx(), "stats.responses4xx"); + assertEquals(1, _statsHandler.getResponses5xx(), "stats.responses5xx"); + assertEquals(1, _statsHandler.getHandlingFailures(), "stats.handlingFailures"); } @Test diff --git a/jetty-core/jetty-server/src/test/resources/jetty-logging.properties b/jetty-core/jetty-server/src/test/resources/jetty-logging.properties index 3a530148f336..553a8d6b3285 100644 --- a/jetty-core/jetty-server/src/test/resources/jetty-logging.properties +++ b/jetty-core/jetty-server/src/test/resources/jetty-logging.properties @@ -4,6 +4,14 @@ #org.eclipse.jetty.io.LEVEL=DEBUG #org.eclipse.jetty.http.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG +#org.eclipse.jetty.server.internal.HttpChannelState.LEVEL=DEBUG +#org.eclipse.jetty.server.LargeHeaderTest.LEVEL=DEBUG +#org.eclipse.jetty.server.ResponseCompleteTest.LEVEL=DEBUG +#org.eclipse.jetty.server.ErrorHandler.LEVEL=DEBUG +#org.eclipse.jetty.server.internal.LEVEL=DEBUG +#org.eclipse.jetty.server.Response.LEVEL=DEBUG +#org.eclipse.jetty.io.AbstractEndPoint.LEVEL=DEBUG +#org.eclipse.jetty.util.thread.SerializedInvoker.LEVEL=DEBUG #org.eclipse.jetty.server.GracefulStopTest.LEVEL=DEBUG #org.eclipse.jetty.server.GracefulStopTest$LatchHandler.LEVEL=DEBUG #org.eclipse.jetty.io.ManagedSelector.LEVEL=DEBUG diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java index fbe3b77b319b..2c91399e1e4f 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java @@ -422,6 +422,12 @@ public InvocationType getInvocationType() { return callback.getInvocationType(); } + + @Override + public String toString() + { + return "%s@%x:%s".formatted(getClass().getSimpleName(), hashCode(), callback); + } } static Callback combine(Callback cb1, Callback cb2) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedInvoker.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedInvoker.java index 9e7b8612d439..72bcd693a732 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedInvoker.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedInvoker.java @@ -205,7 +205,7 @@ public void run() @Override public String toString() { - return String.format("%s@%x[%s] -> %s", getClass().getSimpleName(), hashCode(), _task, _next); + return String.format("%s@%x{%s -> %s}", getClass().getSimpleName(), hashCode(), _task, _next); } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java index 394dc50d98db..f6fb3ad1e915 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.ee10.servlet; import java.io.IOException; -import java.util.Iterator; import java.util.Locale; import java.util.Map; import java.util.function.Supplier; @@ -26,7 +25,6 @@ import jakarta.servlet.http.HttpSession; import org.eclipse.jetty.ee10.servlet.writer.ResponseWriter; import org.eclipse.jetty.http.HttpCookie; -import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpGenerator; import org.eclipse.jetty.http.HttpHeader; @@ -357,26 +355,7 @@ public void resetContent() _encodingFrom = EncodingFrom.NOT_SET; // remove the content related response headers and keep all others - for (Iterator i = getHeaders().iterator(); i.hasNext(); ) - { - HttpField field = i.next(); - if (field.getHeader() == null) - continue; - - switch (field.getHeader()) - { - case CONTENT_TYPE, CONTENT_LENGTH, CONTENT_ENCODING, CONTENT_LANGUAGE, CONTENT_RANGE, CONTENT_MD5, - CONTENT_LOCATION, TRANSFER_ENCODING, CACHE_CONTROL, LAST_MODIFIED, EXPIRES, VARY -> i.remove(); - case ETAG -> - { - if (getStatus() != HttpStatus.NOT_MODIFIED_304) - i.remove(); - } - default -> - { - } - } - } + getHeaders().remove(getStatus() == HttpStatus.NOT_MODIFIED_304 ? HttpHeader.CONTENT_HEADERS_304 : HttpHeader.CONTENT_HEADERS); } /**