diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java index a18eca349713..ef439ef405b7 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java @@ -23,6 +23,7 @@ import org.eclipse.jetty.security.internal.DeferredAuthenticationState; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.util.Callback; /** @@ -179,6 +180,19 @@ static boolean logout(Request request, Response response) return false; } + static AuthenticationState writeError(Request request, Response response, Callback callback, int code) + { + if (request.getContext().getErrorHandler() instanceof ErrorHandler errorHandler) + { + return errorHandler.writeError(request, response, callback, HttpStatus.FORBIDDEN_403) + ? AuthenticationState.SEND_FAILURE + : new AuthenticationState.ServeAs(request.getHttpURI()); + } + + Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); + return AuthenticationState.SEND_FAILURE; + } + /** * A successful Authentication with User information. */ diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java index f27b6570e41b..a58fe63654a5 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/BasicAuthenticator.java @@ -87,6 +87,8 @@ public AuthenticationState validateRequest(Request req, Response res, Callback c if (charset != null) value += ", charset=\"" + charset.name() + "\""; res.getHeaders().put(HttpHeader.WWW_AUTHENTICATE.asString(), value); + + // Don't use AuthenticationState.writeError, to avoid possibility of doing a Servlet error dispatch. Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401); return AuthenticationState.CHALLENGE; } diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java index 7d3430b22f34..f9d695100037 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java @@ -180,8 +180,9 @@ else if (n == 0) "\", algorithm=MD5" + ", qop=\"auth\"" + ", stale=" + stale); - Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401); + // Don't use AuthenticationState.writeError, to avoid possibility of doing a Servlet error dispatch. + Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401); return AuthenticationState.CHALLENGE; } diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java index 8aa1eda5960a..0de9cbcaa007 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java @@ -365,7 +365,7 @@ private AuthenticationState dispatch(String path, Request request, Response resp private AuthenticationState sendError(Request request, Response response, Callback callback) { if (_formErrorPage == null) - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); else if (_dispatch) return dispatch(_formErrorPage, request, response, callback); else diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SPNEGOAuthenticator.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SPNEGOAuthenticator.java index 60458ffc6ab2..f83c6a79a63c 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SPNEGOAuthenticator.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SPNEGOAuthenticator.java @@ -185,6 +185,7 @@ else if (httpSession != null) private void sendChallenge(Request req, Response res, Callback callback, String token) { setSpnegoToken(res, token); + // Don't use AuthenticationState.writeError, to avoid possibility of doing a Servlet error dispatch. Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401); } diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SslClientCertAuthenticator.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SslClientCertAuthenticator.java index 6d82564bdc40..5739ddbd18e9 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SslClientCertAuthenticator.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SslClientCertAuthenticator.java @@ -56,11 +56,8 @@ public String getAuthenticationType() public AuthenticationState validateRequest(Request req, Response res, Callback callback) throws ServerAuthException { if (!(req.getAttribute(EndPoint.SslSessionData.ATTRIBUTE) instanceof EndPoint.SslSessionData sslSessionData)) - { - Response.writeError(req, res, callback, HttpStatus.FORBIDDEN_403); - return AuthenticationState.SEND_FAILURE; - } - + return AuthenticationState.writeError(req, res, callback, HttpStatus.FORBIDDEN_403); + X509Certificate[] certs = sslSessionData.peerCertificates(); try @@ -106,10 +103,7 @@ public AuthenticationState validateRequest(Request req, Response res, Callback c } if (!AuthenticationState.Deferred.isDeferred(res)) - { - Response.writeError(req, res, callback, HttpStatus.FORBIDDEN_403); - return AuthenticationState.SEND_FAILURE; - } + return AuthenticationState.writeError(req, res, callback, HttpStatus.FORBIDDEN_403); return null; } 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 f6a59f7f1200..64257e519442 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 @@ -92,6 +92,25 @@ public boolean errorPageForMethod(String method) return ERROR_METHODS.contains(method); } + /** + * Write an error response. + *

+ * In Servlet implementations of the {@link ErrorHandler}, this method is overridden to signal that + * a sendError should be triggered a when it enters the servlet channel. + *

+ * + * @param request The request. + * @param response The response. + * @param callback The callback to call when the response is written. + * @param code The error status code. + * @return True if the error response was written. + */ + public boolean writeError(Request request, Response response, Callback callback, int code) + { + Response.writeError(request, response, callback, code); + return true; + } + @Override public boolean handle(Request request, Response response, Callback callback) throws Exception { diff --git a/jetty-ee10/jetty-ee10-jaspi/src/main/java/org/eclipse/jetty/ee10/security/jaspi/JaspiAuthenticator.java b/jetty-ee10/jetty-ee10-jaspi/src/main/java/org/eclipse/jetty/ee10/security/jaspi/JaspiAuthenticator.java index 73654970c536..565ce7efac38 100644 --- a/jetty-ee10/jetty-ee10-jaspi/src/main/java/org/eclipse/jetty/ee10/security/jaspi/JaspiAuthenticator.java +++ b/jetty-ee10/jetty-ee10-jaspi/src/main/java/org/eclipse/jetty/ee10/security/jaspi/JaspiAuthenticator.java @@ -233,8 +233,7 @@ public AuthenticationState validateRequest(JaspiMessageInfo messageInfo) throws } if (authStatus == AuthStatus.FAILURE) { - Response.writeError(messageInfo.getBaseRequest(), messageInfo.getBaseResponse(), messageInfo.getCallback(), HttpServletResponse.SC_FORBIDDEN); - return AuthenticationState.SEND_FAILURE; + return AuthenticationState.writeError(messageInfo.getBaseRequest(), messageInfo.getBaseResponse(), messageInfo.getCallback(), HttpServletResponse.SC_FORBIDDEN); } // should not happen throw new IllegalStateException("No AuthStatus returned"); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java index ecc88d6e0a7b..b8cc0c0f5222 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java @@ -14,64 +14,50 @@ package org.eclipse.jetty.ee10.servlet; import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.io.StringWriter; +import java.io.UncheckedIOException; import java.io.Writer; -import java.nio.BufferOverflowException; -import java.nio.ByteBuffer; import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.http.QuotedQualityCSV; -import org.eclipse.jetty.io.ByteBufferOutputStream; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ErrorHandler implements Request.Handler +public class ErrorHandler extends org.eclipse.jetty.server.handler.ErrorHandler { - // TODO This classes API needs to be majorly refactored/cleanup in jetty-10 private static final Logger LOG = LoggerFactory.getLogger(ErrorHandler.class); - public static final String ERROR_PAGE = "org.eclipse.jetty.server.error_page"; - public static final String ERROR_CONTEXT = "org.eclipse.jetty.server.error_context"; - public static final String ERROR_CHARSET = "org.eclipse.jetty.server.error_charset"; - - boolean _showServlet = true; - boolean _showStacks = true; - boolean _disableStacks = false; - boolean _showMessageInTitle = true; - String _cacheControl = "must-revalidate,no-cache,no-store"; public ErrorHandler() { + setShowOrigin(true); + setShowStacks(true); + setShowMessageInTitle(true); } - public boolean errorPageForMethod(String method) + @Override + public boolean writeError(Request request, Response response, Callback callback, int code) { - return switch (method) + // If we have not entered the servlet channel we should trigger a sendError for when we do enter the servlet channel. + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + boolean enteredServletChannel = servletContextRequest.getServletChannel().getCallback() != null; + if (!enteredServletChannel) { - case "GET", "POST", "HEAD" -> true; - default -> false; - }; + response.setStatus(code); + request.setAttribute(ERROR_STATUS, code); + return false; + } + + return super.writeError(request, response, callback, code); } @Override @@ -83,6 +69,8 @@ public boolean handle(Request request, Response response, Callback callback) thr return true; } + generateCacheControl(response); + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); HttpServletRequest httpServletRequest = servletContextRequest.getServletApiRequest(); HttpServletResponse httpServletResponse = servletContextRequest.getHttpServletResponse(); @@ -94,311 +82,66 @@ public boolean handle(Request request, Response response, Callback callback) thr // Look for an error page dispatcher // This logic really should be in ErrorPageErrorHandler, but some implementations extend ErrorHandler // and implement ErrorPageMapper directly, so we do this here in the base class. - String errorPage = (this instanceof ErrorPageMapper) ? ((ErrorPageMapper)this).getErrorPage(httpServletRequest) : null; ServletContextHandler.ServletScopedContext context = servletContextRequest.getErrorContext(); - Dispatcher errorDispatcher = (errorPage != null && context != null) - ? (Dispatcher)context.getServletContext().getRequestDispatcher(errorPage) : null; - - if (errorDispatcher != null) - { - try + Integer errorStatus = (Integer)request.getAttribute(ERROR_STATUS); + Throwable errorCause = (Throwable)request.getAttribute(ERROR_EXCEPTION); + boolean enteredServletChannel = servletContextRequest.getServletChannel().getCallback() != null; + if (this instanceof ErrorPageMapper mapper && enteredServletChannel) + { + ErrorPageMapper.ErrorPage errorPage = mapper.getErrorPage(errorStatus, errorCause); + if (LOG.isDebugEnabled()) + LOG.debug("{} {} {} -> {}", context, errorStatus, errorCause, errorPage); + if (errorPage != null && context.getServletContext().getRequestDispatcher(errorPage.errorPage) instanceof Dispatcher errorDispatcher) { try { - contextHandler.requestInitialized(servletContextRequest, httpServletRequest); - errorDispatcher.error(httpServletRequest, httpServletResponse); - } - finally - { - contextHandler.requestDestroyed(servletContextRequest, httpServletRequest); + try + { + mapper.prepare(errorPage, httpServletRequest, httpServletResponse); + contextHandler.requestInitialized(servletContextRequest, httpServletRequest); + errorDispatcher.error(httpServletRequest, httpServletResponse); + } + finally + { + contextHandler.requestDestroyed(servletContextRequest, httpServletRequest); + } + callback.succeeded(); + return true; } - callback.succeeded(); - return true; - } - catch (ServletException e) - { - if (LOG.isDebugEnabled()) - LOG.debug("Unable to call error dispatcher", e); - if (response.isCommitted()) + catch (ServletException e) { - callback.failed(e); - return true; + if (LOG.isDebugEnabled()) + LOG.debug("Unable to call error dispatcher", e); + if (response.isCommitted()) + { + callback.failed(e); + return true; + } } } } - String message = (String)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_MESSAGE); + String message = (String)request.getAttribute(ERROR_MESSAGE); if (message == null) message = HttpStatus.getMessage(response.getStatus()); - generateAcceptableResponse(servletContextRequest, httpServletRequest, httpServletResponse, response.getStatus(), message); + generateResponse(request, response, response.getStatus(), message, (Throwable)request.getAttribute(ERROR_EXCEPTION), callback); callback.succeeded(); return true; } - /** - * Generate an acceptable error response. - *

This method is called to generate an Error page of a mime type that is - * acceptable to the user-agent. The Accept header is evaluated in - * quality order and the method - * {@link #generateAcceptableResponse(ServletContextRequest, HttpServletRequest, HttpServletResponse, int, String, String)} - * is called for each mimetype until the response is written to or committed.

- * - * @param baseRequest The base request - * @param request The servlet request (may be wrapped) - * @param response The response (may be wrapped) - * @param code the http error code - * @param message the http error message - * @throws IOException if the response cannot be generated - */ - protected void generateAcceptableResponse(ServletContextRequest baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message) throws IOException - { - List acceptable = baseRequest.getHeaders().getQualityCSV(HttpHeader.ACCEPT, QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING); - - if (acceptable.isEmpty() && !baseRequest.getHeaders().contains(HttpHeader.ACCEPT)) - { - generateAcceptableResponse(baseRequest, request, response, code, message, MimeTypes.Type.TEXT_HTML.asString()); - } - else - { - for (String mimeType : acceptable) - { - generateAcceptableResponse(baseRequest, request, response, code, message, mimeType); - if (response.isCommitted() || baseRequest.getServletContextResponse().isWritingOrStreaming()) - break; - } - } - } - - /** - * Returns an acceptable writer for an error page. - *

Uses the user-agent's Accept-Charset to get response - * {@link Writer}. The acceptable charsets are tested in quality order - * if they are known to the JVM and the first known is set on - * {@link HttpServletResponse#setCharacterEncoding(String)} and the - * {@link HttpServletResponse#getWriter()} method used to return a writer. - * If there is no Accept-Charset header then - * ISO-8859-1 is used. If '*' is the highest quality known - * charset, then utf-8 is used. - *

- * - * @param baseRequest The base request - * @param request The servlet request (may be wrapped) - * @param response The response (may be wrapped) - * @return A {@link Writer} if there is a known acceptable charset or null - * @throws IOException if a Writer cannot be returned - */ - @Deprecated - protected Writer getAcceptableWriter(Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - List acceptable = baseRequest.getHeaders().getQualityCSV(HttpHeader.ACCEPT_CHARSET); - if (acceptable.isEmpty()) - { - response.setCharacterEncoding(StandardCharsets.ISO_8859_1.name()); - return response.getWriter(); - } - - for (String charset : acceptable) - { - try - { - if ("*".equals(charset)) - response.setCharacterEncoding(StandardCharsets.UTF_8.name()); - else - response.setCharacterEncoding(Charset.forName(charset).name()); - return response.getWriter(); - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } - } - return null; - } - - /** - * Generate an acceptable error response for a mime type. - *

This method is called for each mime type in the users agent's - * Accept header, a response of the appropriate type is generated. - *

- *

The default implementation handles "text/html", "text/*" and "*/*". - * The method can be overridden to handle other types. Implementations must - * immediate produce a response and may not be async. - *

- * - * @param baseRequest The base request - * @param request The servlet request (may be wrapped) - * @param response The response (may be wrapped) - * @param code the http error code - * @param message the http error message - * @param contentType The mimetype to generate (may be */*or other wildcard) - * @throws IOException if a response cannot be generated - */ - protected void generateAcceptableResponse(ServletContextRequest baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message, String contentType) throws IOException + protected boolean generateAcceptableResponse(Request request, Response response, Callback callback, String contentType, List charsets, int code, String message, Throwable cause) throws IOException { - // We can generate an acceptable contentType, but can we generate an acceptable charset? - // TODO refactor this in jetty-10 to be done in the other calling loop - Charset charset = null; - List acceptable = baseRequest.getHeaders().getQualityCSV(HttpHeader.ACCEPT_CHARSET); - if (!acceptable.isEmpty()) - { - for (String name : acceptable) - { - if ("*".equals(name)) - { - charset = StandardCharsets.UTF_8; - break; - } - - try - { - charset = Charset.forName(name); - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } - } - if (charset == null) - return; - } - - MimeTypes.Type type; - switch (contentType) - { - case "text/html": - case "text/*": - case "*/*": - type = MimeTypes.Type.TEXT_HTML; - if (charset == null) - charset = StandardCharsets.ISO_8859_1; - break; - - case "text/json": - case "application/json": - type = MimeTypes.Type.TEXT_JSON; - if (charset == null) - charset = StandardCharsets.UTF_8; - break; - - case "text/plain": - type = MimeTypes.Type.TEXT_PLAIN; - if (charset == null) - charset = StandardCharsets.ISO_8859_1; - break; - - default: - return; - } - - // write into the response aggregate buffer and flush it asynchronously. - while (true) + boolean result = super.generateAcceptableResponse(request, response, callback, contentType, charsets, code, message, cause); + if (result) { - try - { - // TODO currently the writer used here is of fixed size, so a large - // TODO error page may cause a BufferOverflow. In which case we try - // TODO again with stacks disabled. If it still overflows, it is - // TODO written without a body. - ByteBuffer buffer = baseRequest.getServletContextResponse().getHttpOutput().getByteBuffer(); - ByteBufferOutputStream out = new ByteBufferOutputStream(buffer); - PrintWriter writer = new PrintWriter(new OutputStreamWriter(out, charset)); - - switch (type) - { - case TEXT_HTML: - response.setContentType(MimeTypes.Type.TEXT_HTML.asString()); - response.setCharacterEncoding(charset.name()); - request.setAttribute(ERROR_CHARSET, charset); - handleErrorPage(request, writer, code, message); - break; - case TEXT_JSON: - response.setContentType(contentType); - writeErrorJson(request, writer, code, message); - break; - case TEXT_PLAIN: - response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); - response.setCharacterEncoding(charset.name()); - writeErrorPlain(request, writer, code, message); - break; - default: - throw new IllegalStateException(); - } - - writer.flush(); - break; - } - catch (BufferOverflowException e) - { - if (LOG.isDebugEnabled()) - LOG.warn("Error page too large: {} {} {}", code, message, request, e); - else - LOG.warn("Error page too large: {} {} {}", code, message, request); - baseRequest.getServletContextResponse().resetContent(); - if (!_disableStacks) - { - LOG.info("Disabling showsStacks for {}", this); - _disableStacks = true; - continue; - } - break; - } + // Do an asynchronous completion + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + servletContextRequest.getServletChannel().sendErrorResponseAndComplete(); } - - // Do an asynchronous completion. - baseRequest.getServletChannel().sendErrorResponseAndComplete(); - } - - protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException - { - writeErrorPage(request, writer, code, message, _showStacks); - } - - protected void writeErrorPage(HttpServletRequest request, Writer writer, int code, String message, boolean showStacks) throws IOException - { - if (message == null) - message = HttpStatus.getMessage(code); - - writer.write("\n\n"); - writeErrorPageHead(request, writer, code, message); - writer.write("\n"); - writeErrorPageBody(request, writer, code, message, showStacks); - writer.write("\n\n\n"); + return result; } - protected void writeErrorPageHead(HttpServletRequest request, Writer writer, int code, String message) throws IOException - { - Charset charset = (Charset)request.getAttribute(ERROR_CHARSET); - if (charset != null) - { - writer.write("\n"); - } - writer.write("Error "); - // TODO this code is duplicated in writeErrorPageMessage - String status = Integer.toString(code); - writer.write(status); - if (message != null && !message.equals(status)) - { - writer.write(' '); - writer.write(StringUtil.sanitizeXmlString(message)); - } - writer.write("\n"); - } - - protected void writeErrorPageBody(HttpServletRequest request, Writer writer, int code, String message, boolean showStacks) throws IOException - { - String uri = request.getRequestURI(); - - writeErrorPageMessage(request, writer, code, message, uri); - if (showStacks && !_disableStacks) - writeErrorPageStacks(request, writer); - - ((ServletApiRequest)request).getServletRequestInfo().getServletChannel().getHttpConfiguration() - .writePoweredBy(writer, "
", "
\n"); - } - - protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException + protected void writeErrorHtmlMessage(Request request, Writer writer, int code, String message, Throwable cause, String uri) throws IOException { writer.write("

HTTP ERROR "); String status = Integer.toString(code); @@ -413,11 +156,18 @@ protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, htmlRow(writer, "URI", uri); htmlRow(writer, "STATUS", status); htmlRow(writer, "MESSAGE", message); - if (isShowServlet()) + writeErrorOrigin((String)request.getAttribute(ERROR_ORIGIN), (o) -> { - htmlRow(writer, "SERVLET", request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN)); - } - Throwable cause = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); + try + { + htmlRow(writer, "SERVLET", o); + } + catch (IOException x) + { + throw new UncheckedIOException(x); + } + }); + while (cause != null) { htmlRow(writer, "CAUSED BY", cause); @@ -426,187 +176,21 @@ protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, writer.write("\n"); } - private void htmlRow(Writer writer, String tag, Object value) throws IOException - { - writer.write(""); - writer.write(tag); - writer.write(":"); - if (value == null) - writer.write("-"); - else - writer.write(StringUtil.sanitizeXmlString(value.toString())); - writer.write("\n"); - } - - protected void writeErrorPlain(HttpServletRequest request, PrintWriter writer, int code, String message) + public interface ErrorPageMapper { - writer.write("HTTP ERROR "); - writer.write(Integer.toString(code)); - writer.write(' '); - writer.write(StringUtil.sanitizeXmlString(message)); - writer.write("\n"); - writer.printf("URI: %s%n", request.getRequestURI()); - writer.printf("STATUS: %s%n", code); - writer.printf("MESSAGE: %s%n", message); - if (isShowServlet()) + enum PageLookupTechnique { - writer.printf("SERVLET: %s%n", request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN)); - } - Throwable cause = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - while (cause != null) - { - writer.printf("CAUSED BY %s%n", cause); - if (isShowStacks() && !_disableStacks) - { - cause.printStackTrace(writer); - } - cause = cause.getCause(); + THROWABLE, STATUS_CODE, GLOBAL } - } - protected void writeErrorJson(HttpServletRequest request, PrintWriter writer, int code, String message) - { - Throwable cause = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - Object servlet = request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN); - Map json = new HashMap<>(); - - json.put("url", request.getRequestURI()); - json.put("status", Integer.toString(code)); - json.put("message", message); - if (isShowServlet() && servlet != null) - { - json.put("servlet", servlet.toString()); - } - int c = 0; - while (cause != null) + record ErrorPage(String errorPage, PageLookupTechnique match, Throwable error, Throwable cause, Class matchedClass) { - json.put("cause" + c++, cause.toString()); - cause = cause.getCause(); } - writer.append(json.entrySet().stream() - .map(e -> HttpField.NAME_VALUE_TOKENIZER.quote(e.getKey()) + ":" + HttpField.NAME_VALUE_TOKENIZER.quote(StringUtil.sanitizeXmlString((e.getValue())))) - .collect(Collectors.joining(",\n", "{\n", "\n}"))); - } + ErrorPage getErrorPage(Integer errorStatusCode, Throwable error); - protected void writeErrorPageStacks(HttpServletRequest request, Writer writer) throws IOException - { - Throwable th = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - if (th != null) - { - writer.write("

Caused by:

");
-            // You have to pre-generate and then use #write(writer, String)
-            try (StringWriter sw = new StringWriter();
-                 PrintWriter pw = new PrintWriter(sw))
-            {
-                th.printStackTrace(pw);
-                pw.flush();
-                write(writer, sw.getBuffer().toString()); // sanitize
-            }
-            writer.write("
\n"); - } - } - - /** - * Bad Message Error body - *

Generate an error response body to be sent for a bad message. - * In this case there is something wrong with the request, so either - * a request cannot be built, or it is not safe to build a request. - * This method allows for a simple error page body to be returned - * and some response headers to be set. - * - * @param status The error code that will be sent - * @param reason The reason for the error code (may be null) - * @param fields The header fields that will be sent with the response. - * @return The content as a ByteBuffer, or null for no body. - */ - public ByteBuffer badMessageError(int status, String reason, HttpFields.Mutable fields) - { - if (reason == null) - reason = HttpStatus.getMessage(status); - if (HttpStatus.hasNoBody(status)) - return BufferUtil.EMPTY_BUFFER; - fields.put(HttpHeader.CONTENT_TYPE, MimeTypes.Type.TEXT_HTML_8859_1.asString()); - return BufferUtil.toBuffer("

Bad Message " + status + "

reason: " + reason + "
"); - } - - /** - * Get the cacheControl. - * - * @return the cacheControl header to set on error responses. - */ - public String getCacheControl() - { - return _cacheControl; - } - - /** - * Set the cacheControl. - * - * @param cacheControl the cacheControl header to set on error responses. - */ - public void setCacheControl(String cacheControl) - { - _cacheControl = cacheControl; - } - - /** - * @return True if the error page will show the Servlet that generated the error - */ - public boolean isShowServlet() - { - return _showServlet; - } - - /** - * @param showServlet True if the error page will show the Servlet that generated the error - */ - public void setShowServlet(boolean showServlet) - { - _showServlet = showServlet; - } - - /** - * @return True if stack traces are shown in the error pages - */ - public boolean isShowStacks() - { - return _showStacks; - } - - /** - * @param showStacks True if stack traces are shown in the error pages - */ - public void setShowStacks(boolean showStacks) - { - _showStacks = showStacks; - } - - /** - * Set if true, the error message appears in page title. - * @param showMessageInTitle if true, the error message appears in page title - */ - public void setShowMessageInTitle(boolean showMessageInTitle) - { - _showMessageInTitle = showMessageInTitle; - } - - public boolean getShowMessageInTitle() - { - return _showMessageInTitle; - } - - protected void write(Writer writer, String string) throws IOException - { - if (string == null) - return; - - writer.write(StringUtil.sanitizeXmlString(string)); - } - - public interface ErrorPageMapper - { - String getErrorPage(HttpServletRequest request); + default void prepare(ErrorPage errorPage, HttpServletRequest request, HttpServletResponse response) + {} } public static Request.Handler getErrorHandler(Server server, ContextHandler context) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java index 5f3892e432fd..a27da8ad08a4 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java @@ -20,6 +20,7 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,11 +33,6 @@ public class ErrorPageErrorHandler extends ErrorHandler implements ErrorHandler. public static final String GLOBAL_ERROR_PAGE = "org.eclipse.jetty.server.error_page.global"; private static final Logger LOG = LoggerFactory.getLogger(ErrorPageErrorHandler.class); - private enum PageLookupTechnique - { - THROWABLE, STATUS_CODE, GLOBAL - } - private final Map _errorPages = new HashMap<>(); // code or exception to URL private final List _errorPageList = new ArrayList<>(); // list of ErrorCode by range private boolean _unwrapServletException = true; @@ -58,119 +54,62 @@ public void setUnwrapServletException(boolean unwrapServletException) } @Override - public String getErrorPage(HttpServletRequest request) + public void prepare(ErrorPage errorPage, HttpServletRequest request, HttpServletResponse response) { - String errorPage = null; - - PageLookupTechnique pageSource = null; - - Class matchedThrowable = null; - Throwable error = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - Throwable cause = error; - - // Walk the cause hierarchy - while (errorPage == null && cause != null) + if (errorPage.error() instanceof ServletException && _unwrapServletException) { - pageSource = PageLookupTechnique.THROWABLE; - - Class exClass = cause.getClass(); - errorPage = _errorPages.get(exClass.getName()); - - // walk the inheritance hierarchy - while (errorPage == null) - { - exClass = exClass.getSuperclass(); - if (exClass == null) - break; - errorPage = _errorPages.get(exClass.getName()); - } - - if (errorPage != null) - matchedThrowable = exClass; - - cause = (cause instanceof ServletException) ? ((ServletException)cause).getRootCause() : null; - } - - if (error instanceof ServletException && _unwrapServletException) - { - Throwable unwrapped = unwrapServletException(error, matchedThrowable); + Throwable unwrapped = unwrapServletException(errorPage.error(), errorPage.matchedClass()); if (unwrapped != null) { - request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped); - request.setAttribute(Dispatcher.ERROR_EXCEPTION_TYPE, unwrapped.getClass()); request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION, unwrapped); + request.setAttribute(Dispatcher.ERROR_EXCEPTION_TYPE, unwrapped.getClass()); } } + } - Integer errorStatusCode = null; + @Override + public ErrorPage getErrorPage(Integer errorStatusCode, Throwable error) + { + String errorPage; - if (errorPage == null) + // Walk the cause hierarchy + Throwable cause = error; + while (cause != null) { - pageSource = PageLookupTechnique.STATUS_CODE; + Class exClass = cause.getClass(); - // look for an exact code match - errorStatusCode = (Integer)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS); - if (errorStatusCode != null) + while (exClass != null) { - errorPage = _errorPages.get(Integer.toString(errorStatusCode)); - - // if still not found - if (errorPage == null) - { - // look for an error code range match. - for (ErrorCodeRange errCode : _errorPageList) - { - if (errCode.isInRange(errorStatusCode)) - { - errorPage = errCode.getUri(); - break; - } - } - } + errorPage = _errorPages.get(exClass.getName()); + if (errorPage != null) + return new ErrorPage(errorPage, PageLookupTechnique.THROWABLE, error, cause, exClass); + exClass = exClass.getSuperclass(); } - } - // Try servlet 3.x global error page. - if (errorPage == null) - { - pageSource = PageLookupTechnique.GLOBAL; - errorPage = _errorPages.get(GLOBAL_ERROR_PAGE); + cause = (cause instanceof ServletException se) ? se.getRootCause() : cause.getCause(); } - if (LOG.isDebugEnabled()) + // look for an exact code match + if (errorStatusCode != null) { - StringBuilder dbg = new StringBuilder(); - dbg.append("getErrorPage("); - dbg.append(request.getMethod()).append(' '); - dbg.append(request.getRequestURI()); - dbg.append(") => error_page=").append(errorPage); - switch (pageSource) + errorPage = _errorPages.get(Integer.toString(errorStatusCode)); + if (errorPage != null) + return new ErrorPage(errorPage, PageLookupTechnique.STATUS_CODE, error, error, null); + + // look for an error code range match. + for (ErrorCodeRange errCode : _errorPageList) { - case THROWABLE: - dbg.append(" (using matched Throwable "); - dbg.append(matchedThrowable.getName()); - dbg.append(" / actually thrown as "); - Throwable originalThrowable = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - dbg.append(originalThrowable.getClass().getName()); - dbg.append(')'); - LOG.debug(dbg.toString(), cause); - break; - case STATUS_CODE: - dbg.append(" (from status code "); - dbg.append(errorStatusCode); - dbg.append(')'); - LOG.debug(dbg.toString()); - break; - case GLOBAL: - dbg.append(" (from global default)"); - LOG.debug(dbg.toString()); - break; - default: - throw new IllegalStateException(pageSource.toString()); + if (errCode.isInRange(errorStatusCode)) + return new ErrorPage(errCode.getUri(), PageLookupTechnique.STATUS_CODE, error, error, null); } } - return errorPage; + // Try servlet 3.x global error page. + errorPage = _errorPages.get(GLOBAL_ERROR_PAGE); + if (errorPage != null) + return new ErrorPage(errorPage, PageLookupTechnique.GLOBAL, error, error, null); + + return null; } /** diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index 42e4d2994357..957d8b3adecd 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -1199,13 +1199,14 @@ protected boolean handleByContextHandler(String pathInContext, ContextRequest re if (isProtectedTarget(pathInContext)) { - // At this point we have not entered the state machine of the ServletChannelState, so we do nothing here - // other than to set the error status attribute (and also status). When execution proceeds normally into the - // state machine the request will be treated as an error. Note that we set both the error status and request - // status because the request status is cheaper to check than the error status attribute. - request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS, 404); - response.setStatus(HttpStatus.NOT_FOUND_404); + // If we have a Servlet ErrorHandler, then writeError will return false, and it will signal + // the ServletChannel to trigger a sendError() when it is started. + if (request.getContext().getErrorHandler() instanceof org.eclipse.jetty.server.handler.ErrorHandler errorHandler) + return errorHandler.writeError(request, response, callback, HttpStatus.NOT_FOUND_404); + Response.writeError(request, response, callback, HttpStatus.NOT_FOUND_404); + return true; } + return false; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncListenerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncListenerTest.java index 747ff2cd085e..4490e125de00 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncListenerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncListenerTest.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.io.QuietException; import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; @@ -160,10 +161,10 @@ public void testStartAsyncThrowOnErrorSendErrorCustomErrorPage() throws Exceptio ErrorHandler errorHandler = new ErrorHandler() { @Override - protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException + protected void writeErrorHtmlMessage(Request request, Writer writer, int code, String message, Throwable cause, String uri) throws IOException { writer.write("CUSTOM\n"); - super.writeErrorPageMessage(request, writer, code, message, uri); + super.writeErrorHtmlMessage(request, writer, code, message, cause, uri); } }; server.setErrorHandler(errorHandler); @@ -314,10 +315,10 @@ public void testStartAsyncOnTimeoutSendErrorCustomErrorPage() throws Exception ErrorHandler errorHandler = new ErrorHandler() { @Override - protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException + protected void writeErrorHtmlMessage(Request request, Writer writer, int code, String message, Throwable cause, String uri) throws IOException { writer.write("CUSTOM\n"); - super.writeErrorPageMessage(request, writer, code, message, uri); + super.writeErrorHtmlMessage(request, writer, code, message, cause, uri); } }; server.setErrorHandler(errorHandler); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java index 894b6c6c4ad9..f06d28cecf97 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java @@ -741,6 +741,59 @@ protected void doGet(HttpServletRequest req, HttpServletResponse response) throw assertThat(responseBody, Matchers.containsString("getParameterMap()[ ]=[]")); } + @Test + public void testErrorAttributes() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + HttpServlet failServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws IOException + { + response.sendError(599); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + doGet(req, resp); + } + }; + + contextHandler.addServlet(failServlet, "/fail/599"); + contextHandler.addServlet(ErrorDumpServlet.class, "/error/*"); + + ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler(); + errorPageErrorHandler.addErrorPage(599, "/error/599"); + contextHandler.setErrorHandler(errorPageErrorHandler); + + startServer(contextHandler); + + String rawRequest = """ + POST /fail/599?name=value HTTP/1.1\r + Host: test\r + Connection: close\r + \r + """; + + String rawResponse = _connector.getResponse(rawRequest); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(599)); + assertThat(response.get(HttpHeader.DATE), notNullValue()); + + String responseBody = response.getContent(); + + assertThat(responseBody, Matchers.containsString("ERROR_PAGE: /599")); + assertThat(responseBody, Matchers.containsString("ERROR_CODE: 599")); + assertThat(responseBody, Matchers.containsString("ERROR_EXCEPTION: null")); + assertThat(responseBody, Matchers.containsString("ERROR_EXCEPTION_TYPE: null")); + assertThat(responseBody, Matchers.containsString("ERROR_SERVLET: " + failServlet.getClass().getName())); + assertThat(responseBody, Matchers.containsString("ERROR_REQUEST_URI: /fail/599")); + } + @Test public void testErrorCode() throws Exception { diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java index 101b27c8e971..208865b5088c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java @@ -20,6 +20,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee10.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.security.Constraint; import org.eclipse.jetty.security.EmptyLoginService; @@ -28,25 +29,28 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; public class FormAuthenticatorTest { private Server _server; private LocalConnector _connector; - @BeforeEach - public void configureServer() throws Exception + public void configureServer(FormAuthenticator authenticator) throws Exception { _server = new Server(); _connector = new LocalConnector(_server); _server.addConnector(_connector); ServletContextHandler contextHandler = new ServletContextHandler("/ctx", ServletContextHandler.SESSIONS); + ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler(); + errorPageErrorHandler.addErrorPage(403, "/servletErrorPage"); + contextHandler.setErrorHandler(errorPageErrorHandler); + _server.setHandler(contextHandler); contextHandler.addServlet(new AuthenticationTestServlet(), "/"); @@ -56,7 +60,7 @@ public void configureServer() throws Exception securityHandler.put("/any/*", Constraint.ANY_USER); securityHandler.put("/known/*", Constraint.KNOWN_ROLE); securityHandler.put("/admin/*", Constraint.from("admin")); - securityHandler.setAuthenticator(new FormAuthenticator("/login", "/error", true)); + securityHandler.setAuthenticator(authenticator); _server.start(); } @@ -76,16 +80,19 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws @AfterEach public void stopServer() throws Exception { - if (_server.isRunning()) + if (_server != null && _server.isRunning()) { _server.stop(); _server.join(); + _server = null; + _connector = null; } } @Test public void testLoginDispatch() throws Exception { + configureServer(new FormAuthenticator("/login", null, true)); String response = _connector.getResponse("GET /ctx/admin/user HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("dispatcherType: REQUEST")); @@ -96,9 +103,27 @@ public void testLoginDispatch() throws Exception @Test public void testErrorDispatch() throws Exception { + // With dispatch enabled it does a AuthenticationState.serveAs to the error page with REQUEST dispatch type. + configureServer(new FormAuthenticator("/login", "/error", true)); String response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("dispatcherType: REQUEST")); assertThat(response, containsString("contextPath: /ctx")); assertThat(response, containsString("servletPath: /error")); + stopServer(); + + // With no dispatch it should do a redirect to the error page. + configureServer(new FormAuthenticator("/login", "/error", false)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 302 Found")); + assertThat(response, containsString("Location: /ctx/error")); + stopServer(); + + // With no FormAuthenticator error page, it will do an error dispatch to the servlet error page for that code. + configureServer(new FormAuthenticator("/login", null, true)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, containsString("dispatcherType: ERROR")); + assertThat(response, containsString("contextPath: /ctx")); + assertThat(response, containsString("servletPath: /servletErrorPage")); + stopServer(); } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ServerUpgradeRequestTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ServerUpgradeRequestTest.java index 0d8df396a117..9cfdd3e71c55 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ServerUpgradeRequestTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ServerUpgradeRequestTest.java @@ -124,8 +124,7 @@ public AuthenticationState validateRequest(Request request, Response response, C if (user != null) return new UserAuthenticationSucceeded(getAuthenticationType(), user); - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); - return AuthenticationState.SEND_FAILURE; + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/ServerUpgradeRequestTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/ServerUpgradeRequestTest.java index 8c97bce7cbc2..b9706caee64c 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/ServerUpgradeRequestTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/ServerUpgradeRequestTest.java @@ -134,8 +134,7 @@ public AuthenticationState validateRequest(Request request, Response response, C if (user != null) return new UserAuthenticationSucceeded(getAuthenticationType(), user); - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); - return AuthenticationState.SEND_FAILURE; + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); } } diff --git a/jetty-ee11/jetty-ee11-jaspi/src/main/java/org/eclipse/jetty/ee11/security/jaspi/JaspiAuthenticator.java b/jetty-ee11/jetty-ee11-jaspi/src/main/java/org/eclipse/jetty/ee11/security/jaspi/JaspiAuthenticator.java index 7e4974584a10..42fd58cfb266 100644 --- a/jetty-ee11/jetty-ee11-jaspi/src/main/java/org/eclipse/jetty/ee11/security/jaspi/JaspiAuthenticator.java +++ b/jetty-ee11/jetty-ee11-jaspi/src/main/java/org/eclipse/jetty/ee11/security/jaspi/JaspiAuthenticator.java @@ -233,8 +233,7 @@ public AuthenticationState validateRequest(JaspiMessageInfo messageInfo) throws } if (authStatus == AuthStatus.FAILURE) { - Response.writeError(messageInfo.getBaseRequest(), messageInfo.getBaseResponse(), messageInfo.getCallback(), HttpServletResponse.SC_FORBIDDEN); - return AuthenticationState.SEND_FAILURE; + return AuthenticationState.writeError(messageInfo.getBaseRequest(), messageInfo.getBaseResponse(), messageInfo.getCallback(), HttpServletResponse.SC_FORBIDDEN); } // should not happen throw new IllegalStateException("No AuthStatus returned"); diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java index 5c33b1615ac3..1e5738d22948 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java @@ -36,7 +36,6 @@ public class ErrorHandler extends org.eclipse.jetty.server.handler.ErrorHandler { private static final Logger LOG = LoggerFactory.getLogger(ErrorHandler.class); - public static final String ERROR_CHARSET = "org.eclipse.jetty.server.error_charset"; public ErrorHandler() { @@ -45,6 +44,22 @@ public ErrorHandler() setShowMessageInTitle(true); } + @Override + public boolean writeError(Request request, Response response, Callback callback, int code) + { + // If we have not entered the servlet channel we should trigger a sendError for when we do enter the servlet channel. + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + boolean enteredServletChannel = servletContextRequest.getServletChannel().getCallback() != null; + if (!enteredServletChannel) + { + response.setStatus(code); + request.setAttribute(ERROR_STATUS, code); + return false; + } + + return super.writeError(request, response, callback, code); + } + @Override public boolean handle(Request request, Response response, Callback callback) throws Exception { @@ -70,7 +85,8 @@ public boolean handle(Request request, Response response, Callback callback) thr ServletContextHandler.ServletScopedContext context = servletContextRequest.getErrorContext(); Integer errorStatus = (Integer)request.getAttribute(ERROR_STATUS); Throwable errorCause = (Throwable)request.getAttribute(ERROR_EXCEPTION); - if (this instanceof ErrorPageMapper mapper) + boolean enteredServletChannel = servletContextRequest.getServletChannel().getCallback() != null; + if (this instanceof ErrorPageMapper mapper && enteredServletChannel) { ErrorPageMapper.ErrorPage errorPage = mapper.getErrorPage(errorStatus, errorCause); if (LOG.isDebugEnabled()) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java index 7c4033d38056..d876b04ca89c 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java @@ -114,7 +114,6 @@ import org.slf4j.LoggerFactory; import static jakarta.servlet.ServletContext.TEMPDIR; -import static org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS; /** * Servlet Context. @@ -1198,18 +1197,11 @@ protected boolean handleByContextHandler(String pathInContext, ContextRequest re if (isProtectedTarget(pathInContext)) { - if (getErrorHandler() instanceof ErrorHandler.ErrorPageMapper mapper) - { - ErrorHandler.ErrorPageMapper.ErrorPage errorPage = mapper.getErrorPage(404, null); - if (errorPage != null) - { - // Do nothing here other than set the error status so that the ServletHandler will handle as if a sendError - request.setAttribute(ERROR_STATUS, 404); - response.setStatus(HttpStatus.NOT_FOUND_404); - return false; - } - } - Response.writeError(request, response, callback, HttpStatus.NOT_FOUND_404, null); + // If we have a Servlet ErrorHandler, then writeError will return false, and it will signal + // the ServletChannel to trigger a sendError() when it is started. + if (request.getContext().getErrorHandler() instanceof org.eclipse.jetty.server.handler.ErrorHandler errorHandler) + return errorHandler.writeError(request, response, callback, HttpStatus.NOT_FOUND_404); + Response.writeError(request, response, callback, HttpStatus.NOT_FOUND_404); return true; } return false; diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java index b3ec12e7c358..c5b1ccf11f00 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java @@ -20,6 +20,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee11.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.ee11.servlet.ServletContextHandler; import org.eclipse.jetty.security.Constraint; import org.eclipse.jetty.security.EmptyLoginService; @@ -28,25 +29,28 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; public class FormAuthenticatorTest { private Server _server; private LocalConnector _connector; - @BeforeEach - public void configureServer() throws Exception + public void configureServer(FormAuthenticator authenticator) throws Exception { _server = new Server(); _connector = new LocalConnector(_server); _server.addConnector(_connector); ServletContextHandler contextHandler = new ServletContextHandler("/ctx", ServletContextHandler.SESSIONS); + ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler(); + errorPageErrorHandler.addErrorPage(403, "/servletErrorPage"); + contextHandler.setErrorHandler(errorPageErrorHandler); + _server.setHandler(contextHandler); contextHandler.addServlet(new AuthenticationTestServlet(), "/"); @@ -56,7 +60,7 @@ public void configureServer() throws Exception securityHandler.put("/any/*", Constraint.ANY_USER); securityHandler.put("/known/*", Constraint.KNOWN_ROLE); securityHandler.put("/admin/*", Constraint.from("admin")); - securityHandler.setAuthenticator(new FormAuthenticator("/login", "/error", true)); + securityHandler.setAuthenticator(authenticator); _server.start(); } @@ -76,16 +80,19 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws @AfterEach public void stopServer() throws Exception { - if (_server.isRunning()) + if (_server != null && _server.isRunning()) { _server.stop(); _server.join(); + _server = null; + _connector = null; } } @Test public void testLoginDispatch() throws Exception { + configureServer(new FormAuthenticator("/login", null, true)); String response = _connector.getResponse("GET /ctx/admin/user HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("dispatcherType: REQUEST")); @@ -96,9 +103,27 @@ public void testLoginDispatch() throws Exception @Test public void testErrorDispatch() throws Exception { + // With dispatch enabled it does a AuthenticationState.serveAs to the error page with REQUEST dispatch type. + configureServer(new FormAuthenticator("/login", "/error", true)); String response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("dispatcherType: REQUEST")); assertThat(response, containsString("contextPath: /ctx")); assertThat(response, containsString("servletPath: /error")); + stopServer(); + + // With no dispatch it should do a redirect to the error page. + configureServer(new FormAuthenticator("/login", "/error", false)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 302 Found")); + assertThat(response, containsString("Location: /ctx/error")); + stopServer(); + + // With no FormAuthenticator error page, it will do an error dispatch to the servlet error page for that code. + configureServer(new FormAuthenticator("/login", null, true)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, containsString("dispatcherType: ERROR")); + assertThat(response, containsString("contextPath: /ctx")); + assertThat(response, containsString("servletPath: /servletErrorPage")); + stopServer(); } } diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/ServerUpgradeRequestTest.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/ServerUpgradeRequestTest.java index 8bcb150eaf3e..dea8a77bb067 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/ServerUpgradeRequestTest.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/tests/ServerUpgradeRequestTest.java @@ -124,8 +124,7 @@ public AuthenticationState validateRequest(Request request, Response response, C if (user != null) return new UserAuthenticationSucceeded(getAuthenticationType(), user); - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); - return AuthenticationState.SEND_FAILURE; + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); } } diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/ServerUpgradeRequestTest.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/ServerUpgradeRequestTest.java index 4891dd116e23..9dd2f0354fae 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/ServerUpgradeRequestTest.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/ServerUpgradeRequestTest.java @@ -134,8 +134,7 @@ public AuthenticationState validateRequest(Request request, Response response, C if (user != null) return new UserAuthenticationSucceeded(getAuthenticationType(), user); - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); - return AuthenticationState.SEND_FAILURE; + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); } } diff --git a/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java b/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java index a0232ca91542..6252d5fb8818 100644 --- a/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java +++ b/jetty-integrations/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java @@ -513,14 +513,11 @@ protected AuthenticationState handleNonceRequest(Request request, Response respo return AuthenticationState.CHALLENGE; } - private boolean validateSignInWithEthereumToken(SignInWithEthereumToken siwe, SignedMessage signedMessage, Request request, Response response, Callback callback) + private AuthenticationState validateSignInWithEthereumToken(SignInWithEthereumToken siwe, SignedMessage signedMessage, Request request, Response response, Callback callback) { Session session = request.getSession(false); if (siwe == null) - { - sendError(request, response, callback, "failed to parse SIWE message"); - return false; - } + return sendError(request, response, callback, "failed to parse SIWE message");; try { @@ -528,11 +525,10 @@ private boolean validateSignInWithEthereumToken(SignInWithEthereumToken siwe, Si } catch (Throwable t) { - sendError(request, response, callback, t.getMessage()); - return false; + return sendError(request, response, callback, t.getMessage()); } - return true; + return null; } @Override @@ -552,10 +548,7 @@ public AuthenticationState validateRequest(Request request, Response response, C { session = request.getSession(true); if (session == null) - { - sendError(request, response, callback, "session could not be created"); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, callback, "session could not be created"); } if (isNonceRequest(uri)) @@ -568,10 +561,14 @@ public AuthenticationState validateRequest(Request request, Response response, C // Parse and validate SIWE Message. SignedMessage signedMessage = parseMessage(request, response, callback); if (signedMessage == null) - return AuthenticationState.SEND_FAILURE; + return sendError(request, response, callback, "failed to read SIWE message"); SignInWithEthereumToken siwe = SignInWithEthereumToken.from(signedMessage.message()); - if (siwe == null || !validateSignInWithEthereumToken(siwe, signedMessage, request, response, callback)) - return AuthenticationState.SEND_FAILURE; + if (siwe == null) + return sendError(request, response, callback, "failed to parse SIWE message"); + + AuthenticationState authenticationState = validateSignInWithEthereumToken(siwe, signedMessage, request, response, callback); + if (authenticationState != null) + return authenticationState; String address = siwe.address(); UserIdentity user = login(address, null, request, response); @@ -592,8 +589,7 @@ public AuthenticationState validateRequest(Request request, Response response, C return formAuth; } - sendError(request, response, callback, "auth failed"); - return AuthenticationState.SEND_FAILURE; + return sendError(request, response, callback, "auth failed"); } // Look for cached authentication in the Session. @@ -605,13 +601,13 @@ public AuthenticationState validateRequest(Request request, Response response, C !_loginService.validate(((AuthenticationState.Succeeded)authenticationState).getUserIdentity())) { if (LOG.isDebugEnabled()) - LOG.debug("auth revoked {}", authenticationState); + LOG.debug("authentication revoked {}", authenticationState); logoutWithoutRedirect(request, response); - return AuthenticationState.SEND_FAILURE; + return sendError(request, response, callback, "authentication revoked"); } if (LOG.isDebugEnabled()) - LOG.debug("auth {}", authenticationState); + LOG.debug("authenticationState {}", authenticationState); return authenticationState; } @@ -619,7 +615,7 @@ public AuthenticationState validateRequest(Request request, Response response, C if (AuthenticationState.Deferred.isDeferred(response)) { if (LOG.isDebugEnabled()) - LOG.debug("auth deferred {}", session.getId()); + LOG.debug("authentication deferred {}", session.getId()); return null; } @@ -667,23 +663,17 @@ public AuthenticationState validateRequest(Request request, Response response, C * @param callback the callback. * @param message the reason for the error or null. */ - private void sendError(Request request, Response response, Callback callback, String message) + private AuthenticationState sendError(Request request, Response response, Callback callback, String message) { if (LOG.isDebugEnabled()) LOG.debug("Authentication FAILED: {}", message); if (_errorPath == null) { - if (LOG.isDebugEnabled()) - LOG.debug("auth failed 403"); - if (response != null) - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403, message); + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); } else { - if (LOG.isDebugEnabled()) - LOG.debug("auth failed {}", _errorPath); - String contextPath = Request.getContextPath(request); String redirectUri = URIUtil.addPaths(contextPath, _errorPath); if (message != null) @@ -695,6 +685,7 @@ private void sendError(Request request, Response response, Callback callback, St int redirectCode = request.getConnectionMetaData().getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpStatus.MOVED_TEMPORARILY_302 : HttpStatus.SEE_OTHER_303; Response.sendRedirect(request, response, callback, redirectCode, redirectUri, true); + return AuthenticationState.SEND_FAILURE; } } diff --git a/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java b/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java index 9707c51d0e3d..20b88d377449 100644 --- a/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java +++ b/jetty-integrations/jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java @@ -72,7 +72,9 @@ public boolean handle(Request request, Response response, Callback callback) thr String pathInContext = Request.getPathInContext(request); if ("/error".equals(pathInContext)) { - response.write(true, BufferUtil.toBuffer("ERROR"), callback); + response.setStatus(HttpStatus.FORBIDDEN_403); + String error = Request.getParameters(request).get(EthereumAuthenticator.ERROR_PARAMETER).getValue(); + response.write(true, BufferUtil.toBuffer(error), callback); return true; } if ("/login".equals(pathInContext)) @@ -94,6 +96,7 @@ else if ("/logout".equals(pathInContext)) }; _authenticator = new EthereumAuthenticator(); + _authenticator.setErrorPage("/error"); SecurityHandler.PathMapped securityHandler = new SecurityHandler.PathMapped(); securityHandler.setAuthenticator(_authenticator); diff --git a/jetty-integrations/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-integrations/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index 4bef49c31530..a715b5a336fb 100644 --- a/jetty-integrations/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-integrations/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -432,17 +432,11 @@ public AuthenticationState validateRequest(Request request, Response response, C if (session == null) session = request.getSession(true); if (session == null) - { - sendError(request, response, cb, "session could not be created"); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, cb, "session could not be created"); String sessionIdFrom = (String)request.getAttribute("org.eclipse.jetty.session.RequestedSession.sessionIdFrom"); if (sessionIdFrom != null && !sessionIdFrom.startsWith("cookie")) - { - sendError(request, response, cb, "Session ID must be a cookie to support OpenID authentication"); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, cb, "Session ID must be a cookie to support OpenID authentication"); // Handle a request for authentication. if (isJSecurityCheck(uri)) @@ -450,17 +444,11 @@ public AuthenticationState validateRequest(Request request, Response response, C Fields parameters = getParameters(request); String authCode = parameters.getValue("code"); if (authCode == null) - { - sendError(request, response, cb, "auth failed: no code parameter"); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, cb, "auth failed: no code parameter"); String state = parameters.getValue("state"); if (state == null) - { - sendError(request, response, cb, "auth failed: no state parameter"); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, cb, "auth failed: no state parameter"); // Verify anti-forgery state token. UriRedirectInfo uriRedirectInfo; @@ -469,19 +457,13 @@ public AuthenticationState validateRequest(Request request, Response response, C uriRedirectInfo = removeAndClearCsrfMap(session, state); } if (uriRedirectInfo == null) - { - sendError(request, response, cb, "auth failed: invalid state parameter"); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, cb, "auth failed: invalid state parameter"); // Attempt to login with the provided authCode. OpenIdCredentials credentials = new OpenIdCredentials(authCode, getRedirectUri(request)); UserIdentity user = login(null, credentials, request, response); if (user == null) - { - sendError(request, response, cb, null); - return AuthenticationState.SEND_FAILURE; - } + return sendError(request, response, cb, null); LoginAuthenticator.UserAuthenticationSent openIdAuth = new LoginAuthenticator.UserAuthenticationSent(getAuthenticationType(), user); if (LOG.isDebugEnabled()) @@ -612,23 +594,17 @@ public AuthenticationState validateRequest(Request request, Response response, C * @param message the reason for the error or null. * @throws IOException if sending the error fails for any reason. */ - private void sendError(Request request, Response response, Callback callback, String message) throws IOException + private AuthenticationState sendError(Request request, Response response, Callback callback, String message) throws IOException { if (LOG.isDebugEnabled()) LOG.debug("OpenId authentication FAILED: {}", message); if (_errorPage == null) { - if (LOG.isDebugEnabled()) - LOG.debug("auth failed 403"); - if (response != null) - Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); + return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403); } else { - if (LOG.isDebugEnabled()) - LOG.debug("auth failed {}", _errorPage); - String contextPath = Request.getContextPath(request); String redirectUri = URIUtil.addPaths(contextPath, _errorPage); if (message != null) @@ -640,6 +616,7 @@ private void sendError(Request request, Response response, Callback callback, St int redirectCode = request.getConnectionMetaData().getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpStatus.MOVED_TEMPORARILY_302 : HttpStatus.SEE_OTHER_303; Response.sendRedirect(request, response, callback, redirectCode, redirectUri, true); + return AuthenticationState.SEND_FAILURE; } }