From 9c0207fccdc5fb66ecc9cbf129b533d2833d128e Mon Sep 17 00:00:00 2001 From: gregw Date: Mon, 1 May 2023 18:19:13 +0200 Subject: [PATCH] Fix #9685 merge to 12 Merging #9685 to 12 broke the build as it turned out that the error handling was depending on the attempt to remove the Date header to detect if the response was already committed during a sendError. --- .../jetty/ee10/servlet/ServletChannel.java | 69 +++++++++---------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index 05f1d094df2c..66e7c2d54c06 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -480,48 +480,41 @@ public boolean handle() Throwable cause = errorException instanceof Throwable throwable ? throwable : null; try { - if (_state.isResponseCommitted()) + // Get ready to send an error response + getResponse().resetContent(); + + // the following is needed as you cannot trust the response code and reason + // as those could have been modified after calling sendError + Integer code = (Integer)_servletContextRequest.getAttribute(RequestDispatcher.ERROR_STATUS_CODE); + if (code == null) + code = HttpStatus.INTERNAL_SERVER_ERROR_500; + getResponse().setStatus(code); + + // The handling of the original dispatch failed, and we are now going to either generate + // and error response ourselves or dispatch for an error page. If there is content left over + // from the failed dispatch, then we try to consume it here and if we fail we add a + // Connection:close. This can't be deferred to COMPLETE as the response will be committed + // by then. + if (!_httpInput.consumeAvailable()) + ResponseUtils.ensureNotPersistent(_servletContextRequest, _servletContextRequest.getResponse()); + + ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT); + Request.Handler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler()); + + // If we can't have a body or have no ErrorHandler, then create a minimal error response. + if (HttpStatus.hasNoBody(getResponse().getStatus()) || errorHandler == null) { - abort(cause == null ? new IllegalStateException("Committed on sendError") : cause); + sendResponseAndComplete(); } else { - // Get ready to send an error response - getResponse().resetContent(); - - // the following is needed as you cannot trust the response code and reason - // as those could have been modified after calling sendError - Integer code = (Integer)_servletContextRequest.getAttribute(RequestDispatcher.ERROR_STATUS_CODE); - if (code == null) - code = HttpStatus.INTERNAL_SERVER_ERROR_500; - getResponse().setStatus(code); - - // The handling of the original dispatch failed, and we are now going to either generate - // and error response ourselves or dispatch for an error page. If there is content left over - // from the failed dispatch, then we try to consume it here and if we fail we add a - // Connection:close. This can't be deferred to COMPLETE as the response will be committed - // by then. - if (!_httpInput.consumeAvailable()) - ResponseUtils.ensureNotPersistent(_servletContextRequest, _servletContextRequest.getResponse()); - - ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT); - Request.Handler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler()); - - // If we can't have a body or have no ErrorHandler, then create a minimal error response. - if (HttpStatus.hasNoBody(getResponse().getStatus()) || errorHandler == null) + // TODO: do this non-blocking. + // Callback completeCallback = Callback.from(() -> _state.completed(null), _state::completed); + // _state.completing(); + try (Blocker.Callback blocker = Blocker.callback()) { - sendResponseAndComplete(); - } - else - { - // TODO: do this non-blocking. - // Callback completeCallback = Callback.from(() -> _state.completed(null), _state::completed); - // _state.completing(); - try (Blocker.Callback blocker = Blocker.callback()) - { - dispatch(() -> errorHandler.handle(_servletContextRequest, getResponse(), blocker)); - blocker.block(); - } + dispatch(() -> errorHandler.handle(_servletContextRequest, getResponse(), blocker)); + blocker.block(); } } } @@ -534,7 +527,9 @@ else if (ExceptionUtil.areNotAssociated(cause, x)) if (LOG.isDebugEnabled()) LOG.debug("Could not perform ERROR dispatch, aborting", cause); if (_state.isResponseCommitted()) + { abort(cause); + } else { try