Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes for Response.writeError with servlet error dispatch #12698

Open
wants to merge 6 commits into
base: jetty-12.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,28 @@ public boolean errorPageForMethod(String method)
return ERROR_METHODS.contains(method);
}

/**
* Write an error response, or signal that the error will be handled by a down stream handler.
* <p>
* The default implementation calls {@link Response#writeError(Request, Response, Callback, int)} and returns {@code true}.
* <p>
* <p>
* Servlet implementations will override this method to trigger a sendError when the {@code ServletChannel} is started and return {@code false}.
* </p>
Comment on lines +100 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* <p>
* Servlet implementations will override this method to trigger a sendError when the {@code ServletChannel} is started and return {@code false}.
* </p>
* <p>
* {@code ErrorHandler} extensions may override this method to modify the state of the
* {@code request} and/or {@code response} so that when the method returns {@code false}
* an error page will be generated by a subsequent {@code Handler}. For example,
* Servlet implementations override this method to set attributes that trigger a
* Servlet `sendError` call when the {@code ServletHandler} invokes the {@code ServletChannel}.
* </p>

*
* @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 {@code true} if the error response was written; {@code false} if the state of the request and/or response
* has been changed so that by continuing normal handling an error response will be generated by a down stream handler.
**/
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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading
Loading